Introduce new parameter raise_timeout_exceptions
Merge pull request #61 from DiamondLightSource/timeout-exceptions
Markus Gerstel authored 2 years ago
GitHub committed 2 years ago
50 | 50 | * stdout and stderr is printed by default, can be disabled |
51 | 51 | * stdout and stderr can be passed to any arbitrary function for |
52 | 52 | live processing (separately, both as unicode strings) |
53 | * optionally enforces a time limit on the process | |
53 | * optionally enforces a time limit on the process, raising a | |
54 | subprocess.TimeoutExpired exception if it is exceeded. | |
54 | 55 | |
55 | 56 | Credits |
56 | 57 | ------- |
428 | 428 | environment_override=None, |
429 | 429 | win32resolve=True, |
430 | 430 | working_directory=None, |
431 | raise_timeout_exception=False, | |
431 | 432 | ): |
432 | 433 | """ |
433 | 434 | Run an external process. |
454 | 455 | extension. |
455 | 456 | :param string working_directory: If specified, run the executable from |
456 | 457 | within this working directory. |
458 | :param boolean raise_timeout_exception: Forward compatibility flag. If set | |
459 | then a subprocess.TimeoutExpired exception is raised | |
460 | instead of returning an object that can be checked | |
461 | for a timeout condition. Defaults to False, will be | |
462 | changed to True in a future release. | |
457 | 463 | :return: A ReturnObject() containing the executed command, stdout and stderr |
458 | 464 | (both as bytestrings), and the exitcode. Further values such as |
459 | 465 | process runtime can be accessed as dictionary values. |
471 | 477 | start_time = timeit.default_timer() |
472 | 478 | if timeout is not None: |
473 | 479 | max_time = start_time + timeout |
480 | if not raise_timeout_exception: | |
481 | warnings.warn( | |
482 | "Using procrunner with timeout and without raise_timeout_exception set is deprecated", | |
483 | DeprecationWarning, | |
484 | stacklevel=2, | |
485 | ) | |
474 | 486 | |
475 | 487 | if environment is not None: |
476 | 488 | env = {key: _path_resolve(environment[key]) for key in environment} |
569 | 581 | # send terminate signal and wait some time for buffers to be read |
570 | 582 | p.terminate() |
571 | 583 | if thread_pipe_pool: |
572 | thread_pipe_pool[0].poll(0.5) | |
584 | try: | |
585 | thread_pipe_pool[0].poll(0.5) | |
586 | except BrokenPipeError as e: | |
587 | # on Windows this raises "BrokenPipeError: [Errno 109] The pipe has been ended" | |
588 | # which is for all intents and purposes equivalent to a True return value. | |
589 | if e.winerror != 109: | |
590 | raise | |
591 | thread_pipe_pool.pop(0) | |
573 | 592 | if not stdout.has_finished() or not stderr.has_finished(): |
574 | 593 | time.sleep(2) |
575 | 594 | p.poll() |
579 | 598 | # send kill signal and wait some more time for buffers to be read |
580 | 599 | p.kill() |
581 | 600 | if thread_pipe_pool: |
582 | thread_pipe_pool[0].poll(0.5) | |
601 | try: | |
602 | thread_pipe_pool[0].poll(0.5) | |
603 | except BrokenPipeError as e: | |
604 | # on Windows this raises "BrokenPipeError: [Errno 109] The pipe has been ended" | |
605 | # which is for all intents and purposes equivalent to a True return value. | |
606 | if e.winerror != 109: | |
607 | raise | |
608 | thread_pipe_pool.pop(0) | |
583 | 609 | if not stdout.has_finished() or not stderr.has_finished(): |
584 | 610 | time.sleep(5) |
585 | 611 | p.poll() |
602 | 628 | |
603 | 629 | stdout = stdout.get_output() |
604 | 630 | stderr = stderr.get_output() |
631 | ||
632 | if timeout_encountered and raise_timeout_exception: | |
633 | raise subprocess.TimeoutExpired( | |
634 | cmd=command, timeout=timeout, output=stdout, stderr=stderr | |
635 | ) | |
636 | ||
605 | 637 | time_end = time.strftime("%Y-%m-%d %H:%M:%S GMT", time.gmtime()) |
606 | ||
607 | 638 | result = ReturnObject( |
608 | 639 | exitcode=p.returncode, |
609 | 640 | command=command, |
3 | 3 | import procrunner |
4 | 4 | import pytest |
5 | 5 | import sys |
6 | ||
7 | ||
8 | @mock.patch("procrunner._NonBlockingStreamReader") | |
9 | @mock.patch("procrunner.time") | |
10 | @mock.patch("procrunner.subprocess") | |
11 | @mock.patch("procrunner.Pipe") | |
12 | def test_run_command_aborts_after_timeout_legacy( | |
13 | mock_pipe, mock_subprocess, mock_time, mock_streamreader | |
14 | ): | |
15 | mock_pipe.return_value = mock.Mock(), mock.Mock() | |
16 | mock_process = mock.Mock() | |
17 | mock_process.returncode = None | |
18 | mock_subprocess.Popen.return_value = mock_process | |
19 | task = ["___"] | |
20 | ||
21 | with pytest.raises(RuntimeError): | |
22 | with pytest.warns(DeprecationWarning, match="timeout"): | |
23 | procrunner.run(task, -1, False) | |
24 | ||
25 | assert mock_subprocess.Popen.called | |
26 | assert mock_process.terminate.called | |
27 | assert mock_process.kill.called | |
6 | 28 | |
7 | 29 | |
8 | 30 | @mock.patch("procrunner._NonBlockingStreamReader") |
19 | 41 | task = ["___"] |
20 | 42 | |
21 | 43 | with pytest.raises(RuntimeError): |
22 | procrunner.run(task, -1, False) | |
44 | procrunner.run(task, -1, False, raise_timeout_exception=True) | |
23 | 45 | |
24 | 46 | assert mock_subprocess.Popen.called |
25 | 47 | assert mock_process.terminate.called |
65 | 87 | callback_stdout=mock.sentinel.callback_stdout, |
66 | 88 | callback_stderr=mock.sentinel.callback_stderr, |
67 | 89 | working_directory=mock.sentinel.cwd, |
90 | raise_timeout_exception=True, | |
68 | 91 | ) |
69 | 92 | |
70 | 93 | assert mock_subprocess.Popen.called |
104 | 127 | def test_default_process_environment_is_parent_environment(mock_subprocess): |
105 | 128 | mock_subprocess.Popen.side_effect = NotImplementedError() # cut calls short |
106 | 129 | with pytest.raises(NotImplementedError): |
107 | procrunner.run([mock.Mock()], -1, False) | |
130 | procrunner.run([mock.Mock()], -1, False, raise_timeout_exception=True) | |
108 | 131 | assert mock_subprocess.Popen.call_args[1]["env"] == os.environ |
109 | 132 | |
110 | 133 | |
114 | 137 | mock_env = {"key": mock.sentinel.key} |
115 | 138 | # Pass an environment dictionary |
116 | 139 | with pytest.raises(NotImplementedError): |
117 | procrunner.run([mock.Mock()], -1, False, environment=copy.copy(mock_env)) | |
140 | procrunner.run( | |
141 | [mock.Mock()], | |
142 | -1, | |
143 | False, | |
144 | environment=copy.copy(mock_env), | |
145 | raise_timeout_exception=True, | |
146 | ) | |
118 | 147 | assert mock_subprocess.Popen.call_args[1]["env"] == mock_env |
119 | 148 | |
120 | 149 | |
131 | 160 | False, |
132 | 161 | environment=copy.copy(mock_env1), |
133 | 162 | environment_override=copy.copy(mock_env2), |
163 | raise_timeout_exception=True, | |
134 | 164 | ) |
135 | 165 | mock_env_sum = copy.copy(mock_env1) |
136 | 166 | mock_env_sum.update({key: str(mock_env2[key]) for key in mock_env2}) |
143 | 173 | mock_env2 = {"keyB": str(mock.sentinel.keyB)} |
144 | 174 | with pytest.raises(NotImplementedError): |
145 | 175 | procrunner.run( |
146 | [mock.Mock()], -1, False, environment_override=copy.copy(mock_env2) | |
176 | [mock.Mock()], | |
177 | -1, | |
178 | False, | |
179 | environment_override=copy.copy(mock_env2), | |
180 | raise_timeout_exception=True, | |
147 | 181 | ) |
148 | 182 | random_environment_variable = list(os.environ)[0] |
149 | 183 | if random_environment_variable == list(mock_env2)[0]: |
174 | 208 | environment_override={ |
175 | 209 | random_environment_variable: "X" + random_environment_value |
176 | 210 | }, |
211 | raise_timeout_exception=True, | |
177 | 212 | ) |
178 | 213 | assert ( |
179 | 214 | mock_subprocess.Popen.call_args[1]["env"][random_environment_variable] |
0 | 0 | import os |
1 | import subprocess | |
1 | 2 | import sys |
3 | import timeit | |
2 | 4 | |
3 | 5 | import procrunner |
4 | 6 | import pytest |
67 | 69 | assert ( |
68 | 70 | "LEAK_DETECTOR" not in os.environ |
69 | 71 | ), "overridden environment variable leaked into parent process" |
72 | ||
73 | ||
74 | def test_timeout_behaviour_legacy(tmp_path): | |
75 | start = timeit.default_timer() | |
76 | with pytest.warns(DeprecationWarning, match="timeout"): | |
77 | result = procrunner.run( | |
78 | [sys.executable, "-c", "import time; time.sleep(5)"], | |
79 | timeout=0.1, | |
80 | working_directory=tmp_path, | |
81 | raise_timeout_exception=False, | |
82 | ) | |
83 | runtime = timeit.default_timer() - start | |
84 | if hasattr(result, "timeout"): | |
85 | with pytest.warns(DeprecationWarning, match="\\.timeout"): | |
86 | assert result.timeout | |
87 | else: | |
88 | assert result["timeout"] | |
89 | assert runtime < 3 | |
90 | assert not result.stdout | |
91 | assert not result.stderr | |
92 | assert result.returncode | |
93 | ||
94 | ||
95 | def test_timeout_behaviour(tmp_path): | |
96 | command = (sys.executable, "-c", "import time; time.sleep(5)") | |
97 | start = timeit.default_timer() | |
98 | with pytest.raises(subprocess.TimeoutExpired) as te: | |
99 | procrunner.run( | |
100 | command, | |
101 | timeout=0.1, | |
102 | working_directory=tmp_path, | |
103 | raise_timeout_exception=True, | |
104 | ) | |
105 | runtime = timeit.default_timer() - start | |
106 | assert runtime < 3 | |
107 | assert te.value.stdout == b"" | |
108 | assert te.value.stderr == b"" | |
109 | assert te.value.timeout == 0.1 | |
110 | assert te.value.cmd == command |