Visitar URL original
Change LocalStack of batches from global to client-local in storage by dhermes · Pull Request #990 · googleapis/google-cloud-python · GitHub
Skip to content

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

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

Merged
dhermes merged 3 commits into from
Jul 21, 2015
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb6b7c5
Update storage.Client to hold stack of batch objects.
dhermes Jul 14, 2015
e221edb
Making connection a property on storage client.
dhermes Jul 14, 2015
7112533
Adding comment about using _connection in storage.blob.
dhermes Jul 21, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions gcloud/datastore/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def _push_batch(self, batch):

:type batch: :class:`gcloud.datastore.batch.Batch`, or an object
implementing its API.
:param batch: newly-active batch/batch/transaction.
:param batch: newly-active batch/transaction.
"""
self._batch_stack.push(batch)

Expand All @@ -211,7 +211,7 @@ def current_batch(self):

:rtype: :class:`gcloud.datastore.batch.Batch`, or an object
implementing its API, or ``NoneType`` (if no batch is active).
:returns: The batch/transaction at the toop of the batch stack.
:returns: The batch/transaction at the top of the batch stack.
"""
return self._batch_stack.top

Expand All @@ -222,7 +222,7 @@ def current_transaction(self):
:rtype: :class:`gcloud.datastore.transaction.Transaction`, or an object
implementing its API, or ``NoneType`` (if no transaction is
active).
:returns: The transaction at the toop of the batch stack.
:returns: The transaction at the top of the batch stack.
"""
transaction = self.current_batch
if isinstance(transaction, Transaction):
Expand Down
18 changes: 8 additions & 10 deletions gcloud/storage/batch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,10 @@

import six

from gcloud._helpers import _LocalStack
from gcloud.exceptions import make_exception
from gcloud.storage.connection import Connection


_BATCHES = _LocalStack()


class MIMEApplicationHTTP(MIMEApplication):
"""MIME type for ``application/http``.

Expand Down Expand Up @@ -244,27 +240,29 @@ def finish(self):

url = '%s/batch' % self.API_BASE_URL

