Address compatibility with most recent git-annex where info would exit with non-0 #7292

yarikoptic · 2023-02-21T17:49:30Z

Runner - make exception_on_error=False in effect + use it for annex info.
See individual commits description for more info.

yarikoptic · 2023-02-21T17:53:10Z

datalad/runner/runner.py

@@ -211,8 +212,10 @@ def run(self,
        lgr.debug("Finished %r with status %s", cmd, results['code'])

        # make it such that we always blow if a protocol did not report
-        # a return code at all
-        if results.get('code', True) not in [0, None]:


@mih - this is the True default which puzzled me and I switched to None but may be incorrectly. May be you recall motivation in 9551b91 -- e.g. "when there is no code here we really not treat it as None but want to get None from somewhere deeper within"?

I am not sure what this wants to say

when there is no code here we really not treat it as None but want to get None from somewhere deeper within

but what this conditional did was stated in the still present comment:

make it such that we always blow if a protocol did not report a return code at all

Because in the linked commit it established:

The result is now always a dict that is expected to have keys 'code', 'stdout', 'stderr', matching the semantics of the CommandError exception.

The changed code seems to achieve the same behavior: raise if there is no code in the results. So LGTM.

I can live with LGTM without dwelling on the rest. Cheers!

yarikoptic · 2023-02-21T20:56:31Z

wow -- good number of fails. Something really common to RIA stuff

(tinuous-dev) datalad@smaug:/mnt/datasets/datalad/ci/logs/2023/02/21/pr/7292/a6d04eb$ git grep '%.*FAIL'  | sed -e 's,.*\],,g' | sort | uniq -c
      5  FAILED ../datalad/core/distributed/tests/test_clone.py::test_no_ria_postclonecfg 
      5  FAILED ../datalad/distributed/tests/test_create_sibling_ria.py::test_create_push_url 
      4  FAILED ../datalad/distributed/tests/test_create_sibling_ria.py::test_create_simple 
      5  FAILED ../datalad/distributed/tests/test_ria_basics.py::test_binary_data 
      5  FAILED ../datalad/distributed/tests/test_ria_basics.py::test_obtain_permission_ssh 
      4  FAILED ../datalad/distributed/tests/test_ria_basics.py::test_remote_layout_ssh 
      4  FAILED ../datalad/distributed/tests/test_ria_basics.py::test_version_check_ssh 
      4  FAILED ../datalad/distributed/tests/test_ria_git_remote.py::test_bare_git_version_1 
      4  FAILED ../datalad/distributed/tests/test_ria_git_remote.py::test_bare_git_version_2 
      5  FAILED ../datalad/support/tests/test_sshconnector.py::test_ssh_copy 
      4  XFAIL ../datalad/local/tests/test_rerun.py::test_run_inputs_outputs 
      5  XFAIL ../datalad/resources/procedures/tests/test_noannex.py::test_noannex_fail_if_has_annexed 
      5  XFAIL ../datalad/support/tests/test_sshrun.py::test_exit_code 
      6  XFAIL ../datalad/tests/test_utils.py::test_is_interactive

just to make sure all relate to this change -- #7293

yarikoptic · 2023-02-22T14:13:52Z

fails might be unrelated: #7296

@mih

…on_on_error=False Original behavior to raise that CommandError after process exits was introduced in 9551b91 (0.13.0rc1~120^2~1) whenever exception_on_error was added later on in 9f066ef (0.16.0^2~123^2~7). It is not clear to me (question to @mih) why it was `get` with defaulting to `True`, so I have changed it to `None`. I have then adjusted the logic to actually react to when there was nothing (we got None stored or got as default) really to match comment before. And made it to raise CommandError for non-0 only if exception_on_error was True.

Closes datalad#7286 . 10.20230214 release of git-annex came with a change of behavior for info to exit with non-0 exit code if any path was not known to it to provide info. Moreover it had regression to exit immediately thus possibly not returning any json record for valid files further on command line invocation. That was later fixed in [10.20230214-22-g16d3097a0](https://git.kitenet.net/index.cgi/git-annex.git/commit/?id=16d3097a0832a94eacdd966f30723323fa2eb817) . Here we just making _call_annex_records for info to not raise CommandError if exits with non-0. It relies on prior commit adjusting behavior down in the runner

yarikoptic · 2023-02-25T18:43:19Z

travis -- 1 run stalled, likely the Travis stalls in one of the runs #7253
appveyor/Mac test -- mac with the new buggy (we can't just easily workaround) git-annex. I will xfail those tests for this version of git-annex so we just get it green, FTR

appveryor:

FAILED ../datalad/support/tests/test_annexrepo.py::test_AnnexRepo_is_under_annex[False] - AssertionError
FAILED ../datalad/support/tests/test_annexrepo.py::test_AnnexRepo_is_under_annex[True] - datalad.cmd.BatchedCommandError: BatchedCommandError: 'git -c diff.ignoreSubmodules=none -c annex.merge-annex-branches=false annex info --bytes --fast --json --json-error-messages --batch' failed with exitcode 1
FAILED ../datalad/support/tests/test_annexrepo.py::test_AnnexRepo_web_remote - AssertionError
FAILED ../datalad/core/distributed/tests/test_clone.py::test_gin_cloning - AssertionError
FAILED ../datalad/support/tests/test_annexrepo.py::test_annex_add_no_dotfiles - AssertionError
FAILED ../datalad/support/tests/test_gitrepo.py::test_gitrepo_add_to_git_with_annex_v7 - AssertionError

mac - brew:

FAILED ../distributed/tests/test_clone.py::test_gin_cloning - AssertionError
FAILED ../tests/test_annexrepo.py::test_AnnexRepo_is_under_annex[False] - AssertionError
FAILED ../tests/test_annexrepo.py::test_AnnexRepo_is_under_annex[True] - datalad.cmd.BatchedCommandError: BatchedCommandError: 'git -c diff.ignoreSubmodules=none -c annex.merge-annex-branches=false annex info --bytes --fast --json --json-error-messages --batch' failed with exitcode 1
FAILED ../tests/test_annexrepo.py::test_AnnexRepo_web_remote - AssertionError
FAILED ../tests/test_annexrepo.py::test_annex_add_no_dotfiles - AssertionError
FAILED ../tests/test_gitrepo.py::test_gitrepo_add_to_git_with_annex_v7 - AssertionError

mac - snapshot, where I expected it to pass but it doesn't

has annex cmd:annex: 10.20230127-ge970c4eb9 , which is 10.20230126-79-ge970c4eb9 AKA 10.20230214~11 from Feb 13 22:46:48 2023 . Not sure why we have such an old snapshot (probably already the buggy one but we would not be able to easily xfail with such version)?! We are installing from https://downloads.kitenet.net/git-annex/OSX/current/10.15_Catalina/git-annex.dmg , so not our build snapshots , may be we should switch to our snapshots - I will try within this PR. edit: tried, would need defining GITHUB_TOKEN so decided not to bother -- will drop that commit (FTR: 158d1a6).

yarikoptic · 2023-02-25T19:40:57Z

aha -- will need to fix up for the fact that git-annex version comparison fails in docs build since there is no git-annex! will do after CI finishes this round.

in docs building environment there could be no git-annex

codecov · 2023-02-25T20:58:52Z

Codecov Report

Base: 88.65% // Head: 90.68% // Increases project coverage by +2.03% 🎉

Coverage data is based on head (8228cf8) compared to base (e18dc8c).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##            maint    #7292      +/-   ##
==========================================
+ Coverage   88.65%   90.68%   +2.03%     
==========================================
  Files         326      326              
  Lines       44429    44439      +10     
  Branches     5919     5919              
==========================================
+ Hits        39387    40300     +913     
+ Misses       5027     4124     -903     
  Partials       15       15

Impacted Files	Coverage Δ
datalad/support/annexrepo.py	`90.43% <ø> (ø)`
datalad/core/distributed/tests/test_clone.py	`98.43% <100.00%> (+<0.01%)`	⬆️
datalad/local/tests/test_add_archive_content.py	`99.25% <100.00%> (-0.37%)`	⬇️
datalad/local/tests/test_run_procedure.py	`100.00% <100.00%> (ø)`
datalad/runner/runner.py	`100.00% <100.00%> (ø)`
datalad/support/tests/test_annexrepo.py	`98.04% <100.00%> (+<0.01%)`	⬆️
datalad/support/tests/test_gitrepo.py	`99.77% <100.00%> (+<0.01%)`	⬆️
datalad/tests/utils_pytest.py	`89.09% <100.00%> (+1.09%)`	⬆️
datalad/support/tests/test_sshrun.py	`98.27% <0.00%> (-1.73%)`	⬇️
... and 12 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

yarikoptic · 2023-02-26T02:38:02Z

hm, on the last run metalad tests fail all of a sudden (attn @christian-monch ) ,

2023-02-25T22:36:56.7541231Z     - module: datalad_metalad
2023-02-25T22:36:56.7541507Z     - version: 0.4.11+28.g5c4e165

FAILED tests/test_add.py::test_subdataset_add_dataset_end_to_end - AssertionError
FAILED tests/test_add.py::test_subdataset_add_file_end_to_end - AssertionError
FAILED tests/test_add.py::test_current_dir_add_end_to_end - AssertionError: dicts differ:
 ['root_dataset_id'] differs: '00010203-1011-2021-3031-404142434445' != '<unknown>'
FAILED tests/test_add.py::test_add_file_dump_end_to_end - AssertionError: dicts differ:
 ['root_dataset_id'] differs: '00010203-1011-2021-3031-404142434445' != '<unknown>'

note that version is not 0.4.12... apparently that one is not merged into master, so master doesn't have all fixes... submitted datalad/datalad-metalad#337 .
travis - #7253

overall -- this PR is as ready as it can be

mslw · 2023-02-27T09:31:56Z

I did not look into the specifics, but it seems that you've got this issue covered - I'd support merging considering it shows up also outside the CI when the "latest" git-annex release is installed.

yarikoptic · 2023-02-27T13:51:44Z

With a blessing (thank you @mslw for the review!) I will make. But I would still encourage @mih to look into the question I had, there might be some side effect from my change

yarikoptic-gitmate · 2023-02-27T13:52:52Z

PR released in 0.18.2

yarikoptic added semver-patch Increment the patch version when merged release Create a release when this pr is merged CHANGELOG-missing When a PR's description does not contain a changelog item, yet. labels Feb 21, 2023

yarikoptic requested review from mih and christian-monch February 21, 2023 17:49

github-actions bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Feb 21, 2023

yarikoptic commented Feb 21, 2023

View reviewed changes

yarikoptic mentioned this pull request Feb 22, 2023

A swarm of tests (RIA + smth about ssh version) started to fail #7296

Closed

mslw mentioned this pull request Feb 23, 2023

most recent release of git-annex breaks 9 datalad tests #7286

Closed

yarikoptic and others added 3 commits February 25, 2023 10:38

[release-action] Autogenerate changelog snippet for PR 7292

65b6417

yarikoptic force-pushed the bf-info-exit branch from db062bb to 65b6417 Compare February 25, 2023 15:38

yarikoptic added 2 commits February 25, 2023 15:21

TST: xfail range of annex versions to have buggy info

a6ae688

BF: ensure that some git-annex version is there to compare for first

6d536ad

in docs building environment there could be no git-annex

yarikoptic force-pushed the bf-info-exit branch from 5470fa0 to 6d536ad Compare February 25, 2023 20:22

Lower version for xfail further for OSX snapshot builds

8228cf8

mslw approved these changes Feb 27, 2023

View reviewed changes

yarikoptic merged commit 8b7b8ce into datalad:maint Feb 27, 2023

yarikoptic deleted the bf-info-exit branch February 27, 2023 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Address compatibility with most recent git-annex where info would exit with non-0 #7292

Address compatibility with most recent git-annex where info would exit with non-0 #7292

Uh oh!

yarikoptic commented Feb 21, 2023

Uh oh!

yarikoptic Feb 21, 2023

Uh oh!

mih Feb 27, 2023

Uh oh!

yarikoptic Feb 27, 2023

Uh oh!

yarikoptic commented Feb 21, 2023

Uh oh!

yarikoptic commented Feb 22, 2023

Uh oh!

yarikoptic commented Feb 25, 2023 •

edited

Loading

Uh oh!

yarikoptic commented Feb 25, 2023

Uh oh!

codecov bot commented Feb 25, 2023 •

edited

Loading

Uh oh!

yarikoptic commented Feb 26, 2023

Uh oh!

mslw commented Feb 27, 2023

Uh oh!

yarikoptic commented Feb 27, 2023

Uh oh!

yarikoptic-gitmate commented Feb 27, 2023

Uh oh!

Uh oh!

Address compatibility with most recent git-annex where info would exit with non-0 #7292

Address compatibility with most recent git-annex where info would exit with non-0 #7292

Uh oh!

Conversation

yarikoptic commented Feb 21, 2023

Uh oh!

yarikoptic Feb 21, 2023

Choose a reason for hiding this comment

Uh oh!

mih Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

yarikoptic Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

yarikoptic commented Feb 21, 2023

Uh oh!

yarikoptic commented Feb 22, 2023

Uh oh!

yarikoptic commented Feb 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yarikoptic commented Feb 25, 2023

Uh oh!

codecov bot commented Feb 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yarikoptic commented Feb 26, 2023

Uh oh!

mslw commented Feb 27, 2023

Uh oh!

yarikoptic commented Feb 27, 2023

Uh oh!

yarikoptic-gitmate commented Feb 27, 2023

Uh oh!

Uh oh!

yarikoptic commented Feb 25, 2023 •

edited

Loading

codecov bot commented Feb 25, 2023 •

edited

Loading