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

feat: add retry/timeout to manual surface #222

Merged
merged 41 commits into from Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
8d03921
feat: add retry/timeout to 'client.Client.get_all'
tseaver Oct 13, 2020
e7d2119
feat: add retry/timeout to 'client.Client.collections'
tseaver Oct 13, 2020
6bfe32f
feat: add retry/timeout to 'batch.Batch.commit'
tseaver Oct 13, 2020
6ba6d21
feat: add retry/timeout to 'document.DocumentReference.get'
tseaver Oct 13, 2020
e6ad4a1
feat: add retry/timeout to 'query.Query.get'
tseaver Oct 13, 2020
6e806b0
feat: add retry/timeout to 'query.CollectionGroup.get_partitions'
tseaver Oct 13, 2020
00736fe
feat: add retry/timeout to 'collection.CollectionReference.add'
tseaver Oct 13, 2020
2b80b91
feat: add retry/timeout to 'collection.CollectionReference.list_docum…
tseaver Oct 13, 2020
49d3b03
feat: add retry/timeout to 'collection.CollectionReference.get'
tseaver Oct 13, 2020
16a1e34
feat: add retry/timeout to 'collection.CollectionReference.stream'
tseaver Oct 13, 2020
a9547e6
feat: add retry/timeout to 'document.DocumentReference.collections'
tseaver Oct 13, 2020
d6df19c
feat: add retry/timeout to 'document.DocumentReference.delete'
tseaver Oct 13, 2020
6eae0e7
feat: add retry/timeout to 'document.DocumentReference.create'
tseaver Oct 13, 2020
559f2eb
feat: add retry/timeout to 'document.DocumentReference.set'
tseaver Oct 13, 2020
8038cce
feat: add retry/timeout to 'document.DocumentReference.update'
tseaver Oct 13, 2020
2d413df
feat: add retry/timeout to 'query.Query.stream'
tseaver Oct 13, 2020
e15b8f6
feat: add retry/timeout to 'transaction.Transaction.get_all'
tseaver Oct 13, 2020
9f5bbb4
feat: add retry/timeout to 'transaction.Transaction.get'
tseaver Oct 13, 2020
5a1ef50
feat: add retry/timeout to base class signatures
tseaver Oct 13, 2020
6dec6f3
fix: un-break docs build
tseaver Oct 13, 2020
812c41f
chore: factor out helper for computing retry / timeout kwargs
tseaver Oct 14, 2020
8c67138
chore: factor out common prep for 'collections'/'get_all' to base class
tseaver Oct 14, 2020
bad75e1
chore: factor out test helper for 'collections'
tseaver Oct 14, 2020
db01b59
feat: add retry/timeout to 'async_client.AsyncClient.{collections.get…
tseaver Oct 14, 2020
4090a00
chore: use factored-out helper to build retry/timeout kwargs
tseaver Oct 14, 2020
40fae96
chore: clean up tests for 'client.Client.get_all'
tseaver Oct 14, 2020
df615e4
chore: lint
tseaver Oct 14, 2020
a557a15
feat: add retry/timeout to 'async_batch.AsyncBatch.commit'
tseaver Oct 14, 2020
4e3be50
feat: add retry/timeout to 'async_document.AsyncDocument` methods
tseaver Oct 14, 2020
ec8002c
feat: add retry/timeout to 'async_query.Async{Query,CollectionGroup}'
tseaver Oct 14, 2020
c81cd8c
feat: add retry/timeout to 'async_collection.AsyncCollectionReference'
tseaver Oct 14, 2020
1acbde3
fix: typo
tseaver Oct 14, 2020
b998db2
chore: rename testcases/helper for clarity
tseaver Oct 14, 2020
46f27e6
feat: add retry/timeout to 'async_transaction.AsyncTransaction' methods
tseaver Oct 14, 2020
9b4707a
fix: typo
tseaver Oct 14, 2020
7a976a5
chore: appease pytype
tseaver Oct 14, 2020
4b1ec26
fix: actually test retry / timeout
tseaver Oct 14, 2020
c5e4056
fix: document system-specified defaults for 'retry' / 'timeout'
tseaver Oct 21, 2020
2360eac
fix: use gapic's 'DEFAULT' sentinel for 'retry'
tseaver Oct 21, 2020
f15a523
Merge branch 'master' into 221-retry-timeout
tseaver Oct 21, 2020
3feb63b
chore: lint
tseaver Oct 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 16 additions & 1 deletion google/cloud/firestore_v1/batch.py
Expand Up @@ -14,6 +14,7 @@

