Skip to content
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

Raise a warning in google.api_core.retry.retry_target if the return value is awaitable #542

Open
ohmayr opened this issue Oct 25, 2023 · 1 comment · Fixed by #543 · May be fixed by #649
Open

Raise a warning in google.api_core.retry.retry_target if the return value is awaitable #542

ohmayr opened this issue Oct 25, 2023 · 1 comment · Fixed by #543 · May be fixed by #649
Assignees
Labels
type: question Request for information or clarification. Not an issue.

Comments

@ohmayr
Copy link
Contributor

ohmayr commented Oct 25, 2023

A warning can be raised within google.api_core.retry.retry_target depending on whether the return value of target() is awaitable in the following way:

if inspect.isawaitable(target()):
            raise ValueError("Incorrect use of Retry in async code. Use AsyncRetry instead")

This will ensure that google.api_core.retry.Retry is not incorrectly used in async clients/code and will suggest users to use google.api_core.retry_async.AsyncRetry instead

related issue: fix retries in async client

@daniel-sanche
Copy link
Contributor

I think this change should be re-considered. inspect.isawaitable is notoriously slow, and this change makes it a part of every single rpc. I don't think the benefit of the extra warning is worth adding that performance cost to every library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Request for information or clarification. Not an issue.
Projects
None yet
3 participants