-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -330,21 +330,23 @@ def noisy_refresh_keys(): | |
|
||
def _get_openpgp_env(self, openpgp_key_path=None, debug=False): | ||
if gemato is not None: | ||
timeout = 15 | ||
# Override global proxy setting with one provided in emerge configuration | ||
if "http_proxy" in self.spawn_kwargs["env"]: | ||
proxy = self.spawn_kwargs["env"]["http_proxy"] | ||
elif "https_proxy" in self.spawn_kwargs["env"]: | ||
proxy = self.spawn_kwargs["env"]["https_proxy"] | ||
else: | ||
proxy = None | ||
timeout = 5 | ||
|
||
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 commentThe reason will be displayed to describe this comment to others. Learn more. Does this timeout mean that callers should be adjusted to handle We could possibly catch it here and then re-raise it as There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe 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 |
||
else: | ||
openpgp_env = gemato.openpgp.OpenPGPSystemEnvironment( | ||
proxy=proxy, debug=debug | ||
proxy=proxy, debug=debug, timeout=timeout | ||
) | ||
|
||
return openpgp_env | ||
|
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.