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

Use OutputFrom class as iter_subproc result, which keeps returncodes on exception #565

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

christian-monch
Copy link
Contributor

@christian-monch christian-monch commented Dec 10, 2023

This PR implements a solution for the problem of not-easily available return codes that was discussed here in PR #546.

This is an alternative (and in my view superior) approach to the discussed solution, which is implemented in PR #562.

The reason why this second approach exists is:

  1. It seems much more pythonic to me
  2. It is more general and does not introduce special handling of different exceptions (in this case, the handling of StopIterationin PR Handle StopIteration in iterable_subprocess #562)
  3. It makes for cleaner code when using iter_subproc, i.e. in SshUrlOperations.

Copy link

codecov bot commented Dec 10, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (9cc9171) 92.88% compared to head (a6928b3) 92.90%.

Files Patch % Lines
...ad_next/iterable_subprocess/iterable_subprocess.py 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
+ Coverage   92.88%   92.90%   +0.01%     
==========================================
  Files         143      143              
  Lines       10379    10411      +32     
  Branches     1145     1148       +3     
==========================================
+ Hits         9641     9672      +31     
- Misses        714      715       +1     
  Partials       24       24              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@christian-monch
Copy link
Contributor Author

The change that this PR provides are applied in SshUrlOperations in the PR #566 (in commit e2937d8)

@christian-monch christian-monch force-pushed the keep_returncode_on_exception branch 2 times, most recently from 89c966c to 06952f1 Compare December 11, 2023 21:57
This commit converts the generator function
`output_from` into an instance of a the
generator class `OutputFrom`.

The rationale for this conversion is that
the generator class can store the
return code of the subprocess as attribute.
Thus the returncode is available, even
if an exception is raised within the
`iterable_subprocess`-context.
This commit adds some basic tests to verify
that the `returncode` attribute of the
`iterable_subprocess` generator is properly
set.
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 works for me. It feels much better than the alternative handling, indeed. Thx!

@mih mih merged commit fbae378 into datalad:main Dec 13, 2023
9 checks passed
@mih mih added this to the 1.1 milestone Dec 15, 2023
@christian-monch christian-monch deleted the keep_returncode_on_exception branch January 15, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants