From 7e71f19c705bc9d32cd074d4d48cc2345ca36c3f Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 16 Jul 2020 17:49:06 -0700 Subject: [PATCH 1/4] chore: update minimum version of protoplus to ensure DatetimeWithNanoseconds availability --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index ef4c23071c..a565fb27af 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ "google-cloud-core >= 1.0.3, < 2.0dev", "pytz", "libcst >= 0.2.5", - "proto-plus >= 0.4.0", + "proto-plus >= 1.3.0", ] extras = {} From e4eaccf2990c574c700a337a22e8dcd47d4f05bd Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 16 Jul 2020 18:34:58 -0700 Subject: [PATCH 2/4] feat: Incorporate nanoseconds back into components, such as hashing --- google/cloud/firestore_v1/base_document.py | 6 +----- google/cloud/firestore_v1/watch.py | 2 -- tests/system/test_system.py | 1 - tests/unit/v1/test_async_batch.py | 6 ++---- tests/unit/v1/test_base_client.py | 7 +++---- tests/unit/v1/test_base_document.py | 7 +++++-- tests/unit/v1/test_batch.py | 6 ++---- 7 files changed, 13 insertions(+), 22 deletions(-) diff --git a/google/cloud/firestore_v1/base_document.py b/google/cloud/firestore_v1/base_document.py index a69470f80e..196e3cb5ec 100644 --- a/google/cloud/firestore_v1/base_document.py +++ b/google/cloud/firestore_v1/base_document.py @@ -243,12 +243,8 @@ def __eq__(self, other): return self._reference == other._reference and self._data == other._data def __hash__(self): - # TODO(microgen, https://github.com/googleapis/proto-plus-python/issues/38): - # maybe add datetime_with_nanos to protoplus, revisit - # seconds = self.update_time.seconds - # nanos = self.update_time.nanos seconds = int(self.update_time.timestamp()) - nanos = 0 + nanos = self.update_time.nanosecond return hash(self._reference) + hash(seconds) + hash(nanos) @property diff --git a/google/cloud/firestore_v1/watch.py b/google/cloud/firestore_v1/watch.py index 17c0926122..6aedc4ee18 100644 --- a/google/cloud/firestore_v1/watch.py +++ b/google/cloud/firestore_v1/watch.py @@ -569,8 +569,6 @@ def push(self, read_time, next_resume_token): keys, appliedChanges, read_time - # TODO(microgen): now a datetime - # datetime.datetime.fromtimestamp(read_time.seconds, pytz.utc), ) self.has_pushed = True diff --git a/tests/system/test_system.py b/tests/system/test_system.py index f0a807f6fe..4800014daf 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -340,7 +340,6 @@ def test_update_document(client, cleanup): document.update({"bad": "time-past"}, option=option4) # 6. Call ``update()`` with invalid (in future) "last timestamp" option. - # TODO(microgen): start using custom datetime with nanos in protoplus? timestamp_pb = _datetime_to_pb_timestamp(snapshot4.update_time) timestamp_pb.seconds += 3600 diff --git a/tests/unit/v1/test_async_batch.py b/tests/unit/v1/test_async_batch.py index acb977d869..118178542e 100644 --- a/tests/unit/v1/test_async_batch.py +++ b/tests/unit/v1/test_async_batch.py @@ -66,8 +66,7 @@ async def test_commit(self): write_results = await batch.commit() self.assertEqual(write_results, list(commit_response.write_results)) self.assertEqual(batch.write_results, write_results) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, []) @@ -107,8 +106,7 @@ async def test_as_context_mgr_wo_error(self): write_pbs = batch._write_pbs[::] self.assertEqual(batch.write_results, list(commit_response.write_results)) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, []) diff --git a/tests/unit/v1/test_base_client.py b/tests/unit/v1/test_base_client.py index cc3a7f06b1..631733e075 100644 --- a/tests/unit/v1/test_base_client.py +++ b/tests/unit/v1/test_base_client.py @@ -300,10 +300,9 @@ def test_found(self): self.assertIs(snapshot._reference, mock.sentinel.reference) self.assertEqual(snapshot._data, {"foo": 1.5, "bar": u"skillz"}) self.assertTrue(snapshot._exists) - # TODO(microgen): v2: datetime with nanos implementation needed. - # self.assertEqual(snapshot.read_time, read_time) - # self.assertEqual(snapshot.create_time, create_time) - # self.assertEqual(snapshot.update_time, update_time) + self.assertEqual(snapshot.read_time.timestamp_pb(), read_time) + self.assertEqual(snapshot.create_time.timestamp_pb(), create_time) + self.assertEqual(snapshot.update_time.timestamp_pb(), update_time) def test_missing(self): from google.cloud.firestore_v1.document import DocumentReference diff --git a/tests/unit/v1/test_base_document.py b/tests/unit/v1/test_base_document.py index c478ff9a66..a71b0d198a 100644 --- a/tests/unit/v1/test_base_document.py +++ b/tests/unit/v1/test_base_document.py @@ -17,6 +17,8 @@ import mock import datetime import pytz +from proto.datetime_helpers import DatetimeWithNanoseconds +from google.protobuf import timestamp_pb2 class TestBaseDocumentReference(unittest.TestCase): @@ -268,11 +270,12 @@ def test___hash__(self): client.__hash__.return_value = 234566789 reference = self._make_reference("hi", "bye", client=client) data = {"zoop": 83} - update_time = datetime.datetime.fromtimestamp(123456, pytz.utc) + update_time = DatetimeWithNanoseconds.from_timestamp_pb( + timestamp_pb2.Timestamp(seconds=123456, nanos=123456789)) snapshot = self._make_one( reference, data, True, None, mock.sentinel.create_time, update_time ) - self.assertEqual(hash(snapshot), hash(reference) + hash(123456) + hash(0)) + self.assertEqual(hash(snapshot), hash(reference) + hash(123456) + hash(123456789)) def test__client_property(self): reference = self._make_reference( diff --git a/tests/unit/v1/test_batch.py b/tests/unit/v1/test_batch.py index 5396540c6d..f21dee622a 100644 --- a/tests/unit/v1/test_batch.py +++ b/tests/unit/v1/test_batch.py @@ -64,8 +64,7 @@ def test_commit(self): write_results = batch.commit() self.assertEqual(write_results, list(commit_response.write_results)) self.assertEqual(batch.write_results, write_results) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, []) @@ -104,8 +103,7 @@ def test_as_context_mgr_wo_error(self): write_pbs = batch._write_pbs[::] self.assertEqual(batch.write_results, list(commit_response.write_results)) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, []) From 1bcb413d9e00848a5854097fcc07b2c0765f2e94 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 16 Jul 2020 18:44:30 -0700 Subject: [PATCH 3/4] blacken --- google/cloud/firestore_v1/watch.py | 6 +----- tests/unit/v1/test_base_document.py | 7 +++++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/google/cloud/firestore_v1/watch.py b/google/cloud/firestore_v1/watch.py index 6aedc4ee18..58c663abfc 100644 --- a/google/cloud/firestore_v1/watch.py +++ b/google/cloud/firestore_v1/watch.py @@ -565,11 +565,7 @@ def push(self, read_time, next_resume_token): key = functools.cmp_to_key(self._comparator) keys = sorted(updated_tree.keys(), key=key) - self._snapshot_callback( - keys, - appliedChanges, - read_time - ) + self._snapshot_callback(keys, appliedChanges, read_time) self.has_pushed = True self.doc_tree = updated_tree diff --git a/tests/unit/v1/test_base_document.py b/tests/unit/v1/test_base_document.py index a71b0d198a..8a5f42317e 100644 --- a/tests/unit/v1/test_base_document.py +++ b/tests/unit/v1/test_base_document.py @@ -271,11 +271,14 @@ def test___hash__(self): reference = self._make_reference("hi", "bye", client=client) data = {"zoop": 83} update_time = DatetimeWithNanoseconds.from_timestamp_pb( - timestamp_pb2.Timestamp(seconds=123456, nanos=123456789)) + timestamp_pb2.Timestamp(seconds=123456, nanos=123456789) + ) snapshot = self._make_one( reference, data, True, None, mock.sentinel.create_time, update_time ) - self.assertEqual(hash(snapshot), hash(reference) + hash(123456) + hash(123456789)) + self.assertEqual( + hash(snapshot), hash(reference) + hash(123456) + hash(123456789) + ) def test__client_property(self): reference = self._make_reference( From 874beaf939fbeffce00e1b075dad71ffe7f6c7fe Mon Sep 17 00:00:00 2001 From: Christopher Wilcox Date: Thu, 23 Jul 2020 13:31:27 -0700 Subject: [PATCH 4/4] remove unused imports --- tests/unit/v1/test_base_document.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/v1/test_base_document.py b/tests/unit/v1/test_base_document.py index 8a5f42317e..de4a9c8740 100644 --- a/tests/unit/v1/test_base_document.py +++ b/tests/unit/v1/test_base_document.py @@ -15,8 +15,6 @@ import unittest import mock -import datetime -import pytz from proto.datetime_helpers import DatetimeWithNanoseconds from google.protobuf import timestamp_pb2