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: diff --git a/tests/system/test_transaction.py b/tests/system/test_transaction.py index 2f7a6897..04d70c9f 100644 --- a/tests/system/test_transaction.py +++ b/tests/system/test_transaction.py @@ -148,15 +148,53 @@ 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, 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("BreakTxnCM3", 1234) + orig_entity = datastore.Entity(key=key) + orig_entity["foo"] = "bar" + local_client.put(orig_entity) + + entities_to_delete.append(orig_entity) + + txn = local_client.transaction() + txn.begin() + + entity_in_txn = local_client.get(key, transaction=txn) + + # 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 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)