[SVCS-528] [Blocked] Improve gdoc rendering - WB Part #304

cslzchen · 2017-12-05T15:52:11Z

Ticket

https://openscience.atlassian.net/browse/SVCS-528
MFR counterpart: CenterForOpenScience/modular-file-renderer#303

Purpose

This PR replaces #290. Credit goes to @AddisonSchiller 🎆 🎆 .

Allow MFR to request .gdoc files in .pdf format while keep .docx format for downloading. This will allow MFR to cut out the expensive unoconv conversion for .gdoc files

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.

Changes

WB V1 provider now looks for a mfr= query param. If mfr=true, the google drive provider will use alt_export_name and alt_export_link() instead of its normal counterparts. This will cause the file being sent to be a .pdf instead of .docx.

Side effects

None

QA Notes

Test that .gdoc rendering works as usual.

Deployment Notes

This PR must be merged/deployed at the same time as its MFR counterpart.
This PR should be blocked by @TomBaxter 's v3 update: #276.

coveralls · 2017-12-05T16:01:08Z

Coverage increased (+0.03%) to 89.843% when pulling 4be8f0e on cslzchen:feature/svcs-528-improve-gdoc into f6e049a on CenterForOpenScience:develop.

cslzchen · 2017-12-05T16:12:01Z

waterbutler/providers/googledrive/provider.py

+        else:
+
+            # TODO figure out metadata.raw.get('downloadUrl')
+            download_url = metadata.raw.get('downloadUrl') or drive_utils.get_export_link(metadata.raw)  # type: ignore


This assumes that metadata.raw.get('downloadUrl') is always None. Why?

My understanding of this line is that downloadUrl is always present unless it's a file that that isn't exportable in it's native type, so a .txt will have a downloadUrl, but a .gdoc won't. Google files have frustratingly nonstandard metadata.

cslzchen · 2017-12-05T20:07:15Z

waterbutler/providers/googledrive/provider.py

        metadata = await self.metadata(path, revision=revision)

+        if kwargs.get('mfr', None) and kwargs['mfr'].lower() == 'true':
+            download_url = drive_utils.get_alt_export_link(metadata.raw)  # type: ignore


download_url = metadata.raw.get('downloadUrl') or drive_utils.get_alt_export_link(metadata.raw)

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.

raises key error and update gdrive utils to use `dict.get()` instead of `dict[]`.

cslzchen · 2017-12-06T15:50:18Z

waterbutler/providers/googledrive/provider.py

        metadata = await self.metadata(path, revision=revision)

+        if kwargs.get('mfr', None) and kwargs['mfr'].lower() == 'true':
+            download_url = metadata.raw.get('downloadUrl') or drive_utils.get_alt_export_link(metadata.raw)


FYI, fixed. Need to check metadata.raw.get('downloadUrl') first. There are file types on gdrive (e.g. .txt) that do not have export_links.

what about

if kwargs.get('mfr', '').lower() == 'true':

Also I don't get the or here.

coveralls · 2017-12-06T16:08:08Z

Coverage increased (+0.03%) to 89.809% when pulling a7c70ef on cslzchen:feature/svcs-528-improve-gdoc into 6249a7a on CenterForOpenScience:develop.

NyanHelsing · 2018-04-11T20:45:18Z

waterbutler/providers/googledrive/provider.py

        metadata = await self.metadata(path, revision=revision)

+        if kwargs.get('mfr', None) and kwargs['mfr'].lower() == 'true':
+            download_url = metadata.raw.get('downloadUrl') or drive_utils.get_alt_export_link(metadata.raw)


what about

if kwargs.get('mfr', '').lower() == 'true':

Also I don't get the or here.

NyanHelsing · 2018-04-11T20:53:31Z

waterbutler/providers/googledrive/utils.py

-        if format['mime_type'] == metadata['mimeType']:
-            return format
+    for format_type in DOCS_FORMATS:
+        if format_type.get('mime_type') == metadata.get('mimeType'):


metadata.get('mimeType') should be outside the for loop so it doesn't need to be accessed each iteration. Given that this data structure is tiny so I doubt there will be any perf. difference.

NyanHelsing · 2018-04-11T20:56:34Z

waterbutler/providers/googledrive/utils.py

