Skip to content

Use subprocess timeout#2127

Open
ngie-eign wants to merge 3 commits intogitpython-developers:mainfrom
ngie-eign:use-subprocess-timeout
Open

Use subprocess timeout#2127
ngie-eign wants to merge 3 commits intogitpython-developers:mainfrom
ngie-eign:use-subprocess-timeout

Conversation

@ngie-eign
Copy link
Copy Markdown

subprocess's APIs in 3.3+ support passing timeout to calls, such as
.communicate(..), .wait(..), etc. Pass kill_after_timeout to those
APIs and remove the watchdog handler code as it's not needed once
timeout= is used.

This enables kill_after_timeout on Windows platforms by side-effect as
upstream implements timeout for all supported platforms.

Requires: #2126

In the event the end-user called one of the APIs with
`with_stdout=False`, i.e., they didn't want to capture stdout, the code
would crash with an AttributeError or ValueError when trying to
dereference the stdout/stderr streams attached to `Popen(..)` objects.

Be more defensive by checking the streams first to make sure they're not
`None` before trying to access their corresponding attributes.

Add myself to AUTHORS and add corresponding regression tests for the
change.

Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>
Prior to this the test would fail [silently] on my macOS host during the
test and then pytest would complain loudly about it being an issue
post-session (regardless of whether or not the test was being run).

Squash the unwritable directory to mute noise complaints from pytest.

Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>
@ngie-eign ngie-eign force-pushed the use-subprocess-timeout branch 2 times, most recently from 2d09ccf to 1435486 Compare April 18, 2026 17:29
@ngie-eign ngie-eign marked this pull request as ready for review April 18, 2026 17:54
@ngie-eign
Copy link
Copy Markdown
Author

The failure may have been caused by a sporadic network outage. Let's try again.

subprocess's APIs in 3.3+ support passing timeout to calls, such as
.communicate(..), .wait(..), etc. Pass `kill_after_timeout` to those
APIs and remove the watchdog handler code as it's not needed once
`timeout=` is used.

This enables `kill_after_timeout` on Windows platforms by side-effect as
upstream implements `timeout` for all supported platforms.

Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>
@ngie-eign ngie-eign force-pushed the use-subprocess-timeout branch from 1435486 to 14190cb Compare April 18, 2026 17:55
@ngie-eign ngie-eign marked this pull request as draft April 18, 2026 17:55
@ngie-eign ngie-eign marked this pull request as ready for review April 18, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant