Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ft/docx rendering, RE: #304 #347

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

NyanHelsing
Copy link
Contributor

This is an update to #304

Ticket

https://openscience.atlassian.net/browse/SVCS-488
OSF side PR: CenterForOpenScience/osf.io#8002

Purpose

This ticket replaces #282. Credit goes to @AddisonSchiller 🎆🎆.

.docx rendering is very intensive on the OSF. By using Microsoft's online rendering service to render publicly available .docx files, we can remove a lot of pressure from the unoconv container.

Changes

  • Added support for public renderers.
  • Added the office365 renderer under the public renderers.
  • Added support to parse for a public_file query param. This query param is optional.public_file=True denotes that the file is public (the project it belongs to is public) , while public_file=False denotes that it is private. All other values for public_file cause errors to be raised.
  • ProviderMetadata now has an is_public flag, with default value set to False.

Side Effects

  • Does Microsoft enforces a rate limit?
  • How can we handle the "a few hour" cache problem?
  • What if this external service is down? Should we detect in advance?
  • iframe sandboxing may cause issue, need to verify on staging.

QA Notes

The Office365 renderer does not use the.pdf renderer like unoconv used to, so the pdfs that get made by this renderer may not display exactly the same. More QA notes to come. There is also a README.md in the renderer with more information about testing.

Deployment Notes

  • OSF side must be deployed first (no side effects).

AddisonSchiller and others added 3 commits August 3, 2018 14:56
Adding support for a `public_file` query param so the OSF can request a
public renderer. Added office365 which is a public renderer. This uses office
online to do .docx file conversions.
Cleans up some imports that don't need to be qualified, or explicitly
imports whatever resources the files need.
@NyanHelsing NyanHelsing changed the title Ft/docx rendering Ft/docx rendering, RE: #304 Aug 3, 2018
`markupsafe.escape(url)` is a little more informative to read than just
`escape(url)`, so this reverts the change to use the former.
@coveralls
Copy link

coveralls commented Aug 3, 2018

Coverage Status

Coverage decreased (-0.2%) to 71.042% when pulling edc9c18 on birdbrained:ft/docx-rendering into d15d322 on CenterForOpenScience:develop.

Long strings are better quotes with triple quotes so they're more
readable, also has the nice effect of lettin strings that have other
quotation marks in them not need to be escaped.
This changes th QueryParameterError to append a tuple to its attr stack
rather than a list.
Added a clarifiaction on why the renderer won't work locally. The
rendering service needs to access the file thatis to be rendered, and
the url waterbutler gives the renderer is going to be inaccessible to
the outside world, unless the developer configures waterbutler to use an
externally accessible hostname/ip.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants