Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-133089: Use original timeout value forTimeoutExpired
when the funcsubprocess.run
is called with a timeout#133103
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…ut=None' when the timeout is zeroSigned-off-by: Manjusaka <me@manjusaka.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What about a negativetimeout? Why doeswait
need this too?
This needs doc updates, and a test.
Misc/NEWS.d/next/Library/2025-04-29-02-23-04.gh-issue-133089.8Jy1ZS.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
How about raise ValueError when timeout is |
StanFromIreland commentedApr 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Hmm, looking at bash functions I get conflicting results. Yes, a value error is the way to go for negative timeouts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't thinktimeout=0
should be treated the same astimeout=None
. This isn't done anywhere else in the standard library. For example, usingasyncio
as precedent:
importasyncioasyncdefmain():asyncwithasyncio.timeout(None):awaitasyncio.sleep(0)print("No timeout!")asyncwithasyncio.timeout(0):awaitasyncio.sleep(0)# TimeoutErrorasyncio.run(main())
I think it would be better to fix the negative time.
Uh oh!
There was an error while loading.Please reload this page.
Make sense, I'll find a way to patch it |
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think your documentation addition is useful, but the logic change is not what we want.
If you want a cleaner value in the TimeoutExpired error message in this situation, that could be done by catching andreplacing the info in the TimeoutExpired exception from Popen._communicate'sself.wait(timeout=self._remaining_time(endtime))
with the orig_timeout value that is known at that point..wait()
itself cannot meaningfully know that value.
Uh oh!
There was an error while loading.Please reload this page.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
subprocess.run
's behavior is same with 'timeout=None' when the timeout is zeroTimeoutExpired
when the funcsubprocess.run
is called with a timeoutSigned-off-by: Manjusaka <me@manjusaka.me>
Uh oh!
There was an error while loading.Please reload this page.
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
TimeoutExpired
when the funcsubprocess.run
is called with a timeoutTimeoutExpired
when the funcsubprocess.run
is called with a timeout2bbcaed
intopython:mainUh oh!
There was an error while loading.Please reload this page.
… the func `subprocess.run` is called with a timeout (pythonGH-133103)(cherry picked from commit2bbcaed)Co-authored-by: Nadeshiko Manju <me@manjusaka.me>Signed-off-by: Manjusaka <me@manjusaka.me>Co-authored-by: Gregory P. Smith <greg@krypto.org>
GH-133418 is a backport of this pull request to the3.13 branch. |
…n the func `subprocess.run` is called with a timeout (GH-133103) (#133418)gh-133089: Use original timeout value for `TimeoutExpired` when the func `subprocess.run` is called with a timeout (GH-133103)(cherry picked from commit2bbcaed)Signed-off-by: Manjusaka <me@manjusaka.me>Co-authored-by: Nadeshiko Manju <me@manjusaka.me>Co-authored-by: Gregory P. Smith <greg@krypto.org>
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
bedevere-bot commentedMay 5, 2025
|
The few buildbot failures onpython#133103are possibly just due to racing a child process launch and exit?
The few buildbot failures on#133103are possibly just due to racing a child process launch and exit?
…GH-133420)The few buildbot failures onpython#133103are possibly just due to racing a child process launch and exit?(cherry picked from commitb64aa30)Co-authored-by: Gregory P. Smith <greg@krypto.org>
Uh oh!
There was an error while loading.Please reload this page.
Fix#133089
subprocess.run
'sTimeoutExpired
exception when settingtimeout=0
#133089