Skip to content

requests.exceptions.ConnectionError retry #176

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

Closed
nitishxp opened this issue Apr 28, 2021 · 7 comments · Fixed by #178
Closed

requests.exceptions.ConnectionError retry #176

nitishxp opened this issue Apr 28, 2021 · 7 comments · Fixed by #178
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@nitishxp
Copy link

nitishxp commented Apr 28, 2021

Hi,

Could we add ConnectionError in list of retry so the client libraries which depend over api-core don't need to add.

"/layers/google.python.pip/pip/lib/python3.8/site-packages/google/cloud/bigquery/client.py", line 561, in create_table api_response = self._call_api( File "/layers/google.python.pip/pip/lib/python3.8/site-packages/google/cloud/bigquery/client.py", line 574, in _call_api return call() File "/layers/google.python.pip/pip/lib/python3.8/site-packages/google/api_core/retry.py", line 281, in retry_wrapped_func return retry_target( File "/layers/google.python.pip/pip/lib/python3.8/site-packages/google/api_core/retry.py", line 184, in retry_target return target() File "/layers/google.python.pip/pip/lib/python3.8/site-packages/google/cloud/_http.py", line 472, in api_request response = self._make_request( File "/layers/google.python.pip/pip/lib/python3.8/site-packages/google/cloud/_http.py", line 336, in _make_request return self._do_request( File "/layers/google.python.pip/pip/lib/python3.8/site-packages/google/cloud/_http.py", line 374, in _do_request return self.http.request( File "/layers/google.python.pip/pip/lib/python3.8/site-packages/google/auth/transport/requests.py", line 464, in request response = super(AuthorizedSession, self).request( File "/layers/google.python.pip/pip/lib/python3.8/site-packages/requests/sessions.py", line 530, in request resp = self.send(prep, **send_kwargs) File "/layers/google.python.pip/pip/lib/python3.8/site-packages/requests/sessions.py", line 643, in send r = adapter.send(request, **kwargs) File "/layers/google.python.pip/pip/lib/python3.8/site-packages/requests/adapters.py", line 498, in send raise ConnectionError(err, request=request) requests.exceptions.ConnectionError: ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))

I have reached out to individually to storage,bigquery and auth sdk page to report about this issue and they have acknowledged it googleapis/google-auth-library-python#739, googleapis/google-auth-library-python#737, googleapis/python-storage#414

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Apr 28, 2021
@busunkim96
Copy link
Contributor

Hi,

I don't think we should retry at every level that shows up in the stack trace (that might result in retrying more times than is desirable). I think addressing googleapis/google-auth-library-python#737 should be sufficient. If it turns out that isn't enough, we can revisit tweaking the rtries in google-api-core and/or google-cloud-core.

@parthea parthea reopened this May 1, 2021
@parthea parthea self-assigned this May 1, 2021
@parthea parthea added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed triage me I really want to be triaged. labels May 1, 2021
@parthea
Copy link
Collaborator

parthea commented May 1, 2021

I'm planning to add

    requests.exceptions.ConnectionError,
    auth_exceptions.TransportError,

to the list of errors that can be retried here as we have had success with this configuration in the BigQuery client and it follows the spirit of client side retries in AIP-4221. Googlers see b/182321177 for internal discussion.

gcf-merge-on-green bot pushed a commit that referenced this issue May 3, 2021
…178)

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:
- [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-api-core/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [x] Ensure the tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)

Fixes #176 🦕
@nitishxp
Copy link
Author

nitishxp commented May 4, 2021

@busunkim96
Copy link
Contributor

@nitishxp ConnectionError was added, it's just referred to as requests.exceptions.ConnectionError here.

@nitishxp
Copy link
Author

nitishxp commented May 4, 2021

@busunkim96 ConnectionError is specfic to python https://docs.python.org/3/library/exceptions.html#ConnectionError which is an OS error big query sdk https://github.com/googleapis/python-bigquery/blob/6a48e80bc7d347f381b181f4cf81fef105d0ad0d/google/cloud/bigquery/retry.py#L26 has added os connection error in retry along with requests.ConnectionError

@busunkim96
Copy link
Contributor

Ah I see. Do you have a stack trace that shows an un-retried ConnectionError? I'm wondering if that's specific to BIgQuery.

@nitishxp
Copy link
Author

nitishxp commented May 4, 2021

@busunkim96 Naah , But i have faced this issue in Cloud function when there r too many instances of CF running and same instance is used for another request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants