Skip to content

fix: update the async transactional types to not require extra awaits #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

fix: update the async transactional types to not require extra awaits #1045

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ca23405
fix: update the async transactional types to not require extra awaits
jekozyra May 10, 2025
c0e1446
add typing extensions
jekozyra May 22, 2025
a630701
python <3.10 compat
jekozyra May 22, 2025
576a8a8
use a protocol
jekozyra May 29, 2025
68118b7
Merge branch 'main' into jillian/update-types-for-async-transactions
jekozyra May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions google/cloud/firestore_v1/async_transaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,18 @@
"""Helpers for applying Google Cloud Firestore changes in a transaction."""
from __future__ import annotations

from typing import TYPE_CHECKING, Any, AsyncGenerator, Callable, Coroutine, Optional
from typing import (
TYPE_CHECKING,
Any,
AsyncGenerator,
Awaitable,
Callable,
Coroutine,
Optional,
TypeVar,
ParamSpec,
Concatenate,
)

from google.api_core import exceptions, gapic_v1
from google.api_core import retry_async as retries
Expand All @@ -41,6 +52,10 @@
from google.cloud.firestore_v1.query_profile import ExplainOptions


T = TypeVar("T")
P = ParamSpec("P")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like ParamSpec was added in python 3.10, and we need to support 3.8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added typing_extensions to support expanded type hinting for older python versions. Let me know if that works, otherwise I'll come up with something more hacky for the signatures.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, adding an extra dependency to setup.py isn't an option unfortunately

Let me know if you're able to experiment with getting this working, or I can try to take it from here if you don't have the capacity

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated it to use a protocol instead. The type checking still looks to achieve the intended result on my end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it looks like I was wrong when I said we support 3.8+. We actually still support 3.7 here, which doesn't support Protocol either.

If we can keep some of this confined to the TYPE_CHECKING block, that could be a solution, since we our mypy checks run against a more recent version of Python (Maybe ParamSpec could work that way?). Or maybe there's another way to accomplish the same thing

If you're interested in running some of these tests locally, there's a short section in the CONTRIBUTING file on using nox. It works well with pyenv. Or you can let me know if you want me to take more of this on

I'll take another look at this next week, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to have time to continue looking at this for the forseeable future.



class AsyncTransaction(async_batch.AsyncWriteBatch, BaseTransaction):
"""Accumulate read-and-write operations to be sent in a transaction.

Expand Down Expand Up @@ -236,11 +251,13 @@ class _AsyncTransactional(_BaseTransactional):
A coroutine that should be run (and retried) in a transaction.
"""

def __init__(self, to_wrap) -> None:
def __init__(
self, to_wrap: Callable[Concatenate[AsyncTransaction, P], Awaitable[T]]
) -> None:
super(_AsyncTransactional, self).__init__(to_wrap)

async def _pre_commit(
self, transaction: AsyncTransaction, *args, **kwargs
self, transaction: AsyncTransaction, *args: P.args, **kwargs: P.kwargs
) -> Coroutine:
"""Begin transaction and call the wrapped coroutine.

Expand All @@ -254,7 +271,7 @@ async def _pre_commit(
along to the wrapped coroutine.

Returns:
Any: result of the wrapped coroutine.
T: result of the wrapped coroutine.

Raises:
Exception: Any failure caused by ``to_wrap``.
Expand All @@ -269,20 +286,22 @@ async def _pre_commit(
self.retry_id = self.current_id
return await self.to_wrap(transaction, *args, **kwargs)

async def __call__(self, transaction, *args, **kwargs):
async def __call__(
self, transaction: AsyncTransaction, *args: P.args, **kwargs: P.kwargs
) -> T:
"""Execute the wrapped callable within a transaction.

Args:
transaction
(:class:`~google.cloud.firestore_v1.transaction.Transaction`):
(:class:`~google.cloud.firestore_v1.async_transaction.AsyncTransaction`):
A transaction to execute the callable within.
args (Tuple[Any, ...]): The extra positional arguments to pass
along to the wrapped callable.
kwargs (Dict[str, Any]): The extra keyword arguments to pass
along to the wrapped callable.

Returns:
Any: The result of the wrapped callable.
T: The result of the wrapped callable.

Raises:
ValueError: If the transaction does not succeed in
Expand Down Expand Up @@ -321,13 +340,13 @@ async def __call__(self, transaction, *args, **kwargs):


def async_transactional(
to_wrap: Callable[[AsyncTransaction], Any]
) -> _AsyncTransactional:
to_wrap: Callable[Concatenate[AsyncTransaction, P], Awaitable[T]]
) -> Callable[Concatenate[AsyncTransaction, P], Awaitable[T]]:
"""Decorate a callable so that it runs in a transaction.

Args:
to_wrap
(Callable[[:class:`~google.cloud.firestore_v1.transaction.Transaction`, ...], Any]):
(Callable[[:class:`~google.cloud.firestore_v1.async_transaction.AsyncTransaction`, ...], Any]):
A callable that should be run (and retried) in a transaction.

Returns:
Expand Down
Loading