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

Remove old Runner #5229

Merged
merged 7 commits into from Dec 15, 2020
Merged

Remove old Runner #5229

merged 7 commits into from Dec 15, 2020

Conversation

mih
Copy link
Member

@mih mih commented Dec 10, 2020

Fixed #5003

@codecov
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #5229 (4b00337) into master (ee1ac90) will increase coverage by 0.25%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5229      +/-   ##
==========================================
+ Coverage   89.72%   89.97%   +0.25%     
==========================================
  Files         301      297       -4     
  Lines       42413    41573     -840     
==========================================
- Hits        38054    37407     -647     
+ Misses       4359     4166     -193     
Impacted Files Coverage Δ
datalad/cmd.py 93.70% <ø> (+7.76%) ⬆️
datalad/customremotes/base.py 86.72% <ø> (+13.07%) ⬆️
datalad/interface/common_cfg.py 100.00% <ø> (ø)
datalad/support/gitrepo.py 91.87% <ø> (+1.35%) ⬆️
datalad/support/tests/test_gitrepo.py 99.89% <ø> (-0.01%) ⬇️
datalad/utils.py 85.00% <0.00%> (+0.54%) ⬆️
datalad/customremotes/tests/test_archives.py 89.26% <100.00%> (+2.55%) ⬆️
datalad/customremotes/tests/test_datalad.py 98.03% <100.00%> (-0.04%) ⬇️
datalad/support/annexrepo.py 89.44% <100.00%> (-0.09%) ⬇️
datalad/support/tests/test_annexrepo.py 97.10% <100.00%> (+<0.01%) ⬆️
... and 15 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 ee1ac90...4b00337. Read the comment docs.

This prevents one of the travis runs to exceeds max log size.

This is a test-battery-only problem, because `AnnexRepo.add_urls()` is
not used outside the tests (datalad#5153), and the plan is to get rid of it
eventually
(datalad#5153 (comment)).
This can happen, without being an indication of an error, when I command
like `whereis --json` legitimately produces no results, but at the same
time git-annex runs in debug mode (which can be automatically triggered
via log-level). But what would be an indication of trouble is stdout
content, that wasn't parsed to JSON records.
git-annex stderr will not be empty when in debug mode.
@mih mih marked this pull request as ready for review December 11, 2020 08:39
test_annexjson_protocol() fails when executed under
DATALAD_LOG_LEVEL=8 because git-annex is called with --debug when
`getEffectiveLevel() <= 8` while the condition in the previous commit
treats it as `getEffectiveLevel() <= 7`
Copy link
Contributor

@kyleam kyleam left a comment

Choose a reason for hiding this comment

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

Thanks, I'm very happy to see this. All the commits look good to me (with a boundary fixup pushed for "BF: Rectify test assumption").

Subject: [PATCH 2/6] RF: Remove run_gitcommand_on_file_list_chunks()

Made obsolete by GitWitlessRunner.run_on_file_list_chunks()

Dropping it right away, rather than leaving it around with a DeprecationWarning, is fine with me. It seems unlikely to be used by outside callers, and a quick search across known extensions and on github doesn't bring up anything.

@kyleam
Copy link
Contributor

kyleam commented Dec 11, 2020

Oy, I just now clicked through to the crawler failure. It has a good number of import errors, which I think are all coming from this part of datalad_crawler/base.py:

from datalad.cmd import Runner
from datalad.support.protocol import DryRunProtocol


def get_runner(*args, **kwargs):
    if cfg.obtain('datalad.crawl.dryrun', default=False):
        kwargs = kwargs.copy()
        kwargs['protocol'] = DryRunProtocol()
    return Runner(*args, **kwargs)

https://github.com/datalad/datalad/pull/5229/checks?check_run_id=1538522651

@@ -66,131 +63,6 @@ def get_function_nargs(f):
return len(argspec.args) - 1


class AnnexExchangeProtocol(ProtocolInterface):
Copy link
Member

Choose a reason for hiding this comment

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

bye bye my Dear friend, you were a good help! I can only hope that I will not miss you, in log and Joey we trust!

@mih
Copy link
Member Author

mih commented Dec 13, 2020

Oy, I just now clicked through to the crawler failure. It has a good number of import errors, which I think are all coming from this part of datalad_crawler/base.py:

I started a PR to perform the necessary RF: datalad/datalad-crawler#89

@mih mih added the merge-if-ok OP considers this work done, and requests review/merge label Dec 14, 2020
@mih
Copy link
Member Author

mih commented Dec 14, 2020

@yarikoptic has merged the crawler fix. thx! All good now!

@mih
Copy link
Member Author

mih commented Dec 15, 2020

OK, this is it!

I hope the fall-out over the coming days is not to intense, but I think it is good to merge this now and give it sufficient time to let the dust settle before a 0.14.

Thanks for all the bits you contributed over the years 👀 to make this possible. Witless we will be from now .

@mih mih merged commit b747ff5 into datalad:master Dec 15, 2020
@mih mih deleted the rf-norunner branch December 15, 2020 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-if-ok OP considers this work done, and requests review/merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fully remove (Git)Runner
3 participants