Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

j: return exit code properly#755

Merged
bennyz merged 1 commit intomainfrom
fix-tmt-errcode
Nov 30, 2025
Merged

j: return exit code properly#755
bennyz merged 1 commit intomainfrom
fix-tmt-errcode

Conversation

@mangelajo
Copy link
Copy Markdown
Member

@mangelajo mangelajo commented Nov 28, 2025

fixes the bug of j tmt always returning 0

click in standalone_mode=False mode, as we use it in j, will not raise the click.Exception.Exit but instead return the result as an integer.

this commit looks for the integer, and raisex the exception that can be caught on the external try block.

Summary by CodeRabbit

  • Bug Fixes
    • Improved CLI exit code handling to properly propagate non-zero exit codes and ensure correct process termination when the CLI encounters errors or returns non-zero status codes.

✏️ Tip: You can customize this high-level summary in your review settings.

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 28, 2025

Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit 88241fc
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/692985aae4494d000868bffe
😎 Deploy Preview https://deploy-preview-755--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@mangelajo mangelajo requested a review from kirkbrauer November 28, 2025 11:20
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 28, 2025

Walkthrough

The CLI result handling has been updated to capture the return value and convert non-zero exit codes into exceptions. A new exception handler for click.exceptions.Exit has been added to properly terminate the process with the corresponding exit codes, replacing the previous behavior of ignoring the result.

Changes

Cohort / File(s) Summary
CLI Exit Code Handling
packages/jumpstarter-cli/jumpstarter_cli/j.py
Modified CLI invocation to capture return value and raise BaseExceptionGroup with click.Exit exception for non-zero results; added except* clause to handle exit exceptions and propagate exit codes via sys.exit()

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • Exception handling logic with BaseExceptionGroup and click.exceptions.Exit to ensure proper exception propagation and exit code semantics
  • Verification that the new exception flow doesn't mask or lose critical error information
  • Compatibility with existing error handling and CLI behavior expectations
  • Testing of non-zero exit scenarios and edge cases in exception handling

Poem

🐰 A hop through exits and error flows,
Where BaseExceptionGroups now grow,
Click exceptions caught with finest care,
Exit codes float through the air,
Proper termination's here to stay,
The CLI's found its rightful way!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'j: return exit code properly' directly and clearly describes the main change: fixing proper exit code handling in the CLI command.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-tmt-errcode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mangelajo mangelajo changed the title tmt-driver: returl exit code properly j: return exit code properly Nov 28, 2025
fixes the bug of j tmt always returning 0

click in standalone_mode=False mode, as we use it in j, will not raise
the click.Exception.Exit but instead return the result as an integer.

this commit looks for the integer, and raisex the exception that
can be caught on the external try block.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/jumpstarter-cli/jumpstarter_cli/j.py (1)

45-47: Consider simplifying since sys.exit() terminates immediately.

The loop will only ever execute once since sys.exit() raises SystemExit on the first iteration. Unlike the ClickException handler (lines 48-52) where all exceptions are shown before a single sys.exit(1), here the exit happens inside the loop.

This works correctly, but could be simplified to make the single-exit intent clearer:

     except* click.exceptions.Exit as excgroup:
-        for exc in leaf_exceptions(excgroup):
-            sys.exit(exc.exit_code)
+        exc = next(iter(leaf_exceptions(excgroup)))
+        sys.exit(exc.exit_code)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec806a0 and 88241fc.

📒 Files selected for processing (1)
  • packages/jumpstarter-cli/jumpstarter_cli/j.py (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

Ruff should be used for code formatting and linting, excluding jumpstarter-protocol package

Files:

  • packages/jumpstarter-cli/jumpstarter_cli/j.py
🧠 Learnings (3)
📚 Learning: 2025-05-28T15:09:35.768Z
Learnt from: NickCao
Repo: jumpstarter-dev/jumpstarter PR: 463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).

Applied to files:

  • packages/jumpstarter-cli/jumpstarter_cli/j.py
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.

Applied to files:

  • packages/jumpstarter-cli/jumpstarter_cli/j.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`

Applied to files:

  • packages/jumpstarter-cli/jumpstarter_cli/j.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: Redirect rules - jumpstarter-docs
  • GitHub Check: Header rules - jumpstarter-docs
  • GitHub Check: Pages changed - jumpstarter-docs
  • GitHub Check: build
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.13)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: e2e
🔇 Additional comments (1)
packages/jumpstarter-cli/jumpstarter_cli/j.py (1)

29-31: LGTM! Correctly handles Click's integer return value in standalone_mode=False.

The fix properly captures the exit code returned by Click when standalone_mode=False and wraps it in an exception group for the outer handler to process. The type check and non-zero condition are appropriate.

@bennyz bennyz merged commit afaf058 into main Nov 30, 2025
18 checks passed
@bennyz bennyz deleted the fix-tmt-errcode branch November 30, 2025 06:46
@jumpstarter-backport-bot
Copy link
Copy Markdown

Backport failed for release-0.7, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-0.7
git worktree add -d .worktree/backport-755-to-release-0.7 origin/release-0.7
cd .worktree/backport-755-to-release-0.7
git switch --create backport-755-to-release-0.7
git cherry-pick -x 88241fc359eb1f734bd2b139b68430a94b913022

@jumpstarter-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for release-0.7:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants