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(TST): prevent auto-upgrade of "remote" test sibling, do not use local path for URL #6957

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

yarikoptic
Copy link
Member

Our docker image for testing of interactions over ssh still has git-annex 7.20190819+git2-g908476a9b-1~ndall+1
and that is great so we can ensure that datalad works well even with older versions (note - current minimal
supported is 8.20200309 but upgrade to that one would not matter in this case).

We use create_sibling to create remote through ssh but on the path which is also available locally.
And then instead of always interacting with that repository remotely, we had direct interactions with it
using AnnexRepo or creating a sibling not over ssh but pointing directly to that path. That would have lead
local git-annex possibly to upgrade it to later version of git annex repo (10 ATM) whenever remote (docker)
instance of git-annex would not know that version. And with annex version 10, a new command was added
"filter-process" which it records within .git/config for that annex.

So if we ever upgrade it by acessing a local path, "git push" fails since then
it runs git-annex filter-process and that fails in turn, and we do not even see
that error. Citing it here in full FTR from #6901 (comment)

(git-annex)lena:~/.tmp/datalad_temp_test_target_ssh_simplerf0k0b97[dl-test-branch]git
$> SSH_AGENT_PID= SSH_AUTH_SOCK= git push local_target dl-test-branch
Enumerating objects: 9, done.
Counting objects: 100% (9/9), done.
Delta compression using up to 12 threads
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 798 bytes | 798.00 KiB/s, done.
Total 9 (delta 2), reused 0 (delta 0), pack-reused 0
Invalid argument `filter-process'

Usage: git-annex COMMAND
  git-annex - manage files with git, without checking their contents in

  Commonly used commands:

  add             PATH ...                  add files to annex
...
fatal: the remote end hung up unexpectedly
To ssh://datalad-test/home/yoh/.tmp/datalad_temp_test_target_ssh_simplexx85c2vs/basic
 ! [remote rejected] dl-test-branch -> dl-test-branch (Could not update working tree to new HEAD)
error: failed to push some refs to 'ssh://datalad-test/home/yoh/.tmp/datalad_temp_test_target_ssh_simplexx85c2vs/basic'

where in the tests we only see that "Could not update" making it
difficult to pin point the problem.

In this commit I prevent auto-upgrade on that repo, and if then local operation
fails -- I just skip direct interaction with such AnnexRepo. And I also
replaced "replace"ing of the repo using local path, and instead using "custom"
ssh url so we could still compare that all urls are assigned as expected.

Closes #6901

PS I added release label . Auto-releasing would fail as previous one failed due to already too long of a changelog, uff, so I will release it manually after this PR is merged. It is needed to bring our CI in datalad/git-annex back to green.

…cal path for URL

Our docker image for testing of interactions over ssh still has git-annex  7.20190819+git2-g908476a9b-1~ndall+1
and that is great so we can ensure that datalad works well even with older versions  (note - current minimal
supported is 8.20200309 but upgrade to that one would not matter in this case).

We use create_sibling to create remote through ssh but on the path which is also available locally.
And then instead of always interacting with that repository remotely, we had direct interactions with it
using AnnexRepo or creating a sibling not over ssh but pointing directly to that path.  That would have lead
local git-annex possibly to upgrade it to later version of git annex repo (10 ATM) whenever remote (docker)
instance of git-annex would not know that version. And with annex version 10, a new command was added
"filter-process" which it records within .git/config for that annex.

So if we ever upgrade it by acessing a local path, "git push" fails since then
it runs git-annex filter-process and that fails in turn, and we do not even see
that error.  Citing it here in full FTR from datalad#6901 (comment)

	(git-annex)lena:~/.tmp/datalad_temp_test_target_ssh_simplerf0k0b97[dl-test-branch]git
	$> SSH_AGENT_PID= SSH_AUTH_SOCK= git push local_target dl-test-branch
	Enumerating objects: 9, done.
	Counting objects: 100% (9/9), done.
	Delta compression using up to 12 threads
	Compressing objects: 100% (9/9), done.
	Writing objects: 100% (9/9), 798 bytes | 798.00 KiB/s, done.
	Total 9 (delta 2), reused 0 (delta 0), pack-reused 0
	Invalid argument `filter-process'

	Usage: git-annex COMMAND
	  git-annex - manage files with git, without checking their contents in

	  Commonly used commands:

	  add             PATH ...                  add files to annex
	...
	fatal: the remote end hung up unexpectedly
	To ssh://datalad-test/home/yoh/.tmp/datalad_temp_test_target_ssh_simplexx85c2vs/basic
	 ! [remote rejected] dl-test-branch -> dl-test-branch (Could not update working tree to new HEAD)
	error: failed to push some refs to 'ssh://datalad-test/home/yoh/.tmp/datalad_temp_test_target_ssh_simplexx85c2vs/basic'

where in the tests we only see that "Could not update" making it
difficult to pin point the problem.

In this commit I prevent auto-upgrade on that repo, and if then local operation
fails -- I just skip direct interaction with such AnnexRepo. And I also
replaced "replace"ing  of the repo using local path, and instead using "custom"
ssh url so we could still compare that all urls are assigned as expected.

Closes datalad#6901
@yarikoptic yarikoptic added release Create a release when this pr is merged semver-tests Changes only affect tests, no impact on version labels Aug 17, 2022
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.

I am only vaguely familiar with the command and its code, but the description makes sense.

which was a target_path before and now a full url
@codecov
Copy link

codecov bot commented Aug 19, 2022

Codecov Report

Merging #6957 (b71080d) into maint (5f0b28d) will increase coverage by 1.01%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##            maint    #6957      +/-   ##
==========================================
+ Coverage   89.97%   90.99%   +1.01%     
==========================================
  Files         354      354              
  Lines       46259    46453     +194     
==========================================
+ Hits        41622    42269     +647     
+ Misses       4637     4184     -453     
Impacted Files Coverage Δ
datalad/distribution/tests/test_create_sibling.py 79.83% <94.11%> (+0.17%) ⬆️
datalad/_version.py 45.68% <0.00%> (-0.28%) ⬇️
datalad/distributed/drop.py 97.42% <0.00%> (-0.27%) ⬇️
datalad/distributed/tests/test_drop.py 100.00% <0.00%> (ø)
datalad/tests/test_config.py 99.73% <0.00%> (+<0.01%) ⬆️
datalad/config.py 97.26% <0.00%> (+<0.01%) ⬆️
datalad/tests/utils.py 65.10% <0.00%> (+11.03%) ⬆️
datalad/tests/test_tests_utils.py 92.34% <0.00%> (+92.34%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@yarikoptic
Copy link
Member Author

it is green everywhere (besides known crawler) -- let's proceed. Thanks @mih for blessing

@yarikoptic yarikoptic merged commit 9827f9b into datalad:maint Aug 19, 2022
@yarikoptic yarikoptic deleted the bf-test-create-sibling branch October 14, 2022 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Create a release when this pr is merged semver-tests Changes only affect tests, no impact on version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants