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

file.GetContentFile: stream to disk, add callback #30

Merged
merged 15 commits into from
May 5, 2020

Conversation

casperdcl
Copy link
Contributor

@casperdcl casperdcl commented May 3, 2020

And the (pydrive2) docstrings did say:

:returns: str -- utf-8 decoded content of the file

And again, it (httplib2) said:

The return value is [...] a string that contains the response entity body.

And the devs did shudder. There was wailing in the North and South. There was gnashing of teeth in the East and West.

And then one brave dev, in a quavering voice, spake thusly:

Let there be no more in-memory downloads. Let the files stream until the hard drives overfloweth. Let RAM be ignored, and let us answer the call to progress.

@casperdcl casperdcl self-assigned this May 3, 2020
@casperdcl casperdcl requested review from efiop and shcheklein May 3, 2020 18:26
@casperdcl casperdcl added bug Something isn't working enhancement New feature or request labels May 3, 2020
@casperdcl casperdcl marked this pull request as draft May 3, 2020 18:46
pydrive2/files.py Outdated Show resolved Hide resolved
@casperdcl
Copy link
Contributor Author

https://travis-ci.com/github/iterative/PyDrive2/builds/163561690 fails because a test expects the BOM to be stripped even when there's an implicit remove_bom=False. Maybe the BOM was not inserted in the old implementation but suddenly is inserted using the new download method?

pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
pydrive2/files.py Outdated Show resolved Hide resolved
@@ -34,6 +35,10 @@ def __init__(self, http_error):
# Initialize args for backward compatibility
super().__init__(http_error)

def GetField(self, field):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can simplify DVC code after release probably

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, on my post-merge list :)

@shcheklein
Copy link
Member

Looks good! Fixing the tests sounds good to me.

Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool! 😎 Let's fix the test and release the new version. We can simplify DVC a bit after this release.

@casperdcl casperdcl marked this pull request as ready for review May 5, 2020 19:32
@shcheklein shcheklein merged this pull request into master May 5, 2020
@shcheklein shcheklein deleted the download-stream branch May 5, 2020 20:22
casperdcl added a commit to casperdcl/dvc that referenced this pull request May 5, 2020
efiop pushed a commit to iterative/dvc that referenced this pull request May 6, 2020
* wip

* gdrive: add progress

Part of #2865
See #2865 (comment)

* gdrive: move towards next pydrive2 release

- depends on iterative/PyDrive2#30

* update to latest pydrive>=1.4.11

* avoid unneeded API call

* progress: gdrive: ensure proper bar_format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

download streaming & callbacks
2 participants