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

Error when downloading blobs with storage v1.1.0 #3342

Closed
theacodes opened this issue Apr 28, 2017 · 5 comments
Closed

Error when downloading blobs with storage v1.1.0 #3342

theacodes opened this issue Apr 28, 2017 · 5 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@theacodes
Copy link
Contributor

@theacodes theacodes added api: storage Issues related to the Cloud Storage API. priority: p0 Highest priority. Critical issue. P0 implies highest priority. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Apr 28, 2017
@dhermes
Copy link
Contributor

dhermes commented Apr 28, 2017

So this is kind of a bug, kind of not a bug, but some code needs to change.

TLDR; My vote for a fix: make the Blob.download_to_filename file timestamp update optional, dependent on self.updated is not None


The old implementation did something somewhat silly. It called reload on the instance if the media_link wasn't present, essentially forcing users to incur two round trips.

I "turned off" this "feature" in my fix (and @tseaver asked why / if that was OK during code review). However, that revealed a (IMO long-standing) bug elsewhere in the code:

mtime = time.mktime(self.updated.timetuple())

i.e. the Blob.download_to_filename assumes the updated property is populated, which assumes reload has been called (or the _properties have been populated in some other way).

@theacodes
Copy link
Contributor Author

TLDR; My vote for a fix: make the Blob.download_to_filename file timestamp update optional, dependent on self.updated is not None

Fine with me.

@danigosa
Copy link

Same issue here, is there any workaround or sticking to 1.0.0 'til the fix? All our tests failed miserably because of the upgrade.

@dhermes
Copy link
Contributor

dhermes commented May 2, 2017

@danigosa I'll send out a fix today and we can push a 1.1.1 release. SGTY @jonparrott and @lukesneeringer?

@theacodes
Copy link
Contributor Author

theacodes commented May 2, 2017 via email

dhermes added a commit to dhermes/google-cloud-python that referenced this issue May 2, 2017
@theacodes theacodes removed the priority: p0 Highest priority. Critical issue. P0 implies highest priority. label May 2, 2017
richkadel pushed a commit to richkadel/google-cloud-python that referenced this issue May 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

3 participants