googleapis / python-firestore

Apache License 2.0
218 stars 75 forks source link

Context manager for `google.cloud.firestore_v1.Client` #653

Open and3rson opened 2 years ago

and3rson commented 2 years ago

Is your feature request related to a problem? Please describe. Currently, transactions are only supported via @firestore.transactional, and the motivation is clear: to allow firestore.Client to re-run the entire transaction when contention occurs.

However, a widely used (and very much Pythonic) pattern for working with transactional entities is by using context managers: DB clients, files, streams - they all implement context manager protocol.

firestore.Client, on the other hand, requires the developer to create boilerplate functions for every transaction, even in cases when retries are unwanted.

Furthermore, google.cloud.firestore_v1.services.firestore.FirestoreClient already implements the context manager protocol.

Describe the solution you'd like I'd like firestore.Client to implement a context manager as such:

client = firestore.Client()
ref = client.collection('docs').document('doc1')

with client.transaction() as transaction:
    # Transaction API remains unchanged
    snapshot = ref.get(transaction)
    transaction.update(snapshot, {'value': snapshot.get('value') + 1})

# Equivalent with current API:
# @firestore.transactional
# def update_in_transaction(transaction):
#     snapshot = ref.get(transaction)
#     transaction.update(snapshot, {'value': snapshot.get('value') + 1})
# update_in_transaction(client.transaction(max_attempts=1))

# Async version
client = firestore.AsyncClient()
ref = client.collection('docs').document('doc1')

async with client.transaction() as transaction:
    # Transaction API remains unchanged
    snapshot = await ref.get(transaction)
    transaction.update(snapshot, {'value': snapshot.get('value') + 1})

Of course, contrary to the current suggested usage with @firestore.transactional, it's clear that the context manager approach will not support retries, since the code is only evaluated once. But this is a totally expected behavior that can be documented in the class docstring itself for more transparency.

Describe alternatives you've considered

daniel-sanche commented 1 year ago

Transaction is a subclass of batch.WriteBatch, which implements the context manager interface, so I believe this functionality should already exist. Let me know if there's something it's missing though

oakmegaeddie commented 7 months ago

I tried following code, but got error: ValueError: Transaction not in progress, cannot be used in API requests.

db = firestore.Client(project="project_1", database="db_1")
doc_ref = db.collection('docs').document('doc1')

with db.transaction() as t:
    snapshot = doc_ref.get(transaction=t)
    t.update(doc_ref, {"a": 2})
daniel-sanche commented 7 months ago

Sorry, you are right, firestore's transactions don't work well with context managers. I definitely see this as something that should be addressed. It's especially confusing since it can be used as a context manager, but the transaction isn't initiated when used that way

We're currently re-evaluating how retries are handled in Transactions (and other places). I can try to address this gap as a part of that project

oakmegaeddie commented 2 months ago

Hi @daniel-sanche Is there any update? Thx.