#15: Add `storage.batch.Batch` #654

tseaver · 2015-02-16T18:03:41Z

coveralls · 2015-02-16T18:22:26Z

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

tseaver · 2015-02-16T18:25:22Z

@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 · 2015-02-17T01:21:29Z

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

tseaver · 2015-02-17T18:01:49Z

@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):

The MIMEApplicationHTTP class and its tests
The bits of `Batch.finish()' which build, send, and process the response for the multipart request and corresponding tests.

gcloud/storage/batch.py

+
+        # Flatten payload
+        if six.PY3:             # pragma: NO COVER  Python3
+            buf = io.StringIO()


coveralls · 2015-02-18T03:44:09Z

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

coveralls · 2015-02-18T05:58:13Z

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

gcloud/storage/batch.py

+            headers['Content-Length'] = len(body)
+        if body is None:
+            body = ''
+        lines = ['%s %s HTTP/1.1' % (method, uri)]


coveralls · 2015-02-19T20:23:34Z

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

A batch proxies a connection, deferring write requests.

In preparation for making 'storage.batch.Batch' derive from Connection, allowing it to override only the actual transmission of the HTTP request.

Drop patching the connection's 'http', as well as proxying its attributes (we get that via subclassing).

tseaver · 2015-02-25T19:11:25Z

@dhermes PTAL.

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

gcloud/storage/batch.py

+        lines.append('')
+        lines.append(body)
+        payload = '\r\n'.join(lines)
+        if sys.version_info[0] < 3:  # pragma: NO COVER  Python2


craigcitro · 2015-02-26T18:10:24Z

one quick note: @dhermes was asking about batch+upload. at least according to the docs, this is not supported:
https://cloud.google.com/storage/docs/json_api/v1/how-tos/batch

(last line in "overview")

gcloud/storage/batch.py

+                and ``content`` (a string).
+        :returns: The HTTP response object and the content of the response.
+        """
+        if method == 'GET':


The batching interface is not specific to the storage API.

gcloud/storage/batch.py

+        :raises: ValueError if no requests have been deferred.
+        """
+        if len(self._requests) == 0:
+            raise ValueError("No deferred requests")


dhermes · 2015-02-26T18:22:15Z

RE: #654 (comment)

@craigcitro can we help get apitools:

In PyPI
Supporting Py3
Merging upstream changes from places like gsutil

regression/storage.py

@@ -52,15 +53,16 @@ def setUp(self):
        self.case_buckets_to_delete = []

    def tearDown(self):
-        for bucket in self.case_buckets_to_delete:
-            bucket.delete()
+        with Batch(CONNECTION) as batch:


craigcitro · 2015-02-26T18:26:20Z

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

craigcitro · 2015-02-26T18:27:49Z

@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

The batching interface is not specific to the storage API.

It does not case-normalize header key lookup, but stores headers only as lowercase.

Addresses: #654 (comment)

dhermes · 2015-02-26T18:32:57Z

For posterity (RE: Craig's comment):

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

coveralls · 2015-02-26T18:39:30Z

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

dhermes · 2015-02-26T18:50:23Z

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

LGTM

dhermes · 2015-02-26T18:51:11Z

@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 · 2015-02-26T18:57:23Z

@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 · 2015-02-26T19:04:54Z

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

#15: Add `storage.batch.Batch`

dhermes · 2015-04-09T01:05:51Z

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

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 16, 2015

tseaver mentioned this pull request Feb 16, 2015

Add support for batch requests in gcloud.storage #15

Closed

dhermes reviewed Feb 18, 2015
View reviewed changes

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

dhermes reviewed Feb 18, 2015
View reviewed changes

tseaver added 3 commits February 25, 2015 13:28

Add 'storage.batch.Batch'

653fa14

A batch proxies a connection, deferring write requests.

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.

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).

dhermes reviewed Feb 26, 2015
View reviewed changes

gcloud/storage/batch.py

lines.append('')

lines.append(body)

payload = '\r\n'.join(lines)

if sys.version_info[0] < 3: # pragma: NO COVER Python2

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes reviewed Feb 26, 2015
View reviewed changes

gcloud/storage/batch.py

and ``content`` (a string).

:returns: The HTTP response object and the content of the response.

"""

if method == 'GET':

This comment was marked as spam.

Sign in to view

Fix URL for batch requests.

71c32a8

The batching interface is not specific to the storage API.

dhermes reviewed Feb 26, 2015
View reviewed changes

tseaver added 4 commits February 26, 2015 13:30

Fix URL for batch requests.

b5a8124

The batching interface is not specific to the storage API.

Conform to httplib2.response.

96807c8

It does not case-normalize header key lookup, but stores headers only as lowercase.

Exercise 'storage.batch.Batch' during bucket cleanup.

edeb950

Use six.PY2, rather than sys.version_info comparison.

c083ceb

Addresses: #654 (comment)

Unused import.

c828bda

tseaver added a commit that referenced this pull request Feb 26, 2015

Merge pull request #654 from tseaver/15-add_storage_batch

f0a1157

#15: Add `storage.batch.Batch`

tseaver merged commit f0a1157 into googleapis:master Feb 26, 2015

tseaver deleted the 15-add_storage_batch branch February 26, 2015 19:10

This was referenced Feb 26, 2015

Bring apitools Python3 support. google/apitools#1

Closed

Bring protorpc Python3 support. google/protorpc#3

Closed

dhermes mentioned this pull request Mar 12, 2015

Upgrading version to 0.4.3. #723

Merged

craigcitro mentioned this pull request Apr 7, 2015

DISCUSSION: Switch to apitools as a dependency #755

Closed

#15: Add storage.batch.Batch #654

#15: Add storage.batch.Batch #654

Conversation

tseaver commented Feb 16, 2015

coveralls commented Feb 16, 2015

tseaver commented Feb 16, 2015

dhermes commented Feb 17, 2015

tseaver commented Feb 17, 2015

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Feb 18, 2015

coveralls commented Feb 18, 2015

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

coveralls commented Feb 19, 2015

tseaver commented Feb 25, 2015

This comment was marked as spam.

This comment was marked as spam.

craigcitro commented Feb 26, 2015

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

dhermes commented Feb 26, 2015

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

craigcitro commented Feb 26, 2015

craigcitro commented Feb 26, 2015

dhermes commented Feb 26, 2015

coveralls commented Feb 26, 2015

dhermes commented Feb 26, 2015

dhermes commented Feb 26, 2015

craigcitro commented Feb 26, 2015

coveralls commented Feb 26, 2015

dhermes commented Apr 9, 2015

#15: Add `storage.batch.Batch` #654

#15: Add `storage.batch.Batch` #654