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
process: set publisher methods' max retry to 600s #495
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change, while expedient, is a bit ugly: the deadlines are emitted from this template, based on this method configuration in the googleapis
repository.
If there is a general usecase for setting the deadlines to a higher multiple of the proto-repository-specified timeout, then we should have an issue open against gapic-generator-python
to support it (and maybe we need additional configuration available in the service JSON config).
Yeah, the thing is that currently the same value is used for retry deadline and the default timeout, thus indeed a generator change would be required to support it. I can check at the next Python libs weekly if this is something other libraries could use as well, or whether it's just something specific to Pub/Sub clients - @kamalaboulhosn do you maybe know from the top of your head? |
Update: As discussed offline, we will go with this patchy version until it is decided where (and if) in the generator should this be changed. |
Yeah, this does look a little hacky. I guess this is something that comes from the service config. @hongalex is this a config we can change in a single place that can be distributed to the repos so we don't to do these ad-hoc changes? |
The problem here is that the gapic code generator currently uses the same setting for both the timeout and the retry deadline ( FWIW, it was agreed on the Python libs weekly yesterday that it needs to be investigated whether this is a quirk in the generator, or a lack of feature that would have to be implemented. Until then, the OwlBot replacement rule should be fine in order to not block this P1 change for too long. |
Fixes #494.
PR checklist: