-
Notifications
You must be signed in to change notification settings - Fork 256
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
sync: add a 5 second timeout to gemato's openpgp refresh #1374
base: master
Are you sure you want to change the base?
Conversation
@@ -340,11 +340,11 @@ def _get_openpgp_env(self, openpgp_key_path=None, debug=False): | |||
|
|||
if openpgp_key_path: | |||
openpgp_env = gemato.openpgp.OpenPGPEnvironment( | |||
proxy=proxy, debug=debug |
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.
Let's do 10-15, perhaps? I'm willing to compromise on 5 and something higher if a proxy is set.
I'm thinking of the Tor case where e.g. no circuits have been built yet.
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.
Fair enough, timeout is 15 if a proxy is set and 5 otherwise.
If it takes more than 5 seconds to download a miniscule file from https://gentoo.org/.well-known/openpgpkey/ then something has gone dreadfully wrong. Most commonly, what went wrong is that the user has broken ipv6 connectivity and requests simply hangs forever (no joke -- it doesn't implement Happy Eyeballs and closed the multiple bug reports as "please use stackoverflow to ask questions about how to use requests, we are not a support forum"). Pass a timeout so that we eventually give up and try ipv4 instead. This is a crude hack and the proper solution is to make gemato handle this better, but until gemato is fixed to use a better download library we do the best we can. Bug: psf/requests#6788 Bug: projg2/gemato#35 Bug: https://bugs.gentoo.org/779766 Signed-off-by: Eli Schwartz <[email protected]>
|
||
if openpgp_key_path: | ||
openpgp_env = gemato.openpgp.OpenPGPEnvironment( | ||
proxy=proxy, debug=debug | ||
proxy=proxy, debug=debug, timeout=timeout | ||
) |
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.
Does this timeout mean that callers should be adjusted to handle requests.Timeout
exceptions?
We could possibly catch it here and then re-raise it as portage.exeception.TimeoutException
or something.
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.
Hmm, possibly. It doesn't look like gemato handles requests.Timeout nor uses the base exception classes.
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.
How do you want to handle this given that portage doesn't directly depend on requests?
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.
We can substitute TimeoutError if requests is missing like this:
try:
from requests.exceptions import Timeout
execept ImportError:
Timeout = TimeoutError
Does domain resolution count towards that timeout? Because if it does, then 5 seconds is very low. |
If it takes more than 5 seconds to download a miniscule file from https://gentoo.org/.well-known/openpgpkey/ then something has gone dreadfully wrong. Most commonly, what went wrong is that the user has broken ipv6 connectivity and requests simply hangs forever (no joke -- it doesn't implement Happy Eyeballs and closed the multiple bug reports as "please use stackoverflow to ask questions about how to use requests, we are not a support forum").
Pass a timeout so that we eventually give up and try ipv4 instead. This is a crude hack and the proper solution is to make gemato handle this better, but until gemato is fixed to use a better download library we do the best we can.
Bug: psf/requests#6788
Bug: projg2/gemato#35
Bug: https://bugs.gentoo.org/779766