-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Rest: Warn about swallowed _request_timeout #1069
Rest: Warn about swallowed _request_timeout #1069
Conversation
If `_request_timeout` is neither an int, nor a 2-tuple, it is swallowed without further notice which is a rather unfortunate because the level developers would have to look for this issue is pretty deep. This actually leads to confusion already, see apache/airflow#6643 (comment) While it would break backwards compatibility to raise an exception, we should at least warn the developer.
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Welcome @sbrandtb! |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sbrandtb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I am working on the CLA... |
/check-cla |
/retest |
@sbrandtb: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/check-cla |
This issue is probably better filed upstream, since this file is automatically generated. |
I agree with @micw523. This file is generated by https://github.com/openapitools/openapi-generator. The discussion and change fit better there. /close |
@roycaihw: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
For those that stumble across this issue in the near future, this root issue here is resolved within https://github.com/openapitools/openapi-generator and the fix will be released alongside their upcoming >>> from kubernetes.client.configuration import Configuration
>>> from kubernetes.client.rest import RESTClientObject
>>> rest_client_object = RESTClientObject(Configuration())
>>> impossible_timeout = 1e-10 # seconds, light travels ~3 cm
>>>
>>> # will ignore the provided timeout
>>> rest_client_object.request('GET', 'http://neverssl.com', _request_timeout=impossible_timeout)
<kubernetes.client.rest.RESTResponse object at 0x7f9c03cfff90>
>>>
>>> # will respect the provided timeout
>>> rest_client_object.request('GET', 'http://neverssl.com', _request_timeout=(
... 0.1 * impossible_timeout, 0.9 * impossible_timeout))
Traceback (most recent call last):
socket.timeout: timed out
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
urllib3.exceptions.ConnectTimeoutError: (<urllib3.connection.HTTPConnection object at 0x7f9c03e20410>, 'Connection to neverssl.com timed out. (connect timeout=1.0000000000000001e-11)')
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
urllib3.exceptions.MaxRetryError: HTTPConnectionPool(host='neverssl.com', port=80): Max retries exceeded with url: / (Caused by ConnectTimeoutError(<urllib3.connection.HTTPConnection object at 0x7f9c03e20410>, 'Connection to neverssl.com timed out. (connect timeout=1.0000000000000001e-11)')) I actually ended up isolating the problem on my end before finding this issue here, so search keywords to aid the all mighty Googs: Kubernetes doesn't timeout, ignores timeout, never times out, hangs on network, waits for network |
If
_request_timeout
is neither an int, nor a 2-tuple, it is swallowed without further notice which is a rather unfortunate because the level developers would have to look for this issue is pretty deep.This actually leads to confusion already, see apache/airflow#6643 (comment)
While it would break backwards compatibility to raise an exception, we should at least warn the developer.
This is meant as kind of a discussion base. I am not fully aware of how you developers would like to deal with this. Feedback appreciated 🙂