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

BF: obtain information about annex special remotes also from annex journal #6135

Merged

Conversation

yarikoptic
Copy link
Member

An alternative to #6133 implementation
which first forces git-annex to commit.

The use case was detected while working on datalad/datalad-crawler#112
to accompany #6105 .

Although we could have easily "fixed" the test in datalad-crawler, I consider
the approach of going around git-annex commands to interrogate git-annex
metadata to be fragile, in particular for such a case as here: some other
command could have been ran in alwayscommit=false mode, and information in
git-annex branch might not yet reflect actual state of git-annex. Ideally we
should avoid doing such "going around git-annex", but ATM there is no git-annex
command which would "expose" all this information.

…urnal

An alternative to datalad#6133 implementation
which first forces git-annex to commit.

The use case was detected while working on datalad/datalad-crawler#112
to accompany datalad#6105 .

Although we could have easily "fixed" the test in datalad-crawler, I consider
the approach of going around git-annex commands to interrogate git-annex
metadata to be fragile, in particular for such a case as here: some other
command could have been ran in alwayscommit=false mode, and information in
git-annex branch might not yet reflect actual state of git-annex.  Ideally we
should avoid doing such "going around git-annex", but ATM there is no git-annex
command which would "expose" all this information.
@yarikoptic yarikoptic changed the base branch from master to maint November 2, 2021 20:28
@yarikoptic yarikoptic added the semver-patch Increment the patch version when merged label Nov 2, 2021
@codecov
Copy link

codecov bot commented Nov 2, 2021

Codecov Report

Merging #6135 (ad7c39c) into maint (dccedaf) will decrease coverage by 29.78%.
The diff coverage is 65.41%.

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

@@             Coverage Diff             @@
##            maint    #6135       +/-   ##
===========================================
- Coverage   85.97%   56.19%   -29.79%     
===========================================
  Files         312      312               
  Lines       42215    42494      +279     
===========================================
- Hits        36296    23878    -12418     
- Misses       5919    18616    +12697     
Impacted Files Coverage Δ
datalad/cmdline/common_args.py 100.00% <ø> (ø)
datalad/cmdline/helpers.py 41.52% <0.00%> (-29.57%) ⬇️
datalad/core/distributed/tests/test_clone.py 0.00% <0.00%> (-97.05%) ⬇️
datalad/core/distributed/tests/test_push.py 0.00% <ø> (-98.09%) ⬇️
datalad/core/local/tests/test_diff.py 0.00% <0.00%> (-97.24%) ⬇️
datalad/core/local/tests/test_run.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/core/local/tests/test_status.py 0.00% <0.00%> (-97.33%) ⬇️
datalad/customremotes/archives.py 0.00% <0.00%> (-62.15%) ⬇️
datalad/customremotes/tests/test_base.py 0.00% <0.00%> (-93.69%) ⬇️
datalad/distributed/export_to_figshare.py 26.41% <0.00%> (+0.16%) ⬆️
... and 261 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 80d93a5...9a9a542. Read the comment docs.

Copy link
Member

@mih mih left a comment

Choose a reason for hiding this comment

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

This LGTM. Thx!

However, I want to remind about a comment from Joey

Also reading .git/annex/journal/ will miss configs of private remotes, which are stored in a separate journal and never reach the git-annex branch.

This should be mentioned in the docs, at least. We are not using this yet, but soon "ephemeral clones" will be private annex repos, and then any special remote configured in them would still be invisible to the method. Ping #5835

@mih mih merged commit a31e85b into datalad:maint Nov 5, 2021
@yarikoptic yarikoptic deleted the bf-sticking-our-fingers-also-into-annex-journal branch November 5, 2021 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants