fix: use correct typing for retries #1026
Conversation
google-cla
bot
added
the
cla: yes
| @@ -22,6 +22,8 @@ from google.auth.transport.grpc import SslCredentials # type: ignore | |||
| from google.auth.exceptions import MutualTLSChannelError # type: ignore | |||
| from google.oauth2 import service_account # type: ignore | |||
|
|
|||
| OptionalRetry = Union[retries.Retry, object] | |||
There was a problem hiding this comment.
Wouldn't this be equivalent to Any type?
There was a problem hiding this comment.
I had to do some gymnastics to get a default sentinel type checkable in the BQ library:
There was a problem hiding this comment.
I'm not sure. I do know that gapic_v1.method.DEFAULT_RETRY is just an instance of object.
There was a problem hiding this comment.
Right. That's what I mean. object is a very generic type. Basically everything is object, so union with that is basically Any.
Would folks be open to making a more explicit type for the default sentinel like we did in BQ?
There was a problem hiding this comment.
To test if it is equivalent to Any, we could try passing a string as a retry argument. It ideally should fail type checks.
There was a problem hiding this comment.
I believe DEFAULT_RETRY is a specific object used as a canary.
There was a problem hiding this comment.
If we update api-core to provide a different type for the marker object, then we can tweak the templates to use it -- otherwise, the typechecker hates that we are using that specific object as the default when we say the parameter is a retries.Retry.
There was a problem hiding this comment.
Okay. Well I suspect this makes the type checker happy with any object, which isn't what we want. Did you see the pattern I used in google-cloud-bigquery? That pattern uses the fact that enums can be type checked to look for a specific object.
tseaver
force-pushed
the
1024-1025-fix-retry-client_options-types
branch
from
f604149
to
4ae06f1
Compare
fix: import 'ClientOptions' class correctly
Closes #1024
Closes #1025