-
Notifications
You must be signed in to change notification settings - Fork 75
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.
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
fix: Correct AsyncTransaction Return Types #751
base: main
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
f8c8446
to
a255665
Compare
Completely unrelated to this PR, it looks like your repo doesn't have |
a255665
to
88238cc
Compare
So it looks like there's a mismatch between what is returned and what is declared in the type hints. I agree with this fix to align with the type hints in principle, but I am worried that it could break existing users who are used to the "wrong" return value You mention that you see errors whether you treat the result as a coroutine or an async generator. Is there really no way to access the generator? Right now, I'm thinking this would make sense to address in a future breaking change, along with other type fixes needed as part of #773. But we can probably get this in sooner if it really is completely broken as-is |
Converting to draft as there has been no recent activity |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #750