From 4cffec351e903ad65be7a8dc1599b9055be0e6e0 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 10 Dec 2024 16:27:09 -0800 Subject: [PATCH 1/4] added test for no context manager --- tests/system/test_transaction.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/system/test_transaction.py b/tests/system/test_transaction.py index 2f7a6897..2d8f4676 100644 --- a/tests/system/test_transaction.py +++ b/tests/system/test_transaction.py @@ -160,3 +160,34 @@ def test_failure_with_contention(datastore_client, entities_to_delete, database_ # transaction. entity_in_txn[contention_prop_name] = "inside" txn.put(entity_in_txn) + + +@pytest.mark.parametrize("database_id", [None, _helpers.TEST_DATABASE], indirect=True) +def test_failure_with_contention_no_context_manager(datastore_client, entities_to_delete): + contention_prop_name = "baz" + local_client = _helpers.clone_client(datastore_client) + + # Insert an entity which will be retrieved in a transaction + # and updated outside it with a contentious value. + key = local_client.key("BreakTxn", 1234) + orig_entity = datastore.Entity(key=key) + orig_entity["foo"] = "bar" + local_client.put(orig_entity) + + entities_to_delete.append(orig_entity) + + with pytest.raises(Conflict): + txn = local_client.transaction() + txn.begin() + + entity_in_txn = local_client.get(key) + + # Update the original entity outside the transaction. + orig_entity[contention_prop_name] = "outside" + datastore_client.put(orig_entity) + + # Try to update the entity which we already updated outside the + # transaction. + entity_in_txn[contention_prop_name] = "inside" + txn.put(entity_in_txn) + txn.commit() From be49dbc8b0fdf6c36add703c552cab61eb2c711d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 13 Dec 2024 16:24:03 -0800 Subject: [PATCH 2/4] updated tests --- tests/system/test_transaction.py | 49 ++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/tests/system/test_transaction.py b/tests/system/test_transaction.py index 2d8f4676..4c077ca2 100644 --- a/tests/system/test_transaction.py +++ b/tests/system/test_transaction.py @@ -148,46 +148,51 @@ def test_failure_with_contention(datastore_client, entities_to_delete, database_ entities_to_delete.append(orig_entity) - with pytest.raises(Conflict): - with local_client.transaction() as txn: - entity_in_txn = local_client.get(key) + with local_client.transaction() as txn: + entity_in_txn = local_client.get(key) - # Update the original entity outside the transaction. - orig_entity[contention_prop_name] = "outside" + # Update the original entity outside the transaction. + orig_entity[contention_prop_name] = "outside" + with pytest.raises(Conflict): datastore_client.put(orig_entity) - # Try to update the entity which we already updated outside the - # transaction. - entity_in_txn[contention_prop_name] = "inside" - txn.put(entity_in_txn) + # Try to update the entity which we already updated outside the + # transaction. + entity_in_txn[contention_prop_name] = "inside" + txn.put(entity_in_txn) + # now that transaction is complete, should be able to update outside + datastore_client.put(orig_entity) @pytest.mark.parametrize("database_id", [None, _helpers.TEST_DATABASE], indirect=True) -def test_failure_with_contention_no_context_manager(datastore_client, entities_to_delete): +def test_failure_with_contention_no_context_manager(datastore_client, entities_to_delete, database_id): contention_prop_name = "baz" local_client = _helpers.clone_client(datastore_client) # Insert an entity which will be retrieved in a transaction # and updated outside it with a contentious value. - key = local_client.key("BreakTxn", 1234) + key = local_client.key("BreakTxnCM3", 1234) orig_entity = datastore.Entity(key=key) orig_entity["foo"] = "bar" local_client.put(orig_entity) entities_to_delete.append(orig_entity) - with pytest.raises(Conflict): - txn = local_client.transaction() - txn.begin() + txn = local_client.transaction() + txn.begin() - entity_in_txn = local_client.get(key) + entity_in_txn = local_client.get(key, transaction=txn) - # Update the original entity outside the transaction. - orig_entity[contention_prop_name] = "outside" + # Update the original entity outside the transaction. + # should fail due to contention + orig_entity[contention_prop_name] = "outside" + with pytest.raises(Conflict): datastore_client.put(orig_entity) - # Try to update the entity which we already updated outside the - # transaction. - entity_in_txn[contention_prop_name] = "inside" - txn.put(entity_in_txn) - txn.commit() + # Try to update the entity inside the transaction + entity_in_txn[contention_prop_name] = "inside" + txn.put(entity_in_txn) + txn.commit() + + # now that transaction is complete, should be able to update outside + datastore_client.put(orig_entity) \ No newline at end of file From d2f2a0352943ef613d7593e69146acb5d38d85a6 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 13 Dec 2024 16:39:20 -0800 Subject: [PATCH 3/4] update docstrings --- google/cloud/datastore/batch.py | 8 +++++++- google/cloud/datastore/client.py | 5 +++-- google/cloud/datastore/transaction.py | 4 +++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/google/cloud/datastore/batch.py b/google/cloud/datastore/batch.py index 69100bc6..f54ae367 100644 --- a/google/cloud/datastore/batch.py +++ b/google/cloud/datastore/batch.py @@ -281,7 +281,9 @@ def begin(self): This method is called automatically when entering a with statement, however it can be called explicitly if you don't want - to use a context manager. + to use a context manager. If used outside a context manager, + `client.get` calls targeting the batch and commit/rollback calls + will need to be managed explicitly as well Overridden by :class:`google.cloud.datastore.transaction.Transaction`. @@ -365,6 +367,10 @@ def rollback(self): Marks the batch as aborted (can't be used again). + This is called automatically upon exiting a with statement, + however it can be called explicitly if you don't want to use a + context manager. + Overridden by :class:`google.cloud.datastore.transaction.Transaction`. :raises: :class:`~exceptions.ValueError` if the batch is not diff --git a/google/cloud/datastore/client.py b/google/cloud/datastore/client.py index ca3d4e0c..06422996 100644 --- a/google/cloud/datastore/client.py +++ b/google/cloud/datastore/client.py @@ -407,7 +407,7 @@ def _pop_batch(self): @property def current_batch(self): - """Currently-active batch. + """Currently-active batch, if within the scope of a Batch context manager. :rtype: :class:`google.cloud.datastore.batch.Batch`, or an object implementing its API, or ``NoneType`` (if no batch is active). @@ -417,7 +417,8 @@ def current_batch(self): @property def current_transaction(self): - """Currently-active transaction. + """Currently-active transaction, if within the scope of a Transaction + context manager. :rtype: :class:`google.cloud.datastore.transaction.Transaction`, or an object implementing its API, or ``NoneType`` (if no transaction diff --git a/google/cloud/datastore/transaction.py b/google/cloud/datastore/transaction.py index 52c17ce2..f8d8eeed 100644 --- a/google/cloud/datastore/transaction.py +++ b/google/cloud/datastore/transaction.py @@ -212,7 +212,9 @@ def begin(self, retry=None, timeout=None): This method is called automatically when entering a with statement, however it can be called explicitly if you don't want - to use a context manager. + to use a context manager. If used outside a context manager, + `client.get` calls targeting the transaction and commit/rollback calls + will need to be managed explicitly as well. :type retry: :class:`google.api_core.retry.Retry` :param retry: From 1868eea0f1a769a9d24c5d685f5903d99d030cac Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Sat, 14 Dec 2024 00:50:36 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/system/test_transaction.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/system/test_transaction.py b/tests/system/test_transaction.py index 4c077ca2..04d70c9f 100644 --- a/tests/system/test_transaction.py +++ b/tests/system/test_transaction.py @@ -165,7 +165,9 @@ def test_failure_with_contention(datastore_client, entities_to_delete, database_ @pytest.mark.parametrize("database_id", [None, _helpers.TEST_DATABASE], indirect=True) -def test_failure_with_contention_no_context_manager(datastore_client, entities_to_delete, database_id): +def test_failure_with_contention_no_context_manager( + datastore_client, entities_to_delete, database_id +): contention_prop_name = "baz" local_client = _helpers.clone_client(datastore_client) @@ -195,4 +197,4 @@ def test_failure_with_contention_no_context_manager(datastore_client, entities_t txn.commit() # now that transaction is complete, should be able to update outside - datastore_client.put(orig_entity) \ No newline at end of file + datastore_client.put(orig_entity)