Skip to content

Remove an unnecessary warning from the runner #6425

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

Conversation

christian-monch
Copy link
Contributor

Remove an unnecessary warning from the runner in the case where stdin is provided, but does not fit any of our specially handled cases.

Fixes #6395

Changelog

🐛 Bug Fixes

@adswa
Copy link
Member

adswa commented Feb 7, 2022

Thanks for the fix! Just tried it out 👍

@christian-monch christian-monch added the semver-internal Changes only affect the internal API label Feb 7, 2022
@christian-monch
Copy link
Contributor Author

Thanks for the fix! Just tried it out +1

Your welcome :-)

@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #6425 (d76ddbd) into master (cc84266) will increase coverage by 0.44%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6425      +/-   ##
==========================================
+ Coverage   89.57%   90.01%   +0.44%     
==========================================
  Files         345      348       +3     
  Lines       43032    44149    +1117     
==========================================
+ Hits        38545    39741    +1196     
+ Misses       4487     4408      -79     
Impacted Files Coverage Δ
datalad/runner/nonasyncrunner.py 99.62% <100.00%> (+0.01%) ⬆️
datalad/_version.py 46.99% <0.00%> (-53.01%) ⬇️
datalad/cli/parser.py 80.63% <0.00%> (-3.66%) ⬇️
datalad/cli/main.py 70.52% <0.00%> (-0.91%) ⬇️
datalad/customremotes/tests/test_archives.py 89.28% <0.00%> (-0.65%) ⬇️
datalad/config.py 97.03% <0.00%> (-0.53%) ⬇️
datalad/local/configuration.py 97.87% <0.00%> (-0.03%) ⬇️
datalad/cli/common_args.py 100.00% <0.00%> (ø)
datalad/downloaders/__init__.py 100.00% <0.00%> (ø)
datalad/interface/common_cfg.py 100.00% <0.00%> (ø)
... and 60 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 cc84266...d76ddbd. Read the comment docs.

@mih
Copy link
Member

mih commented Feb 7, 2022

Is the nosec interpreted by codeclimate?

kwargs = {
**self.popen_kwargs,
**dict(
bufsize=0,
stdin=subprocess.PIPE if self.write_stdin else self.stdin,
stdout=subprocess.PIPE if self.catch_stdout else None,
stderr=subprocess.PIPE if self.catch_stderr else None,
shell=True if isinstance(self.cmd, str) else False
shell=True if isinstance(self.cmd, str) else False # nosec
Copy link
Member

Choose a reason for hiding this comment

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

What nosec - what understand it, code climate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is understood by bandit, a security linter that is used by codeclimate.

@christian-monch
Copy link
Contributor Author

christian-monch commented Feb 8, 2022

Is the nosec interpreted by codeclimate?

It is used by bandit, a security linter that is used by codeclimate.

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.

LGTM, thx! I restarted the error-ing test. Once they passed, we could merge from my POV.

@christian-monch christian-monch force-pushed the bf-remove-unnecessary-runner-warning branch from d331bb7 to d76ddbd Compare February 8, 2022 15:34
@codeclimate
Copy link

codeclimate bot commented Feb 8, 2022

Code Climate has analyzed commit d76ddbd and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@christian-monch christian-monch merged commit 726a9d0 into datalad:master Feb 8, 2022
@christian-monch christian-monch deleted the bf-remove-unnecessary-runner-warning branch February 8, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-internal Changes only affect the internal API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weird [INFO][WARNING] messages emitted by clone
4 participants