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

Handle StopIteration in iterable_subprocess #562

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions datalad_next/iterable_subprocess/iterable_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,17 @@ def raise_if_not_none(exception):
except _BrokenPipeError as e:
if proc.returncode == 0:
raise e.__context__ from None
except StopIteration:
# This except-clause prioritizes `IterableSubprocessError` over
# `StopIteration` to avoid swallowing the former.
# This assumes that any `StopIteration` that is
# caught here, and that coincides with a non-zero return code,
# is raised due to an unexpected short read from `stdout` of
# the subprocess and directly connected to the non-zero
# return code and can therefore be ignored, because the
# return code would transport all necessary information.
if proc.returncode == 0:
raise

if proc.returncode:
raise IterableSubprocessError(proc.returncode, b''.join(stderr_deque)[-chunk_size:])
Expand Down
17 changes: 16 additions & 1 deletion datalad_next/iterable_subprocess/test_iterable_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def test_exception_from_not_found_process_propagated():
b''.join(output)


def test_exception_from_return_code():
def test_exception_from_return_code(monkeypatch):
monkeypatch.setenv('LANG', 'C')
with pytest.raises(IterableSubprocessError, match='No such file or directory') as excinfo:
with iterable_subprocess(['ls', 'does-not-exist'], ()) as output:
a = b''.join(output)
Expand Down Expand Up @@ -311,3 +312,17 @@ def yield_input():

with iterable_subprocess(['funzip'], yield_input()) as output:
assert b''.join(output) == contents


def test_iterable_subprocess_error_if_non_zero_exit_code_and_stop_iteration_raised_in_context():
with pytest.raises(IterableSubprocessError):
with iterable_subprocess(['ls', 'does-not-exist'], ()) as ls:
next(ls)
raise StopIteration


def test_stop_iteration_if_zero_exit_code_and_stop_iteration_raised_in_context():
with pytest.raises(StopIteration):
with iterable_subprocess(['echo', 'a'], ()) as echo:
next(echo)
raise StopIteration
30 changes: 27 additions & 3 deletions datalad_next/runners/iter_subproc.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,33 @@ def iter_subproc(
passing chunks to the process, while the standard error thread
fetches the error output, and while the main thread iterates over
the process's output from client code in the context.
On context exit, the main thread closes the process's standard output,
waits for the standard input thread to exit, waits for the standard
error thread to exit, and wait for the process to exit.

On context exit without an exception, the main thread closes the
process's standard output, waits for the standard input thread to exit,
waits for the standard error thread to exit, and wait for the process to
exit. If the process exited with a non-zero return code, an
``IterableSubprocessError`` is raised, containing the process's return
code.

If the context is exited due to an exception that was raised in the
context, the main thread terminates the process via ``Popen.terminate()``,
closes the process's standard output, waits for the standard input
thread to exit, waits for the standard error thread to exit, and waits
for the process to exit. If the exception is not `StopIteration`, the
exception will be re-raised. If the exception is `StopIteration`, the
main thread will check whether the process exited with a non-zero
return code. If it did, an ``IterableSubprocessError`` is raised, else
a ``StopIteration`` is raised.

Note that any exception, except from ``StopIteration``, that is raised
in the context will bubble up to the main thread. In this case, the
return code of the subprocess will not be checked and no
``IterableSubprocessError`` will be raised if the process exited with a
non-zero return code. If you need to check the return code, you should
catch any exception that is not ``StopIteration`` within the context.
Similarly, if you need to know whether a ``StopIteration`` was raised
inside the context, you should catch it within the context.

"""
return iterable_subprocess(
args,
Expand Down
Loading