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

Change LocalStack of batches from global to client-local in storage #990

Merged
merged 3 commits into from
Jul 21, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jul 14, 2015

Has #989 as diffbase

@dhermes dhermes added the api: storage Issues related to the Cloud Storage API. label Jul 14, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 14, 2015
@tseaver
Copy link
Contributor

tseaver commented Jul 20, 2015

@dhermes ping me after #989 lands (a rebase would help review).

@dhermes
Copy link
Contributor Author

dhermes commented Jul 20, 2015

Will do

@dhermes dhermes force-pushed the storage-move-stack-to-client branch from be75a35 to 8a64306 Compare July 20, 2015 21:23
@dhermes
Copy link
Contributor Author

dhermes commented Jul 20, 2015

@tseaver Rebased

@@ -291,7 +291,7 @@ def download_to_file(self, file_obj, client=None):
headers['Range'] = 'bytes=0-%d' % (self.chunk_size - 1,)
request = http_wrapper.Request(download_url, 'GET', headers)

download.InitializeDownload(request, client.connection.http)
download.InitializeDownload(request, client._connection.http)

This comment was marked as spam.

dhermes added 3 commits July 20, 2015 16:55
This is to allow seamless switching between the stored
connection and the active batch.

Also undoing hack in storage system test since batches are
supported correctly now on clients.
Doing this to address review commits. Also testing current()
in the presence of an empty batch stack.
@dhermes dhermes force-pushed the storage-move-stack-to-client branch from 8a64306 to 7112533 Compare July 21, 2015 00:05
@dhermes
Copy link
Contributor Author

dhermes commented Jul 21, 2015

@tseaver I addressed 2 of the 3 comments and responded to the third (slightly changed a bit).

@tseaver
Copy link
Contributor

tseaver commented Jul 21, 2015

I can let the "real client" bit go, I guess. LGTM

@dhermes
Copy link
Contributor Author

dhermes commented Jul 21, 2015

Thanks!

dhermes added a commit that referenced this pull request Jul 21, 2015
Change LocalStack of batches from global to client-local in storage
@dhermes dhermes merged commit 63af1e8 into googleapis:master Jul 21, 2015
@dhermes dhermes deleted the storage-move-stack-to-client branch July 21, 2015 02:32
@dhermes dhermes mentioned this pull request Jul 21, 2015
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. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants