Improve error messaging and string formatting consistency #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add timeout to subprocess.communicate() to prevent indefinite hanging - Fix pexpect timeout from None to use actual timeout parameter - Add proper process cleanup with graceful termination → wait → force kill - Improve credential cache daemon cleanup with timeout handling - Add comprehensive error handling for timeout scenarios - Ensure all subprocess exceptions properly clean up processes Fixes issue where git subprocesses could accumulate and consume system resources when commands hang due to network issues or authentication problem
- Use f-strings consistently throughout timeout error handling - Add more specific error messages including PID for credential daemon cleanup - Improve git command failure logging with more descriptive messages - Include timeout error details in warning logs for better debugging 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Replace f-strings in logging calls with % formatting for lazy evaluation - Prevents eager evaluation of expressions in logging statements - Follows Python logging best practices for performance and error handling - Maintains same informative error messages with proper logging patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Extract subprocess functions to module level with proper timeout handling - Add timeout parameter to call_subprocess_with_authentication - Implement graceful termination (terminate → wait → kill) for subprocess timeouts - Add comprehensive test suite for timeout scenarios and process cleanup - Improve credential daemon cleanup with proper timeout handling - Ensure proper logging with lazy evaluation for all timeout scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Zsailer
force-pushed
the
subprocess-improvements
branch
from
38a301a to
aacce32
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Changes Made
%formatting to follow Python logging best practicesWhy These Changes Matter
%formatting in logging is only evaluated if the message will actually be logged, unlike f-strings which are evaluated eagerlyTest Plan