Title: Clarify some Git.execute kill_after_timeout limitations by EliahKagan · Pull Request #1761 · gitpython-developers/GitPython · GitHub
Open Graph Title: Clarify some Git.execute kill_after_timeout limitations by EliahKagan · Pull Request #1761 · gitpython-developers/GitPython
X Title: Clarify some Git.execute kill_after_timeout limitations by EliahKagan · Pull Request #1761 · gitpython-developers/GitPython
Description: This makes the two preliminary changes discussed as possibly useful in #1756 (comment):
The Git.execute docstring is modified to include additional portability information for kill_after_timeout. I've tried to do this in a way that avoids going into too much detail, but still gives enough information to allow developers of software using GitPython to infer what systems may be reasonable to use it on, or what the effects of using it on systems where it is less effective might be. I considered linking in the docstring to #1756 or discussing the race condition issue, but omitted both at this time. I'd be pleased to add such information if it is wanted.
The implementation of the local kill_process function, which is used as the callback for kill_after_timeout, is modified to avoid giving the impression that it could be safely or effectively used in similar form on Windows. This mostly consists of removing code that was present with the intention of supporting Windows. But one of the changes that is part of this is raising AssertionError in the event that it somehow were called on Windows. This should not currently be able to happen because Windows is checked for, and an exception raised, prior to registering the callback in the first place. But this is an area of the code that is somewhat complicated, and I think this may be justified to express how that code is not portable or safe for Windows as written. The changes to kill_process are done in their own commit (and just one), so that they are easier to identify if in the future some of the attempts for it to support Windows are revisited (though this could not be done by calling the ps command).
See discussion in #1756. However, I do not assum based on what has been discussed there that these specific changes are necessarily ideal, and I'd be pleased to make changes on request.
I considered adding a test but decided not to, based in part on the point about tests in #1756 (comment), and in part on the limited nature of the changes. However, I did do the following to check that it still seems to work:
>>> git.Git().execute(['sleep', '10'], kill_after_timeout=5)
Traceback (most recent call last):
File "
Open Graph Description: This makes the two preliminary changes discussed as possibly useful in #1756 (comment): The Git.execute docstring is modified to include additional portability information for kill_after_timeout....
X Description: This makes the two preliminary changes discussed as possibly useful in #1756 (comment): The Git.execute docstring is modified to include additional portability information for kill_after_timeout....
Opengraph URL: https://github.com/gitpython-developers/GitPython/pull/1761
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/checks(.:format) |
| route-controller | pull_requests |
| route-action | checks |
| fetch-nonce | v2:69fb2edf-0d96-15fc-843c-4fafcbbfdc08 |
| current-catalog-service-hash | 87dc3bc62d9b466312751bfd5f889726f4f1337bdff4e8be7da7c93d6c00a25a |
| request-id | D37A:205EEA:F862C8:156863C:696896C0 |
| html-safe-nonce | 6aab95df40a20e52db47ca8181f1356553f013a81a4ca9bd17cf6f731dc4fbf7 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJEMzdBOjIwNUVFQTpGODYyQzg6MTU2ODYzQzo2OTY4OTZDMCIsInZpc2l0b3JfaWQiOiIyNzc0Mzg2NjQxMTg0NzkwMjA4IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 22437777e6e6a44f65cff04cf582229ee998dbf3ada00fcdd9b9815b2e078062 |
| hovercard-subject-tag | pull_request:1637589541 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,checks,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/gitpython-developers/GitPython/pull/1761/checks |
| twitter:image | https://avatars.githubusercontent.com/u/1771172?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/1771172?s=400&v=4 |
| og:image:alt | This makes the two preliminary changes discussed as possibly useful in #1756 (comment): The Git.execute docstring is modified to include additional portability information for kill_after_timeout.... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 50f46dc2d6192249fd8ebf20e76c800f4f2596d4a5f3ab63dd63a754df154f54 |
| turbo-cache-control | no-preview |
| go-import | github.com/gitpython-developers/GitPython git https://github.com/gitpython-developers/GitPython.git |
| octolytics-dimension-user_id | 503709 |
| octolytics-dimension-user_login | gitpython-developers |
| octolytics-dimension-repository_id | 1126087 |
| octolytics-dimension-repository_nwo | gitpython-developers/GitPython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 1126087 |
| octolytics-dimension-repository_network_root_nwo | gitpython-developers/GitPython |
| turbo-body-classes | logged-out env-production page-responsive full-width full-width-p-0 |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | fef287f17234b4529a4b112a3d47fe8551e32ddd |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width