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

annexrepo: Set annex.retry=3 when calling git-annex #4382

Merged
merged 2 commits into from Apr 13, 2020

Conversation

kyleam
Copy link
Collaborator

@kyleam kyleam commented Apr 7, 2020

git-annex has an annex.retry option that instructs it to retry on
transfer failure, but it defaults to 0. Add a datalad.annex.retry
option that defaults to 3 and is used as the value for annex.retry in
our calls to git-annex, taking care to not override annex.retry if the
user has configured it.

This implements @yarikoptic's proposal from gh-2896 and will hopefully
help resolve some flaky test failures (e.g., gh-3653 and gh-4371).

Closes #2896.

In AnnexRepo.__init__(), we access .config twice for read-only
inspection and will access it once more in the next commit, so store
it as a local variable.
@codecov
Copy link

codecov bot commented Apr 7, 2020

Codecov Report

Merging #4382 into master will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4382      +/-   ##
==========================================
+ Coverage   88.70%   88.85%   +0.14%     
==========================================
  Files         285      285              
  Lines       37761    37764       +3     
==========================================
+ Hits        33496    33554      +58     
+ Misses       4265     4210      -55     
Impacted Files Coverage Δ
datalad/interface/common_cfg.py 100.00% <ø> (ø)
datalad/support/annexrepo.py 86.34% <100.00%> (+0.03%) ⬆️
datalad/downloaders/http.py 72.11% <0.00%> (-2.79%) ⬇️
datalad/downloaders/tests/test_http.py 58.79% <0.00%> (-2.17%) ⬇️
datalad/utils.py 83.96% <0.00%> (+0.09%) ⬆️
datalad/tests/test_cmd.py 97.32% <0.00%> (+0.53%) ⬆️
datalad/core/local/create.py 93.00% <0.00%> (+0.69%) ⬆️
datalad/tests/utils_testrepos.py 93.80% <0.00%> (+0.88%) ⬆️
datalad/cmd.py 89.96% <0.00%> (+1.37%) ⬆️
datalad/__init__.py 91.85% <0.00%> (+1.48%) ⬆️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1646113...906fce4. Read the comment docs.

@yarikoptic
Copy link
Member

yarikoptic commented Apr 7, 2020

wouldn't it be a worthwhile and appropriate addition to maint by making it more robust?

@kyleam
Copy link
Collaborator Author

kyleam commented Apr 7, 2020

wouldn't it be a worthwhile and appropriate addition to maint by making it more robust?

In my view, this change isn't a bug fix. It's a new option, that may help with some flaky tests. I don't like widely applying stuff to maint, as I've expressed before. If you insist and no one else chimes in to object, I'll rebase it there.

@bpoldrack
Copy link
Member

bpoldrack commented Apr 8, 2020

Agree with @kyleam re maint. I understand the motivation for putting it there, but we should follow very clear principle what goes where. If we already start blurring the border, I fear we'll end up in an unpleasant spot (again).

git-annex has an annex.retry option that instructs it to retry on
transfer failure, but it defaults to 0.  Add a datalad.annex.retry
option that defaults to 3 and is used as the value for annex.retry in
our calls to git-annex, taking care to not override annex.retry if the
user has configured it.

This implements @yarikoptic's proposal from dataladgh-2896 and will hopefully
help resolve some flaky test failures (e.g., dataladgh-3653 and dataladgh-4371).

Closes datalad#2896.
@kyleam kyleam changed the title annexrepo: Set annex.retries=3 when calling git-annex annexrepo: Set annex.retry=3 when calling git-annex Apr 8, 2020
@kyleam
Copy link
Collaborator Author

kyleam commented Apr 8, 2020

Given (1) a confusing typo in the subject and (2) the workflow builds failing across the board in setup, I've amended the message of the last commit and force-pushed (no code changes).

@kyleam
Copy link
Collaborator Author

kyleam commented Apr 8, 2020

The failing doc run is due to the upgrade to the most recent sphinx. I think the fix for the deprecation warning (treated as an error) is trivial, but locally I see additional failures that need to be investigated (builds fine with previous sphinx release) edit: The github workflow docbuild is showing the same warning->failure error that I see locally. The deprecation warning isn't causing the failure.

@kyleam
Copy link
Collaborator Author

kyleam commented Apr 8, 2020

Cancelled CrippledFS and -container builds, which were hanging in the "Set up system" step.

@yarikoptic
Copy link
Member

yarikoptic commented Apr 13, 2020

ok then, master it is ... let's hope it sees release some time soon ;-)

@yarikoptic yarikoptic merged commit 23d7683 into datalad:master Apr 13, 2020
9 of 12 checks passed
@kyleam kyleam deleted the annex-retry branch Apr 14, 2020
@yarikoptic yarikoptic added this to the 0.13.0 milestone Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants