-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: Adding ConnectionError to retry mechanism #822
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
Conversation
This commit fixes issue googleapis#558
a56b686
to
4ef1a56
Compare
We've noticed |
Also noticed |
@damgad Any idea how these modifications could be made "in place" until this PR is merged? |
@dylancaponi Sure, you can always monkeypatch the
That will just overwrite the the function from the library. You can also do it in shorter way:
I would also suggest to make sure that you've pinned the version of google-api-python-client in your requirements, as consequences of the automatic update may be unpredictable. |
@busunkim96 Any progress? This causes problems for Apache Airflow. Apache Airflow is used by the Cloud Composer service. |
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.
Thanks for the patch @damgad!
@busunkim96 Hello. Thanks for accepting this change. Do you know when it will appear in the new release? |
@damgad We've implemented the shorter monkeypatch for a few weeks. It seems to retry the call in question but not reestablish the connection. So we just get connectionRetryError 3x now instead of once per call. Wondering if I implemented something wrong or misunderstood the purpose of this PR. |
@dylancaponi Seems that you've correctly applied the patch and it works how it should work. However, the aim of the whole retry mechanism is to overcome single occurring errors - temporary issues that may disappear with the next try. In your case, you should either investigate why ConnectionRetryError really happens and try to fix the root cause or you can also increase the |
A new release has been released that includes this fix. |
As described in #558 current retry mechanism does not catch
ConnectionError
(andConnectionResetError
which inherits from it). That pull request fixes the issue by adding the python3 specific ConnectionError into the mechanism.