-    format = get_format(metadata)
-    return metadata['exportLinks'][format['type']]
+    format_type = get_format(metadata)
+    return metadata.get('exportLinks').get(format_type.get('type'))


What if metadata doesn't have an exportLinks?

NyanHelsing · 2018-07-31T19:40:45Z

I think some of the stuff here may be better passed as a query param that waterbutler passes through to gdrive transparently, this pr currently puts code that deals with exporting into mfr, I'd like to look at if theres a way to put that logic in mfr

cslzchen mentioned this pull request Dec 5, 2017

[SVCS-528] Look for mfr query param from mfr in v1 provider #290

Closed

cslzchen mentioned this pull request Dec 5, 2017

[SVCS-528] Improve gdoc rendering - MFR Part CenterForOpenScience/modular-file-renderer#303

Open

cslzchen commented Dec 5, 2017

View reviewed changes

cslzchen force-pushed the feature/svcs-528-improve-gdoc branch from 4768410 to 38b4e1a Compare December 6, 2017 15:38

AddisonSchiller and others added 2 commits December 6, 2017 10:45

Look for mfr query param from mfr in v1 provider

197582d

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.

Fix the issue where get_alt_export_link

a7c70ef

raises key error and update gdrive utils to use `dict.get()` instead of `dict[]`.

cslzchen force-pushed the feature/svcs-528-improve-gdoc branch from 38b4e1a to a7c70ef Compare December 6, 2017 15:45

cslzchen commented Dec 6, 2017

View reviewed changes

cslzchen changed the title ~~[SVCS-528] Improve gdoc rendering (WB Part)~~ [SVCS-528] [Blocked] Improve gdoc rendering (WB Part) Dec 6, 2017

cslzchen added the Final Review label Dec 7, 2017

cslzchen changed the title ~~[SVCS-528] [Blocked] Improve gdoc rendering (WB Part)~~ [SVCS-528] [Blocked] Improve gdoc rendering - WB Part Dec 7, 2017

NyanHelsing reviewed Apr 11, 2018

View reviewed changes

felliott added the Blocked label May 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SVCS-528] [Blocked] Improve gdoc rendering - WB Part #304

[SVCS-528] [Blocked] Improve gdoc rendering - WB Part #304

Uh oh!

cslzchen commented Dec 5, 2017 •

edited

Loading

Uh oh!

coveralls commented Dec 5, 2017 •

edited

Loading

Uh oh!

cslzchen Dec 5, 2017

Uh oh!

Johnetordoff Dec 5, 2017 •

edited

Loading

Uh oh!

cslzchen Dec 5, 2017

Uh oh!

cslzchen Dec 6, 2017

Uh oh!

NyanHelsing Apr 11, 2018

Uh oh!

coveralls commented Dec 6, 2017 •

edited

Loading

Uh oh!

NyanHelsing Apr 11, 2018

Uh oh!

NyanHelsing Apr 11, 2018

Uh oh!

NyanHelsing Apr 11, 2018

Uh oh!

NyanHelsing commented Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[SVCS-528] [Blocked] Improve gdoc rendering - WB Part #304

Are you sure you want to change the base?

[SVCS-528] [Blocked] Improve gdoc rendering - WB Part #304

Uh oh!

Conversation

cslzchen commented Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

Purpose

Changes

Side effects

QA Notes

Deployment Notes

Uh oh!

coveralls commented Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cslzchen Dec 5, 2017

Choose a reason for hiding this comment

Uh oh!

Johnetordoff Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cslzchen Dec 5, 2017

Choose a reason for hiding this comment

Uh oh!

cslzchen Dec 6, 2017

Choose a reason for hiding this comment

Uh oh!

NyanHelsing Apr 11, 2018

Choose a reason for hiding this comment

Uh oh!

coveralls commented Dec 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NyanHelsing Apr 11, 2018

Choose a reason for hiding this comment

Uh oh!

NyanHelsing Apr 11, 2018

Choose a reason for hiding this comment

Uh oh!

NyanHelsing Apr 11, 2018

Choose a reason for hiding this comment

Uh oh!

NyanHelsing commented Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cslzchen commented Dec 5, 2017 •

edited

Loading

coveralls commented Dec 5, 2017 •

edited

Loading

Johnetordoff Dec 5, 2017 •

edited

Loading

coveralls commented Dec 6, 2017 •

edited

Loading