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
22 changes: 22 additions & 0 deletions google/cloud/firestore_v1/client.py
Expand Up @@ -24,6 +24,8 @@
:class:`~google.cloud.firestore_v1.document.DocumentReference`
"""

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

from google.cloud.firestore_v1.base_client import (
BaseClient,
DEFAULT_DATABASE,
Expand Down Expand Up @@ -202,11 +204,25 @@ def document(self, *document_path: Tuple[str]) -> DocumentReference:
*self._document_path_helper(*document_path), client=self
)

@staticmethod
def _make_retry_timeout_kwargs(retry, timeout):
kwargs = {}

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

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

return kwargs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Later commits factor this method out into _helpers.make_retry_timeout_kwargs.


def get_all(
self,
references: list,
field_paths: Iterable[str] = None,
transaction: Transaction = None,
retry: retries.Retry = None,
timeout: float = None,
) -> Generator[Any, Any, None]:
"""Retrieve a batch of documents.

Expand Down Expand Up @@ -237,13 +253,18 @@ def get_all(
transaction (Optional[:class:`~google.cloud.firestore_v1.transaction.Transaction`]):
An existing transaction that these ``references`` will be
retrieved in.
retry (google.api_core.retry.Retry): Designation of what errors, if any,
should be retried.
timeout (float): The timeout for this request.

Yields:
.DocumentSnapshot: The next document snapshot that fulfills the
query, or :data:`None` if the document does not exist.
"""
document_paths, reference_map = _reference_info(references)
mask = _get_doc_mask(field_paths)
kwargs = self._make_retry_timeout_kwargs(retry, timeout)

response_iterator = self._firestore_api.batch_get_documents(
request={
"database": self._database_string,
Expand All @@ -252,6 +273,7 @@ def get_all(
"transaction": _helpers.get_transaction_id(transaction),
},
metadata=self._rpc_metadata,
**kwargs,
)

for get_doc_response in response_iterator:
Expand Down
49 changes: 49 additions & 0 deletions tests/unit/v1/test_client.py
Expand Up @@ -303,6 +303,55 @@ def test_get_all(self):
metadata=client._rpc_metadata,
)

def test_get_all_w_retry_timeout(self):
from google.api_core.retry import Retry
from google.cloud.firestore_v1.types import common
from google.cloud.firestore_v1.document import DocumentSnapshot

data1 = {"a": u"cheese"}
data2 = {"b": True, "c": 18}
retry = Retry(predicate=object())
timeout = 123.0
info = self._info_for_get_all(data1, data2)
client, document1, document2, response1, response2 = info

# Exercise the mocked ``batch_get_documents``.
field_paths = ["a", "b"]
snapshots = self._get_all_helper(
client,
[document1, document2],
[response1, response2],
field_paths=field_paths,
retry=retry,
timeout=timeout,
)
self.assertEqual(len(snapshots), 2)

snapshot1 = snapshots[0]
self.assertIsInstance(snapshot1, DocumentSnapshot)
self.assertIs(snapshot1._reference, document1)
self.assertEqual(snapshot1._data, data1)

snapshot2 = snapshots[1]
self.assertIsInstance(snapshot2, DocumentSnapshot)
self.assertIs(snapshot2._reference, document2)
self.assertEqual(snapshot2._data, data2)

# Verify the call to the mock.
doc_paths = [document1._document_path, document2._document_path]
mask = common.DocumentMask(field_paths=field_paths)
client._firestore_api.batch_get_documents.assert_called_once_with(
request={
"database": client._database_string,
"documents": doc_paths,
"mask": mask,
"transaction": None,
},
retry=retry,
timeout=timeout,
metadata=client._rpc_metadata,
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

40fae96 removes most of the redundancy in the tests for Client.get_all.


def test_get_all_with_transaction(self):
from google.cloud.firestore_v1.document import DocumentSnapshot

Expand Down