-
Notifications
You must be signed in to change notification settings - Fork 665
Replace ImmediateExit with sysexit_with_message for direct exits #4534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Replace MoleculeError with ImmediateExit for validation errors that should present cleanly to users in normal mode while retaining full traceback information when --debug flag is enabled. Changes: - Config validation errors (schema, driver, interpolation) - Scenario resolution errors - Init command validation (directory exists, template missing) - Login host validation errors - Update corresponding test assertions to check exception message directly The existing click_command_ex decorator already provides debug-aware exception handling for ImmediateExit, showing clean messages in normal mode and full tracebacks with --debug.
This comment was marked as outdated.
This comment was marked as outdated.
- Replace raise ImmediateExit() calls with sysexit_with_message() for user-facing validation errors - Simplifies error handling by using direct sys.exit() instead of exception propagation - Maintains same logging behavior (logger.critical) and exit codes - Update tests to catch SystemExit instead of ImmediateExit where functions now call sysexit_with_message - Keep ImmediateExit handling in Click decorators for integration compatibility - Update docstrings to reflect direct exit behavior instead of exception raising This change provides cleaner error flow while preserving existing user experience.
This comment was marked as outdated.
This comment was marked as outdated.
Remove 'Raises' sections from functions that now use sysexit_with_message() instead of raising exceptions. These functions now exit directly with sys.exit() rather than propagating exceptions.
Co-authored-by: Kate Case <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces exception-based error handling (ImmediateExit) with direct system exits (sysexit_with_message) for user-facing validation errors to improve UX by eliminating Python tracebacks.
- Replace
raise ImmediateExit()calls withsysexit_with_message()for validation errors - Update test expectations from catching
ImmediateExittoSystemExit - Remove unused imports and update docstrings to reflect direct exit behavior
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/molecule/scenarios.py | Replace exception-based scenario validation with direct exit |
| src/molecule/config.py | Replace config validation exceptions with direct exits |
| src/molecule/command/login.py | Replace hostname validation exceptions with direct exits |
| src/molecule/utils/util.py | Fix sysexit_with_message to use sys.exit directly |
| src/molecule/verifier/testinfra.py | Replace test failure exception with direct exit |
| src/molecule/provisioner/ansible.py | Replace inventory validation exception with direct exit |
| src/molecule/dependency/base.py | Replace dependency installation failure exception with direct exit |
| src/molecule/command/init/scenario.py | Replace scenario creation validation exceptions with direct exits |
| src/molecule/command/init/base.py | Replace template validation exception with direct exit |
| src/molecule/command/base.py | Replace scenario execution exceptions with direct exits |
| src/molecule/command/list.py | Replace list command exception handling with direct exit |
| tests/unit/*.py | Update test expectations to catch SystemExit instead of MoleculeError/ImmediateExit |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Co-authored-by: Copilot <[email protected]>
User-facing validation errors currently display full Python tracebacks, creating poor UX for common scenarios like invalid molecule.yml configuration or missing scenarios. Fixes #4532 Replace `raise ImmediateExit()` calls with `sysexit_with_message()` for user-facing validation errors, simplifying error handling by using direct `sys.exit()` instead of exception propagation. Source code: - Replace `ImmediateExit` exceptions with `sysexit_with_message()` calls in validation functions - Maintain existing logging behavior (`logger.critical`) and exit codes - Preserve `ImmediateExit` handling in Click decorators for integration compatibility - Update docstrings to reflect direct exit behavior Tests: - Update tests to catch `SystemExit` instead of `ImmediateExit` - Modify test assertions to verify logged messages via `caplog` instead of exception attributes This change moves from an exception-based exit pattern to a direct exit pattern for user-facing validation errors. The `sysexit_with_message()` function logs the error message using `logger.critical()` and calls `sys.exit()` directly, providing the same user experience with cleaner code flow. Functions affected include config validation, scenario verification, host selection validation, and dependency installation error handling. Before: ``` $ molecule login CRITICAL Failed to validate /path/to/molecule.yml <full Python traceback> molecule.exceptions.MoleculeError ``` After: ``` $ molecule login CRITICAL Failed to validate /path/to/molecule.yml ``` This provides clean error messages without tracebacks while maintaining the same logging behavior and exit codes. --------- Co-authored-by: Kate Case <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <[email protected]>
Problem
User-facing validation errors currently display full Python tracebacks, creating poor UX for common scenarios like invalid molecule.yml configuration or missing scenarios.
Fixes #4532
Solution
Replace
raise ImmediateExit()calls withsysexit_with_message()for user-facing validation errors, simplifying error handling by using directsys.exit()instead of exception propagation.Changes
Source code:
ImmediateExitexceptions withsysexit_with_message()calls in validation functionslogger.critical) and exit codesImmediateExithandling in Click decorators for integration compatibilityTests:
SystemExitinstead ofImmediateExitcaploginstead of exception attributesTechnical Details
This change moves from an exception-based exit pattern to a direct exit pattern for user-facing validation errors. The
sysexit_with_message()function logs the error message usinglogger.critical()and callssys.exit()directly, providing the same user experience with cleaner code flow.Functions affected include config validation, scenario verification, host selection validation, and dependency installation error handling.
Behavior
Before:
After:
This provides clean error messages without tracebacks while maintaining the same logging behavior and exit codes.