Skip to content

#15: Add storage.batch.Batch #654

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Feb 26, 2015
Merged

#15: Add storage.batch.Batch #654

merged 9 commits into from
Feb 26, 2015

Conversation

@tseaver
Copy link
Contributor

@tseaver tseaver commented Feb 16, 2015

Step #2 in #15 (comment)

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 16, 2015
@tseaver tseaver mentioned this pull request Feb 16, 2015
Closed
@coveralls
Copy link

coveralls commented Feb 16, 2015

Coverage Status

Changes Unknown when pulling 5fed4d2 on tseaver:15-add_storage_batch into * on GoogleCloudPlatform:master*.

@tseaver
Copy link
Contributor Author

tseaver commented Feb 16, 2015

@craigcitro, @dhermes I'm willing to do the work to vendor in and use the apitools batch module, if Craig doesn't think it will have a PyPI release soon. I can also work on getting it up-to-speed in terms of test coverage and Py3k straddling in the canonical repository, if that would help.

@dhermes
Copy link
Contributor

dhermes commented Feb 17, 2015

Should I review this or rely on the vendored in code?

@tseaver
Copy link
Contributor Author

tseaver commented Feb 17, 2015

@dhermes we don't have a vendored-in version of that module yet. My earlier question was whether we should go ahead and vendor that code in, given that it has no coverage and does not straddle Py3k yet.

The amount of code in the new storage/batch.py module which would be replaced if we chose to vendor the apitools module is relatively small (135 out of 544 lines):

dhermes
dhermes reviewed Feb 18, 2015
View reviewed changes
gcloud/storage/batch.py Outdated

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@coveralls
Copy link

coveralls commented Feb 18, 2015

Coverage Status

Coverage remained the same at 100.0% when pulling ae0e9cf on tseaver:15-add_storage_batch into 8e68a20 on GoogleCloudPlatform:master.

@dhermes dhermes added the api: storage Issues related to the Cloud Storage API. label Feb 18, 2015
@coveralls
Copy link

coveralls commented Feb 18, 2015

Coverage Status

Coverage remained the same at 100.0% when pulling 6261272 on tseaver:15-add_storage_batch into 8e68a20 on GoogleCloudPlatform:master.

dhermes
dhermes reviewed Feb 18, 2015
View reviewed changes
gcloud/storage/batch.py

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@coveralls
Copy link

coveralls commented Feb 19, 2015

Coverage Status

Coverage remained the same at 100.0% when pulling af2a382 on tseaver:15-add_storage_batch into 8e68a20 on GoogleCloudPlatform:master.

@tseaver
Add 'storage.batch.Batch'
653fa14 
A batch proxies a connection, deferring write requests.
@tseaver
Factor out 'storage.connection.Connection._do_request()'.
63ea7ba 
In preparation for making 'storage.batch.Batch' derive from Connection,
allowing it to override only the actual transmission of the HTTP request.
@tseaver
Make 'Batch' subclass 'Connection', and override '_do_request'.
dd2046a 
Drop patching the connection's 'http', as well as proxying its attributes
(we get that via subclassing).
@tseaver
Copy link
Contributor Author

tseaver commented Feb 25, 2015

@dhermes PTAL.

Rebased on top of master, squashing all previous commits; then implemented subclassing per our discussion today.

dhermes
dhermes reviewed Feb 26, 2015
View reviewed changes
gcloud/storage/batch.py Outdated

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes
dhermes reviewed Feb 26, 2015
View reviewed changes
gcloud/storage/batch.py

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Feb 26, 2015

RE: #654 (comment)

@craigcitro can we help get apitools:

  • In PyPI
  • Supporting Py3
  • Merging upstream changes from places like gsutil

dhermes
dhermes reviewed Feb 26, 2015
View reviewed changes
regression/storage.py

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@craigcitro
Copy link
Contributor

craigcitro commented Feb 26, 2015

replying to myself for posterity: i believe neither uploads nor downloads work in a batch.

@craigcitro
Copy link
Contributor

craigcitro commented Feb 26, 2015

@dhermes yes! so here's the current apitools plan:

  • i'm working on merging the gsutil fork today
  • once i do that, i'm going to submit to pypi
  • after that, help getting it py3 ready would be AWESOME
  • even right now, py3 support for protorpc would be super

@tseaver
Fix URL for batch requests.
b5a8124 
The batching interface is not specific to the storage API.
@tseaver
Conform to httplib2.response.
96807c8 
It does not case-normalize header key lookup, but stores headers only
as lowercase.
@tseaver
Exercise 'storage.batch.Batch' during bucket cleanup.
edeb950
@tseaver
Use six.PY2, rather than sys.version_info comparison.
c083ceb 
Addresses:  #654 (comment)
@dhermes
Copy link
Contributor

