Skip to content
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

vimPlugins: backoff on timeout in update.py #73499

Merged
merged 2 commits into from Nov 27, 2019
Merged

Conversation

@timokau
Copy link
Member

timokau commented Nov 16, 2019

Motivation for this change

Updating vim-plugins recently started timing out regularly for me. It
may have to do with an ISP switch on my side, but I don't think that
should cause timeouts. I guess I'm probably not the only one
experiencing this, so in this comment I introduce exponential backoff.
Every request will be retried up to 3 times (3 seconds, 6 seconds, 12
seconds delay).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

CC @Mic92 @rvolosatovs

from functools import wraps


def retry(ExceptionToCheck, tries=4, delay=3, backoff=2, logger=None):

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 16, 2019

Contributor

Can you add type hints here as well?

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 16, 2019

Contributor
Suggested change
def retry(ExceptionToCheck, tries=4, delay=3, backoff=2, logger=None):
def retry(ExceptionToCheck: Exception, tries: int = 4, delay : int = 3, backoff: int = 2) -> Callable:

This comment has been minimized.

Copy link
@Mic92

Mic92 Nov 16, 2019

Contributor

All the type hints are untested, so you may want to check with mypy.

This comment has been minimized.

Copy link
@timokau

timokau Nov 16, 2019

Author Member

Unfortunately Exception didn't work, maybe python special cases that? mypy complained:

update.py:58: error: Exception type must be derived from BaseException
update.py:81: error: Argument 1 to "retry" has incompatible type "Type[URLError]"; expected "Exception"
update.py:94: error: Argument 1 to "retry" has incompatible type "Type[URLError]"; expected "Exception"

So I used "Any" instead (which also respects that you could theoretically pass in a tuple of Exceptions).

pkgs/misc/vim-plugins/update.py Outdated Show resolved Hide resolved
pkgs/misc/vim-plugins/update.py Outdated Show resolved Hide resolved
pkgs/misc/vim-plugins/update.py Outdated Show resolved Hide resolved
@timokau timokau force-pushed the timokau:updatepy-retry branch from cafdb56 to 20ce577 Nov 16, 2019
@timokau

This comment has been minimized.

Copy link
Member Author

timokau commented Nov 16, 2019

Addressed your comments. Also fixed unrelated formatting and typing issues.

@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Nov 19, 2019

Unfortunately we now have a merging conflict.

timokau added 2 commits Nov 16, 2019
Updating vim-plugins recently started timing out regularly for me. It
may have to do with an ISP switch on my side, but I don't think that
should cause timeouts. I guess I'm probably not the only one
experiencing this, so in this comment I introduce exponential backoff.
Every request will be retried up to 3 times (3 seconds, 6 seconds, 12
seconds delay).
@timokau timokau force-pushed the timokau:updatepy-retry branch from 20ce577 to 8c757f4 Nov 19, 2019
@timokau

This comment has been minimized.

Copy link
Member Author

timokau commented Nov 19, 2019

Rebased.

@timokau

This comment has been minimized.

Copy link
Member Author

timokau commented Nov 27, 2019

@Mic92 please have another look :)

@Mic92 Mic92 merged commit 9ecb2c4 into NixOS:master Nov 27, 2019
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@timokau timokau deleted the timokau:updatepy-retry branch Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.