"""Helpers for batch requests to the Google Cloud Firestore API."""

from google.api_core import retry as retries # type: ignore

from google.cloud.firestore_v1.base_batch import BaseWriteBatch

Expand All @@ -33,22 +34,36 @@ class WriteBatch(BaseWriteBatch):
def __init__(self, client) -> None:
super(WriteBatch, self).__init__(client=client)

def commit(self) -> list:
def commit(self, retry: retries.Retry = None, timeout: float = None) -> list:
"""Commit the changes accumulated in this batch.

Args
retry (google.api_core.retry.Retry): Designation of what errors, if any,
should be retried.
timeout (float): The timeout for this request.

Returns:
List[:class:`google.cloud.proto.firestore.v1.write.WriteResult`, ...]:
The write results corresponding to the changes committed, returned
in the same order as the changes were applied to this batch. A
write result contains an ``update_time`` field.
"""
kwargs = {}

if retry is not None:
kwargs["retry"] = retry

if timeout is not None:
kwargs["timeout"] = timeout
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since factored out to use _helpers.make_retry_timeout_kwargs.


commit_response = self._client._firestore_api.commit(
request={
"database": self._client._database_string,
"writes": self._write_pbs,
"transaction": None,
},
metadata=self._client._rpc_metadata,
**kwargs,
)

self._write_pbs = []
Expand Down
30 changes: 25 additions & 5 deletions tests/unit/v1/test_batch.py
Expand Up @@ -35,7 +35,7 @@ def test_constructor(self):
self.assertIsNone(batch.write_results)
self.assertIsNone(batch.commit_time)

def test_commit(self):
def _commit_helper(self, retry=None, timeout=None):
from google.protobuf import timestamp_pb2
from google.cloud.firestore_v1.types import firestore
from google.cloud.firestore_v1.types import write
Expand All @@ -49,19 +49,27 @@ def test_commit(self):
)
firestore_api.commit.return_value = commit_response

kwargs = {}

if retry is not None:
kwargs["retry"] = retry

if timeout is not None:
kwargs["timeout"] = timeout
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, later commits update to use the helper here.


# Attach the fake GAPIC to a real client.
client = _make_client("grand")
client._firestore_api_internal = firestore_api

# Actually make a batch with some mutations and call commit().
batch = self._make_one(client)
document1 = client.document("a", "b")
batch.create(document1, {"ten": 10, "buck": u"ets"})
batch.create(document1, {"ten": 10, "buck": "ets"})
document2 = client.document("c", "d", "e", "f")
batch.delete(document2)
write_pbs = batch._write_pbs[::]

write_results = batch.commit()
write_results = batch.commit(**kwargs)
self.assertEqual(write_results, list(commit_response.write_results))
self.assertEqual(batch.write_results, write_results)
self.assertEqual(batch.commit_time.timestamp_pb(), timestamp)
Expand All @@ -76,8 +84,20 @@ def test_commit(self):
"transaction": None,
},
metadata=client._rpc_metadata,
**kwargs,
)

def test_commit(self):
self._commit_helper()

def test_commit_w_retry_timeout(self):
from google.api_core.retry import Retry

retry = Retry(predicate=object())
timeout = 123.0

self._commit_helper(retry=retry, timeout=timeout)

def test_as_context_mgr_wo_error(self):
from google.protobuf import timestamp_pb2
from google.cloud.firestore_v1.types import firestore
Expand All @@ -98,7 +118,7 @@ def test_as_context_mgr_wo_error(self):

with batch as ctx_mgr:
self.assertIs(ctx_mgr, batch)
ctx_mgr.create(document1, {"ten": 10, "buck": u"ets"})
ctx_mgr.create(document1, {"ten": 10, "buck": "ets"})
ctx_mgr.delete(document2)
write_pbs = batch._write_pbs[::]

Expand Down Expand Up @@ -127,7 +147,7 @@ def test_as_context_mgr_w_error(self):

with self.assertRaises(RuntimeError):
with batch as ctx_mgr:
ctx_mgr.create(document1, {"ten": 10, "buck": u"ets"})
ctx_mgr.create(document1, {"ten": 10, "buck": "ets"})
ctx_mgr.delete(document2)
raise RuntimeError("testing")

Expand Down