dhermes commented Feb 26, 2015

For posterity (RE: Craig's comment):

Note: Currently, Google Cloud Storage does not support batch operations for
media, either for upload or download.

screen_shot_074

@coveralls
Copy link

coveralls commented Feb 26, 2015

Coverage Status

Coverage remained the same at 100.0% when pulling de6fd48 on tseaver:15-add_storage_batch into 808c38b on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor

dhermes commented Feb 26, 2015

I think everything looks good except removing that unused sys import.

LGTM

@dhermes
Copy link
Contributor

dhermes commented Feb 26, 2015

@craigcitro Can we start a new issue to discuss apitools / protorpc? Should the issue be here or in one (or both) of those repos?

@craigcitro
Copy link
Contributor

craigcitro commented Feb 26, 2015

@dhermes yeah, new issue wherever sgtm. maybe a "py3 support" issue in each of protorpc/apitools, and an issue here for "better apitools dependency" blocked by those two?

@coveralls
Copy link

coveralls commented Feb 26, 2015

Coverage Status

Coverage remained the same at 100.0% when pulling c828bda on tseaver:15-add_storage_batch into d43c4a0 on GoogleCloudPlatform:master.

tseaver added a commit that referenced this pull request Feb 26, 2015
@tseaver
Merge pull request #654 from tseaver/15-add_storage_batch
f0a1157 
#15: Add `storage.batch.Batch`
@tseaver tseaver merged commit f0a1157 into googleapis:master Feb 26, 2015
@tseaver tseaver deleted the 15-add_storage_batch branch February 26, 2015 19:10
This was referenced Feb 26, 2015
Closed
Closed
@dhermes dhermes mentioned this pull request Mar 12, 2015
Merged
@craigcitro craigcitro mentioned this pull request Apr 7, 2015
Closed
@dhermes
Copy link
Contributor

dhermes commented Apr 9, 2015

@tseaver We discussed using the apitools batch support when this went in. We should discuss if / how that's possible (in light of #811).

parthea pushed a commit that referenced this pull request Nov 24, 2025
@software-dov
fix: modules referenced in MapField message type are properly aliased (…
6ede787 
…#654)

This was noticed when attempting to generate Bigtable Admin in a
message definition: an imported module is given an alias to prevent
collision with a field name. When the module is referenced to describe
the type of a singleton field it is properly disambiguated. When used
to describe the type of a MapField it is _not_ disambiguated.

Fix for that.

Closes #618
parthea pushed a commit that referenced this pull request Nov 24, 2025
@daniel-sanche
fix: json fields dictionary has modification side effect (#654)
7d94be9
parthea pushed a commit that referenced this pull request Nov 24, 2025
feat: don't flush entire global cache on transient errors (#654)
afa4fdb 
It turns out this was unnecessary and can cause problems of its own.

Fixes #649 #634
parthea pushed a commit that referenced this pull request Nov 25, 2025
@daniel-sanche
fix: json fields dictionary has modification side effect (#654)
24689d8
parthea pushed a commit that referenced this pull request Nov 26, 2025
@tseaver
feat: add Python 3.9 support, drop Python 3.5 support (#655)
65b79d4 
Closes #654.
parthea pushed a commit that referenced this pull request Nov 26, 2025
@release-please
chore: release 1.24.0 (#656)
a4c7d60 
🤖 I have created a release \*beep\* \*boop\* 
---
## [1.24.0](https://www.github.com/googleapis/google-auth-library-python/compare/v1.23.0...v1.24.0) (2020-12-11)


### Features

* add Python 3.9 support, drop Python 3.5 support ([#655](https://www.github.com/googleapis/google-auth-library-python/issues/655)) ([6de753d](https://www.github.com/googleapis/google-auth-library-python/commit/6de753d585254c813b3e6cbde27bf5466261ba10)), closes [#654](https://www.github.com/googleapis/google-auth-library-python/issues/654)


### Bug Fixes

* avoid losing the original '_include_email' parameter in impersonated credentials ([#626](https://www.github.com/googleapis/google-auth-library-python/issues/626)) ([fd9b5b1](https://www.github.com/googleapis/google-auth-library-python/commit/fd9b5b10c80950784bd37ee56e32c505acb5078d))


### Documentation

* fix typo in import ([#651](https://www.github.com/googleapis/google-auth-library-python/issues/651)) ([3319ea8](https://www.github.com/googleapis/google-auth-library-python/commit/3319ea8ae876c73a94f51237b3bbb3f5df2aef89)), closes [#650](https://www.github.com/googleapis/google-auth-library-python/issues/650)
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tseaver @coveralls @dhermes @craigcitro @googlebot