-
Notifications
You must be signed in to change notification settings - Fork 88
[SVCS-528] Look for mfr query param from mfr in v1 provider
#290
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
[SVCS-528] Look for mfr query param from mfr in v1 provider
#290
Conversation
We can export .gdoc as a pdf if we know its coming from mfr. Added `alt_export` functions and properties to Gdrive utils and metadata. Gdrive download now looks for 'mfr' in its kwargs and will request file as pdf if used properly.
mfr query param from mfr in v1 providermfr query param from mfr in v1 provider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass done! I will do a local test during the second pass. 🔥 🔥
|
|
||
| metadata = await self.metadata(path, revision=revision) | ||
|
|
||
| if 'mfr' in kwargs and kwargs['mfr'] and kwargs['mfr'].lower() == 'true': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For dictionaries, we can use the .get with default value None if key does not exist.
mfr_only = kwargs.get('mfr', None)
if mfr_only and mfr_only.lower() == 'true':
...|
|
||
| download_resp = await self.make_request( | ||
| 'GET', | ||
| metadata.raw.get('downloadUrl') or drive_utils.get_export_link(metadata.raw), # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On metadata.raw.get('downloadUrl') vs drive_utils.get_export_link(metadata.raw): from the name I assume that the former is for download while the latter for export. Is it possible that the downloadUrl turns out to be not None so we never get a chance to hit the get_alt_export_link or get_export_link.
| def get_alt_download_extension(metadata): | ||
| format = get_format(metadata) | ||
| try: | ||
| return format['alt_download_ext'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about the following? Please note that format is a python built-in.
metadata_format = get_format(metadata)
return metadata_format.get('alt_download_ext', None) or metadata_format.get('download_ext', None)| def get_alt_export_link(metadata): | ||
| format = get_format(metadata) | ||
| try: | ||
| return metadata['exportLinks'][format['alt_type']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is my example, but it looks more complicated than yours. I can't use type since it is a built-in as well.
metadata_format = get_format(metadata)
format_type = metadata_format.get('alt_type', None) or metadata_format.get('type', None)
export_links = metadata.get('exportLinks', None)
if format_type and export_links:
return export_links.get('format_type', None)
return None|
|
||
| class TestOperationsOrMisc: | ||
|
|
||
| def test_misc_utils(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to make this more specific (both method name and the test inside). What is this test trying to do?
Change download_url for `mfr` case
|
Looks like this has been failing Travis putting it in AD. |
|
👍 |
|
Replaced by #304. |
We can export .gdoc as a pdf if we know its coming from
mfr. Added
alt_exportfunctions and properties to Gdriveutils and metadata. Gdrive download now looks for 'mfr' in its
kwargs and will request file as pdf if used properly.
This ticket should be merged BEFORE its MFR counterpart
Ticket
https://openscience.atlassian.net/browse/SVCS-528
MFR counterpart: CenterForOpenScience/modular-file-renderer#292
Purpose
Allow mfr to request .gdoc files in other export formats (to .pdf from .docx). However allow the osf to still download in .docx format
This will allow MFR to cut out the expensive unoconv conversion for .gdoc files
Changes
v1 provider now looks for a 'mfr' query param. if true, the google drive provider will use
alt_export_nameandalt_export_link()instead of its normal counterparts. This will cause the file being sent to be a .pdf over a .docxAdded tests for new changes.
Side effects
There should be none. File downloads from gdrive will not be effected as long as they don't include
...&mfr=trueas a query paramQA Notes
This needs to be tested with its mfr counterpart to have noticeable changes.
Deployment Notes