response, content = self._client.connection._make_request(
# Use the private ``_connection`` rather than the public
# ``.connection``, since the public connection may be this
# current batch.
response, content = self._client._connection._make_request(
'POST', url, data=body, headers=headers)
responses = list(_unpack_batch_response(response, content))
self._finish_futures(responses)
return responses

@staticmethod
def current():
def current(self):
"""Return the topmost batch, or None."""
return _BATCHES.top
return self._client.current_batch

def __enter__(self):
_BATCHES.push(self)
self._client._push_batch(self)
return self

def __exit__(self, exc_type, exc_val, exc_tb):
try:
if exc_type is None:
self.finish()
finally:
_BATCHES.pop()
self._client._pop_batch()


def _generate_faux_mime_message(parser, response, content):
Expand Down
18 changes: 15 additions & 3 deletions gcloud/storage/blob.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def generate_signed_url(self, expiration, method='GET',

if credentials is None:
client = self._require_client(client)
credentials = client.connection.credentials
credentials = client._connection.credentials

return generate_signed_url(
credentials, resource=resource,
Expand Down Expand Up @@ -291,7 +291,13 @@ 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)
# Use the private ``_connection`` rather than the public
# ``.connection``, since the public connection may be a batch. A
# batch wraps a client's connection, but does not store the `http`
# object. The rest (API_BASE_URL and build_api_url) are also defined
# on the Batch class, but we just use the wrapped connection since
# it has all three (http, API_BASE_URL and build_api_url).
download.InitializeDownload(request, client._connection.http)

This comment was marked as spam.

Sign in to view


# Should we be passing callbacks through from caller? We can't
# pass them as None, because apitools wants to print to the console
Expand Down Expand Up @@ -379,7 +385,13 @@ def upload_from_file(self, file_obj, rewind=False, size=None,
determined
"""
client = self._require_client(client)
connection = client.connection
# Use the private ``_connection`` rather than the public
# ``.connection``, since the public connection may be a batch. A
# batch wraps a client's connection, but does not store the `http`
# object. The rest (API_BASE_URL and build_api_url) are also defined
# on the Batch class, but we just use the wrapped connection since
# it has all three (http, API_BASE_URL and build_api_url).
connection = client._connection
content_type = (content_type or self._properties.get('contentType') or
'application/octet-stream')

Expand Down
68 changes: 68 additions & 0 deletions gcloud/storage/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""gcloud storage client for interacting with API."""


from gcloud._helpers import _LocalStack
from gcloud.client import JSONClient
from gcloud.exceptions import NotFound
from gcloud.iterator import Iterator
Expand Down Expand Up @@ -45,6 +46,73 @@ class Client(JSONClient):

_connection_class = Connection

def __init__(self, project=None, credentials=None, http=None):
self._connection = None
super(Client, self).__init__(project=project, credentials=credentials,
http=http)
self._batch_stack = _LocalStack()

@property
def connection(self):
"""Get connection or batch on the client.

:rtype: :class:`gcloud.storage.connection.Connection`
:returns: The connection set on the client, or the batch
if one is set.
"""
if self.current_batch is not None:
return self.current_batch
else:
return self._connection

@connection.setter
def connection(self, value):
"""Set connection on the client.

Intended to be used by constructor (since the base class calls)
self.connection = connection
Will raise if the connection is set more than once.

:type value: :class:`gcloud.storage.connection.Connection`
:param value: The connection set on the client.

:raises: :class:`ValueError` if connection has already been set.
"""
if self._connection is not None:
raise ValueError('Connection already set on client')
self._connection = value

def _push_batch(self, batch):
"""Push a batch onto our stack.

"Protected", intended for use by batch context mgrs.

:type batch: :class:`gcloud.storage.batch.Batch`
:param batch: newly-active batch
"""
self._batch_stack.push(batch)

def _pop_batch(self):
"""Pop a batch from our stack.

"Protected", intended for use by batch context mgrs.

:raises: IndexError if the stack is empty.
:rtype: :class:`gcloud.storage.batch.Batch`
:returns: the top-most batch/transaction, after removing it.
"""
return self._batch_stack.pop()

@property
def current_batch(self):
"""Currently-active batch.

:rtype: :class:`gcloud.storage.batch.Batch` or ``NoneType`` (if
no batch is active).
:returns: The batch at the top of the batch stack.
"""
return self._batch_stack.top

def get_bucket(self, bucket_name):
"""Get a bucket by name.

Expand Down
68 changes: 46 additions & 22 deletions gcloud/storage/test_batch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,21 @@ def test_ctor_w_explicit_connection(self):
self.assertEqual(len(batch._requests), 0)
self.assertEqual(len(batch._target_objects), 0)

def test_current(self):
from gcloud.storage.client import Client
project = 'PROJECT'
credentials = _Credentials()
client = Client(project=project, credentials=credentials)

This comment was marked as spam.

Sign in to view

batch1 = self._makeOne(client)
self.assertTrue(batch1.current() is None)

client._push_batch(batch1)
self.assertTrue(batch1.current() is batch1)

batch2 = self._makeOne(client)
client._push_batch(batch2)
self.assertTrue(batch1.current() is batch2)

def test__make_request_GET_normal(self):
from gcloud.storage.batch import _FutureDict
URL = 'http://example.com/api'
Expand Down Expand Up @@ -354,39 +369,31 @@ def test_finish_nonempty_non_multipart_response(self):
batch._requests.append(('DELETE', URL, {}, None))
self.assertRaises(ValueError, batch.finish)

def test_current(self):
from gcloud.storage.batch import _BATCHES
klass = self._getTargetClass()
batch_top = object()
self.assertEqual(list(_BATCHES), [])
_BATCHES.push(batch_top)
self.assertTrue(klass.current() is batch_top)
_BATCHES.pop()
self.assertEqual(list(_BATCHES), [])

def test_as_context_mgr_wo_error(self):
from gcloud.storage.batch import _BATCHES
from gcloud.storage.client import Client
URL = 'http://example.com/api'
expected = _Response()
expected['content-type'] = 'multipart/mixed; boundary="DEADBEEF="'
http = _HTTP((expected, _THREE_PART_MIME_RESPONSE))
connection = _Connection(http=http)
client = _Client(connection)
project = 'PROJECT'
credentials = _Credentials()
client = Client(project=project, credentials=credentials)
client._connection._http = http

self.assertEqual(list(_BATCHES), [])
self.assertEqual(list(client._batch_stack), [])

target1 = _MockObject()
target2 = _MockObject()
target3 = _MockObject()
with self._makeOne(client) as batch:
self.assertEqual(list(_BATCHES), [batch])
self.assertEqual(list(client._batch_stack), [batch])
batch._make_request('POST', URL, {'foo': 1, 'bar': 2},
target_object=target1)
batch._make_request('PATCH', URL, {'bar': 3},
target_object=target2)
batch._make_request('DELETE', URL, target_object=target3)

self.assertEqual(list(_BATCHES), [])
self.assertEqual(list(client._batch_stack), [])
self.assertEqual(len(batch._requests), 3)
self.assertEqual(batch._requests[0][0], 'POST')
self.assertEqual(batch._requests[1][0], 'PATCH')
Expand All @@ -400,19 +407,23 @@ def test_as_context_mgr_wo_error(self):

def test_as_context_mgr_w_error(self):
from gcloud.storage.batch import _FutureDict
from gcloud.storage.batch import _BATCHES
from gcloud.storage.client import Client
URL = 'http://example.com/api'
http = _HTTP()
connection = _Connection(http=http)
project = 'PROJECT'
credentials = _Credentials()
client = Client(project=project, credentials=credentials)
client._connection = connection

self.assertEqual(list(_BATCHES), [])
self.assertEqual(list(client._batch_stack), [])

target1 = _MockObject()
target2 = _MockObject()
target3 = _MockObject()
try:
with self._makeOne(connection) as batch:
self.assertEqual(list(_BATCHES), [batch])
with self._makeOne(client) as batch:
self.assertEqual(list(client._batch_stack), [batch])
batch._make_request('POST', URL, {'foo': 1, 'bar': 2},
target_object=target1)
batch._make_request('PATCH', URL, {'bar': 3},
Expand All @@ -422,7 +433,7 @@ def test_as_context_mgr_w_error(self):
except ValueError:
pass

self.assertEqual(list(_BATCHES), [])
self.assertEqual(list(client._batch_stack), [])
self.assertEqual(len(http._requests), 0)
self.assertEqual(len(batch._requests), 3)
self.assertEqual(batch._target_objects, [target1, target2, target3])
Expand Down Expand Up @@ -597,4 +608,17 @@ class _MockObject(object):
class _Client(object):

def __init__(self, connection):
self.connection = connection
self._connection = connection


class _Credentials(object):

_scopes = None

@staticmethod
def create_scoped_required():
return True

def create_scoped(self, scope):
self._scopes = scope
return self
6 changes: 5 additions & 1 deletion gcloud/storage/test_blob.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,4 +1082,8 @@ def __call__(self, *args, **kwargs):
class _Client(object):

def __init__(self, connection):
self.connection = connection
self._connection = connection

@property
def connection(self):
return self._connection
55 changes: 55 additions & 0 deletions gcloud/storage/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,61 @@ def test_ctor_connection_type(self):
client = self._makeOne(project=PROJECT, credentials=CREDENTIALS)
self.assertTrue(isinstance(client.connection, Connection))
self.assertTrue(client.connection.credentials is CREDENTIALS)
self.assertTrue(client.current_batch is None)
self.assertEqual(list(client._batch_stack), [])

def test__push_batch_and__pop_batch(self):
from gcloud.storage.batch import Batch

PROJECT = object()
CREDENTIALS = _Credentials()

client = self._makeOne(project=PROJECT, credentials=CREDENTIALS)
batch1 = Batch(client)
batch2 = Batch(client)
client._push_batch(batch1)
self.assertEqual(list(client._batch_stack), [batch1])
self.assertTrue(client.current_batch is batch1)
client._push_batch(batch2)
self.assertTrue(client.current_batch is batch2)
# list(_LocalStack) returns in reverse order.
self.assertEqual(list(client._batch_stack), [batch2, batch1])
self.assertTrue(client._pop_batch() is batch2)
self.assertEqual(list(client._batch_stack), [batch1])
self.assertTrue(client._pop_batch() is batch1)
self.assertEqual(list(client._batch_stack), [])

def test_connection_setter(self):
PROJECT = object()
CREDENTIALS = _Credentials()
client = self._makeOne(project=PROJECT, credentials=CREDENTIALS)
client._connection = None # Unset the value from the constructor
client.connection = connection = object()
self.assertTrue(client._connection is connection)

def test_connection_setter_when_set(self):
PROJECT = object()
CREDENTIALS = _Credentials()
client = self._makeOne(project=PROJECT, credentials=CREDENTIALS)
self.assertRaises(ValueError, setattr, client, 'connection', None)

def test_connection_getter_no_batch(self):
PROJECT = object()
CREDENTIALS = _Credentials()
client = self._makeOne(project=PROJECT, credentials=CREDENTIALS)
self.assertTrue(client.connection is client._connection)
self.assertTrue(client.current_batch is None)

def test_connection_getter_with_batch(self):
from gcloud.storage.batch import Batch
PROJECT = object()
CREDENTIALS = _Credentials()
client = self._makeOne(project=PROJECT, credentials=CREDENTIALS)
batch = Batch(client)
client._push_batch(batch)
self.assertTrue(client.connection is not client._connection)
self.assertTrue(client.connection is batch)
self.assertTrue(client.current_batch is batch)

def test_get_bucket_miss(self):
from gcloud.exceptions import NotFound
Expand Down
Loading