Transactions - Bug when using transaction APIs explicitly (not through the context manager)

[flea89]

TL;DR : when using trx.begin() and trx.commit() the transactions don't behave as expected.

Please refer to this merge request to see the bug highlighted in the test.

From a quick look at the code, the problem seems to be the transaction is not added to the client._batch_stack when the APIs are called explicitly. _push_batch an _pop_batch are called only through the context manager __enter__ and __exit__.

Because of this client.current_batch and current_transaction

Environment details

• OS type and version: MacOs Ventura 13.1
• Python version: 3.8.16
• pip version: 22.0.4
• google-cloud-datastore version: 2.15.2

Steps to reproduce

1. Create a transaction client.transaction()
2. Start a transaction txn.begin()
3. Get an object entity_in_txn = client.get(key)
4. Update and put the same entity outside of the transaction.
5. Update the entity and put the entity within the transaction

6. Commit the transaction txn.commit()

   The transaction should raise a Conflit but it doesn't. (When doing the same with the context manager the Exeption is raised, see here

Code example

See here

Stack trace

# example
N/A

[daniel-sanche]

I believe currently, transactions are designed to only work as part of a context manager

That said, I think it would be a good idea to re-structure it so that it can be used without a context manager, as in your example

[artoale]

I believe currently, transactions are designed to only work as part of a context manager

That said, I think it would be a good idea to re-structure it so that it can be used without a context manager, as in your example

The official documentation states:

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.

I'd suggest either changing the behaviour (which should be a matter of moving the logic from __enter__ to begin or update the docs to reflect that the method is unsupported. Current behaviour fails in fairly hard-to-identify ways

[daniel-sanche]

Ok, good catch. I'll mark this as a bug again then. We should make sure to add tests for this use case as well