googleapis / python-datastore

Apache License 2.0
80 stars 44 forks source link

feat: add new_transaction support #499

Closed daniel-sanche closed 8 months ago

daniel-sanche commented 1 year ago

This PR modifies the Transaction class to add a new begin_later argument.

When True, the transaction will not initialize itself at the start of the context manager __enter__ block. Instead, it will only begin at the first rpc. If the first rpc is a read call, this allows us to create the transaction at the same time as the call, saving a network call.

This new behaviour is enabled by default, and is aligned with changes in other languages


For reviewers

In Python, a transaction is created in a context manager, and typically interacted with through the client (which passes through to the Transaction object when needed):

with client.transaction(begin_later=True):
  client.query(query)
  client.get(entity)
  client.put(entity)
  client.delete(entity)

if beginlater is False, it will call begin automatically when entering that context manager block (using __enter_\ method), retrieving a transaction_id from the server. Otherwise, begin is deferred until a get/query/commit call forces the transaction to talk to the backend

States

The state machine looks like this. Green lines denote transitions that are only valid when begin_later=True orange lines denote transitions that are valid when begin_later=False

Untitled Diagram drawio (2)

cindy-peng commented 9 months ago

This PR modifies the Transaction class to add a new begin_later argument.

When True, the transaction will not initialize itself at the start of the context manager __enter__ block. Instead, it will only begin at the first rpc. If the first rpc is a read call, this allows us to create the transaction at the same time as the call, saving a network call.

This new behaviour is enabled by default, and is aligned with changes in other languages

Hi Daniel, it looks like begin_later is by default set to False so by default disabled? Do we want this behavior to be enabled by default?

daniel-sanche commented 9 months ago

Yes, I believe it's intended to be off by default. Having it on or off may be more efficient depending on the circumstances, so we opted to keep it off by default (correct me if I'm misremembering @danieljbruce @bhshkh)