fix(ga-cli): propagate exit code when update steps fail#658
Open
Kailigithub wants to merge 1 commit into
Open
Conversation
When `ga update` runs `git pull` and it fails (network down, merge conflict, dirty tree, non-fast-forward), the old code printed the git error to stdout and proceeded to `pip install -e .` on top of stale or conflicted source. The next `ga` invocation would then run against a corrupted editable install with unpredictable behaviour. Make the command fail fast: route git and pip errors to stderr, and `sys.exit(rc)` on either failure so the shell sees a non-zero status and the caller knows the update was not applied. Add a behavioural test in `tests/test_ga_cli_update_fail_fast.py`: - `git pull` fails -> SystemExit(128), pip never runs - `git pull` ok + `pip install` fails -> SystemExit(1), both steps called - both ok -> no SystemExit, both steps called Test patches `builtins.print` (not the module-level binding) because `print` in the module resolves through the builtins namespace.
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
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
ga updatepreviously swallowed errors fromgit pullandpip install -e ., printed them to stdout, and then proceeded to run the next step. Whengit pullfailed (network down, merge conflict, dirty tree, non-fast-forward), the command still ranpip install -e .on top of stale or conflicted source, silently corrupting the editable install and leaving the user with agathat behaved unpredictably.This change makes
cmd_updatefail fast:sys.stderr(where it belongs).sys.exit(rc)on either failure so the shell sees the non-zero status and the caller can detect the failed update.Test plan
tests/test_ga_cli_update_fail_fast.pycovers the three cases:git pullfails ->SystemExit(128),pip installnever runs.git pullok +pip installfails ->SystemExit(1), both steps called in order.SystemExit, both steps called in order.Verified via the three-step dance:
main(unfixed code does not raiseSystemExit).Test patches
builtins.printrather than the module'sprintbinding, becauseprintinga_cli/cli.pyresolves through the builtins namespace.Diff scope
ga_cli/cli.py: +6 / -3 lines (one source file, focused oncmd_update).tests/test_ga_cli_update_fail_fast.py: +121 lines (new test file).Total: under 130 lines, one commit, one PR.
Behavioural impact
git pullno longer cascades into a corruptedpip install. The shell prompt returns to the user with a non-zero exit code and the error visible on stderr.sysandsubprocessare already used.