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: Inform users about repo version auto-upgrades (try 2) #5698

Merged
merged 2 commits into from Jun 3, 2021

Conversation

kyleam
Copy link
Contributor

@kyleam kyleam commented May 26, 2021

This resurrects and updates gh-3027, which was reverted in gh-3044 due to a stalling test.

AnnexRepo.__init__() uses config.get() to get datalad.repo.version,
which will return the version as a string.  obtain() could be used
instead and would handle the conversion to int, but that doesn't work
well here because it's better for git-annex-init to determine
The current minimum version of git-annex (8.20200309) auto-upgrades
v3-7 to v8.  Let the user know that, despite having explicitly
specified a version, they will end up with a different one.

We tried this once before in abed6ef (ENH: annexrepo: Inform users
about repo version auto-upgrades, 2018-11-29), but that was reverted
due to triggering a stall in test_annex_ssh().  Since then, git-annex
has had a good number of SSH-related fixups and test_annex_ssh() has
also undergone major changes, so let's hope that problem has gone
away.

Note that, at the time of abed6ef, a dictionary was used for
_AUTO_UPGRADEABLE_VERSIONS because some versions upgraded to v5 and
others to v8.  For the currently supported git-annex versions,
everything upgrades to v8, so the dictionary isn't as necessary.
Stick with it, though, because it nicely corresponds to git-annex's
autoUpgradeableVersions and, although perhaps unlikely, it's possible
that there will be multiple upgrade targets again.

Re: datalad#2969 (comment)

Closes datalad#3045.
@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #5698 (d719809) into master (ddc7108) will decrease coverage by 1.93%.
The diff coverage is 88.23%.

❗ Current head d719809 differs from pull request most recent head 71c7c84. Consider uploading reports for the commit 71c7c84 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5698      +/-   ##
==========================================
- Coverage   90.55%   88.61%   -1.94%     
==========================================
  Files         308      305       -3     
  Lines       42151    42125      -26     
==========================================
- Hits        38170    37331     -839     
- Misses       3981     4794     +813     
Impacted Files Coverage Δ
datalad/support/annexrepo.py 91.43% <77.77%> (-0.21%) ⬇️
datalad/support/tests/test_annexrepo.py 97.12% <100.00%> (-0.49%) ⬇️
datalad/plugin/wtf.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/addurls.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/no_annex.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/add_readme.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/check_dates.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/support/tests/utils.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/export_archive.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/export_to_figshare.py 0.00% <0.00%> (-100.00%) ⬇️
... and 74 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 ddc7108...71c7c84. Read the comment docs.

# git-annex-init complain if it can't actually handle it.
lgr.warning(
"Expected an int for datalad.repo.version, got %s",
version)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we might want to do something about version instead of just going forward here. this code path is not tested so ramifications aren't immediately clear

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at the logic: so we will end up with some version which we alert about here but then do not announce about auto upgrades. I think that is ok, and we can proceed.

@yarikoptic yarikoptic merged commit 0b0e7fe into datalad:master Jun 3, 2021
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.

None yet

2 participants