diff --git a/datalad_next/runners/run.py b/datalad_next/runners/run.py index 1f5593554..eef6731dc 100644 --- a/datalad_next/runners/run.py +++ b/datalad_next/runners/run.py @@ -6,6 +6,7 @@ import logging import subprocess +import sys from collections.abc import Generator from contextlib import contextmanager from pathlib import Path @@ -257,14 +258,14 @@ def run( try: yield KillingResultGenerator(result) except: - import sys, time e = sys.exc_info()[1] - lgr.warning( - f'Possible stall: exception ({e!r}) raised in ' - f'run-context ({cmd!r}), waiting for subprocess exit. If the ' - f'subprocess exit was not triggered yet, and if no termination-' - f' or kill-timeout is active, this might wait forever.' - ) + if kill_time is None: + lgr.warning( + f'Possible stall: exception ({e!r}) raised in ' + f'run-context ({cmd!r}) without kill-timeout. Waiting for ' + f'subprocess exit, If subprocess exit was not triggered yet, ' + f'this might wait forever.' + ) raise finally: # Arm the protocol, that will enable terminate signaling or kill @@ -276,5 +277,9 @@ def run( # the result code of the terminated process. # NOTE: if terminate_time and kill_time are both None, this might # loop forever. + lgr.debug( + f'Waiting for termination of {cmd!r}, terminate_time: ' + f'{terminate_time!r}, kill_time: {kill_time!r}' + ) for _ in result: pass diff --git a/datalad_next/runners/tests/test_run.py b/datalad_next/runners/tests/test_run.py index 8f35dc918..efa146293 100644 --- a/datalad_next/runners/tests/test_run.py +++ b/datalad_next/runners/tests/test_run.py @@ -8,7 +8,6 @@ from pathlib import Path from queue import Queue from random import randint -from typing import Generator import pytest @@ -380,20 +379,32 @@ def test_run_nongenerator(): assert res['stderr'] == 'error' -def test_run_exception_in_context(monkeypatch): - # Check that an exception in the context is logged and re-raises: +@pytest.mark.parametrize( + 'termination_time,kill_time,expect_warning', + [ + (1, None, True), + (None, 1, False), + (1, 1, False), + (None, None, True), + ] +) +def test_run_exception_in_context(monkeypatch, termination_time, kill_time, expect_warning): + # Check that an exception in the context is re-raised, and that a + # stall-warning is emitted, if no timeout was set. warnings = [] monkeypatch.setattr( 'datalad_next.runners.run.lgr.warning', lambda s: warnings.append(s) ) + with pytest.raises(ValueError): - with run([ - sys.executable, '-u', '-c', - 'import time\n' - 'time.sleep(5)\n' - ], - StdOutCaptureGeneratorProtocol, - ): + with run([sys.executable, '-u', '-c', 'import time\ntime.sleep(2)\n'], + StdOutCaptureGeneratorProtocol, + terminate_time=termination_time, + kill_time=kill_time): raise ValueError('Something') - assert warnings[0].startswith('Possible stall:') + if expect_warning: + assert len(warnings) == 1 + assert warnings[0].startswith('Possible stall:') + else: + assert len(warnings) == 0