- Notifications
You must be signed in to change notification settings - Fork 1
[GCP-5388] Storage: Blob.exists() does not work within Batch context#1
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
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
522ff0b42ca94410fbf748d43ebcf1f01a7File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -56,6 +56,7 @@ | ||
| from google.cloud.storage._helpers import _scalar_property | ||
| from google.cloud.storage._signing import generate_signed_url_v2 | ||
| from google.cloud.storage._signing import generate_signed_url_v4 | ||
| from google.cloud.storage.batch import _FutureDict | ||
| from google.cloud.storage.acl import ACL | ||
| from google.cloud.storage.acl import ObjectACL | ||
| @@ -466,18 +467,24 @@ def exists(self, client=None): | ||
| query_params["fields"] = "name" | ||
| try: | ||
| # We intentionally pass `_target_object=None` since fields=name | ||
| # would limit the local properties. | ||
| if client.current_batch is not None: | ||
| target = FutureBoolResult() | ||
| result = target | ||
| else: | ||
| # We intentionally pass `_target_object=None` since fields=name | ||
| # would limit the local properties. | ||
| target, result = None, True | ||
| client._connection.api_request( | ||
| method="GET", | ||
| path=self.path, | ||
| query_params=query_params, | ||
| _target_object=None, | ||
| _target_object=target, | ||
| ) | ||
| # NOTE: This will not fail immediately in a batch. However, when | ||
| # Batch.finish() is called, the resulting `NotFound` will be | ||
| # raised. | ||
| return True | ||
| return result | ||
| except NotFound: | ||
| return False | ||
| @@ -2057,3 +2064,32 @@ def _add_query_parameters(base_url, name_value_pairs): | ||
| query = parse_qsl(query) | ||
| query.extend(name_value_pairs) | ||
| return urlunsplit((scheme, netloc, path, urlencode(query), frag)) | ||
| class FutureBoolResult(object): | ||
| """Represents the result of future api request. | ||
| This can be used as a target object for the future request | ||
| (which is part of batch mechanism) to get result of this request. | ||
| """ | ||
| _properties = {} | ||
| @property | ||
| def _is_successfull(self): | ||
| """Checks if binded future request have been completed without any exception.""" | ||
| return not isinstance(self._properties, _FutureDict) | ||
IlyaFaer marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| def __bool__(self): | ||
| return self._is_successfull | ||
| def __nonzero__(self): | ||
| return self._is_successfull | ||
| def __repr__(self): | ||
| return str(self._is_successfull) | ||
| def __eq__(self, other): | ||
| return self._is_successfull == other | ||
| def __ne__(self, other): | ||
| return self._is_successfull != other | ||
IlyaFaer marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -134,7 +134,7 @@ def blacken(session): | ||
| "tests", | ||
| "docs", | ||
| ) | ||
| @nox.session(python="3.7") | ||
| def docs(session): | ||
| """Build the docs for this library.""" | ||
| @@ -154,4 +154,4 @@ def docs(session): | ||
| os.path.join("docs", "_build", "doctrees", ""), | ||
| os.path.join("docs", ""), | ||
| os.path.join("docs", "_build", "html", ""), | ||
| ) | ||
| ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any changes in nox file, if not, please remove this file from the the PR. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -20,13 +20,45 @@ | ||
| import os | ||
| import tempfile | ||
| import unittest | ||
| import requests | ||
| import google.cloud.storage.blob | ||
| import mock | ||
| import six | ||
| from six.moves import http_client | ||
| def _make_response(status_code=http_client.OK, content=b"", headers={}): | ||
| response = requests.Response() | ||
| response.status_code = status_code | ||
| response._content = content | ||
| response.headers = headers | ||
| response.request = requests.Request() | ||
| return response | ||
| def _make_connection(): | ||
| from google.cloud.storage._http import Connection | ||
| mock_conn = mock.create_autospec(Connection) | ||
| mock_conn.user_agent = "testing 1.2.3" | ||
| mock_conn._make_request.return_value = _make_response( | ||
| status_code=200, | ||
| headers={ | ||
| "content-type": "multipart/mixed; boundary=batch_pK7JBAk73-E=_AA5eFwv4m2Q=", | ||
| }, | ||
| content=b""" | ||
| --batch_pK7JBAk73-E=_AA5eFwv4m2Q= | ||
| HTTP/1.1 404 NotFound | ||
| --batch_pK7JBAk73-E=_AA5eFwv4m2Q= | ||
| HTTP/1.1 200 OK | ||
IlyaFaer marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| --batch_pK7JBAk73-E=_AA5eFwv4m2Q=--""" | ||
| ) | ||
| return mock_conn | ||
| def _make_credentials(): | ||
| import google.auth.credentials | ||
| @@ -582,6 +614,30 @@ def test_exists_miss(self): | ||
| }, | ||
| ) | ||
| def test_exists_miss_within_batch(self): | ||
| from google.cloud.storage.client import Client | ||
| from google.cloud.exceptions import NotFound | ||
| client = Client(credentials=_make_credentials()) | ||
| client._base_connection = _make_connection() | ||
| bucket = _Bucket(client) | ||
| blob1 = self._make_one("nonesuch1", bucket=bucket) | ||
| blob2 = self._make_one("nonesuch2", bucket=bucket) | ||
| try: | ||
| with client.batch(): | ||
| bool1 = blob1.exists() | ||
| bool2 = blob2.exists() | ||
| except NotFound: | ||
| pass | ||
IlyaFaer marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| self.assertFalse(bool1) | ||
| self.assertTrue(bool2) | ||
| self.assertEqual(str(bool1), 'False') | ||
| self.assertTrue(bool1 != True) # noqa: E712 | ||
| self.assertTrue(bool1 == False) # noqa: E712 | ||
| def test_exists_hit_w_user_project(self): | ||
| BLOB_NAME = "blob-name" | ||
| USER_PROJECT = "user-project-123" | ||
| @@ -3311,6 +3367,7 @@ def delete_blob(self, blob_name, client=None, generation=None): | ||
| class _Client(object): | ||
| def __init__(self, connection): | ||
| self._base_connection = connection | ||
| self.current_batch = None | ||
| @property | ||
| def _connection(self): | ||
Uh oh!
There was an error while loading. Please reload this page.