Repository navigation
default thread timeout too short for CI #1339
Description
Activity
cc: @dwhswenson
Thanks for reporting and sorry for the hassle. Does that timeout not also means that clones or fetches can't take longer than that before assuming a hang? CC @Yobmod
@sroet A PR would definitely be welcome, even though it would be good to understand it fully first.
Hi, same on my CI-System, where fetches takes long time and 10 sec time-out are definitive to short. A default value of 60 sec for time-out are also fine for me.
The failure @sroet linked to in OpenPathSampling is raised from within Autorelease, which includes a tool to test that the version listed in the code is reasonable based on the versions implied by tags in the repo. Autorelease uses GitPython (thanks for a great package for interacting with git from Python!), and the problem here is when it fetches the upstream to ensure it knows all tags.
I use Autorelease on all my projects, and OpenPathSampling is the only one where this is a problem. I suspect this is because OpenPathSampling is a relatively large repository (both in clone size and in number of commits) -- though there are plenty of repos out there that are larger in each.
In previous passing nightly builds, total time for the "Autorelease check" step in the OpenPathSampling GitHub actions runs varies widely: I've seen as long as 1m27s, but that seems like an outlier. ~15s is standard. For my smaller repos, the "Autorelease check" step takes more like 2-5 seconds.
So a few thoughts:
- There's probably no default value of
timeoutthat will satisfy all users. If you're working with a sufficiently massive repo, this will be a problem. OpenPathSampling is large, but not an extreme outlier. - I see the timeout added in #1318, but I don't see any way for client code like Autorelease to actually make use of this. Either surfacing the the
timeoutparameter inRemote.fetch(and everything else that useshandle_process_output) or using an approach similar to updating thecmd.execute_kwargsdict would make it easier for me to implement a workaround. (For now, the easy solution is going to be pinning to an old version of GitPython.) - Is there a reason that the timeout in
handle_process_outputwould ever be less thanexecute_kwargs['kill_after_timeout']? Is there a reason not to reuse the value ofexecute_kwargs['kill_after_timeout']inhandle_process_output? This would default to no timeout, but would give client code a straightforward way to change this without adding confusion due to different kinds of timeouts.
- There's probably no default value of
Thanks for the elaborate explanation and for sharing your thoughts, I found them very insightful and helpful.
[…](thanks for a great package for interacting with git from Python!)[…]
I am not so sure about that anymore. Even though I don't know every possible usage of it, I think that it could have been at its best if it would have stuck to executing git and parsing its output, but do that very, very well. GitPython today is a deeply flawed hybrid which wants to be git, and probably beyond fixable.
For a start, I have yanked this release as the timeout, no matter how long, represents a breaking bug.
It looks like there is a PR by now, maybe @dwhswenson could take a look at #1340 as well. Thank you.

Since this morning our CI started failing with:
This seems a side effect of the timeout introduced by #1318
Could this time-out default to
60 sinstead (or be taken and propagated by thefetchfunction)?I could make a PR if needed, with either of the two solutions