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

Firestore: return non existent snapshot if document not found instead of raising NotFound exception #5007

Merged

Conversation

chemelnucfin
Copy link
Contributor

closes #4809

@chemelnucfin chemelnucfin added api: firestore Issues related to the Firestore API. type: process A process-related concern. May include testing, release, or the like. labels Mar 9, 2018
@chemelnucfin chemelnucfin self-assigned this Mar 9, 2018
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 9, 2018
@@ -420,17 +419,11 @@ def get(self, field_paths=None, transaction=None):
Returns:
~.firestore_v1beta1.document.DocumentSnapshot: A snapshot of
the current document.

This comment was marked as spam.

@@ -582,8 +582,7 @@ def _parse_batch_get(get_doc_response, reference_map, client):
a document factory.

Returns:
Optional[.DocumentSnapshot]: The retrieved snapshot. If the
snapshot is :data:`None`, that means the document is ``missing``.
Optional[.DocumentSnapshot]: The retrieved snapshot.

This comment was marked as spam.

@@ -420,17 +419,11 @@ def get(self, field_paths=None, transaction=None):
Returns:
~.firestore_v1beta1.document.DocumentSnapshot: A snapshot of
the current document.

Raises:
~google.cloud.exceptions.NotFound: If the document does not exist.
"""
snapshot_generator = self._client.get_all(
[self], field_paths=field_paths, transaction=transaction)
snapshot = _consume_single_get(snapshot_generator)

This comment was marked as spam.


# Create a minimal fake client with a dummy response.
response_iterator = iter([None])
read_time = 123
snapshot = DocumentSnapshot(None, None, False, read_time, None, None)

This comment was marked as spam.

@chemelnucfin
Copy link
Contributor Author

@tseaver addressed your comments. thanks. I kept a lot of the unittest to not compare to expected since that was the expected behavior.

@chemelnucfin chemelnucfin merged commit 77a1f68 into googleapis:master Mar 12, 2018
@chemelnucfin chemelnucfin deleted the firestore_to_dict_non_exists branch March 12, 2018 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the Firestore API. cla: yes This human has signed the Contributor License Agreement. type: process A process-related concern. May include testing, release, or the like.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firestore: Non-existing Documents
3 participants