-
Notifications
You must be signed in to change notification settings - Fork 13
Introduce fail fast mode #1046
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
Introduce fail fast mode #1046
Conversation
This comment has been minimized.
This comment has been minimized.
8bd879f to
7c8f7f2
Compare
want to use from fail fast mode validator
|
Sorry for waiting for you. I'll review this PR by today. |
launchable/utils/fail_fast_mode.py
Outdated
| sys.exit(1) | ||
|
|
||
|
|
||
| class FailFastModeValidator: |
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.
I feel like this API is not Pythonic, so how about something like this: #1058
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.
Turn too many arguments into a data class.
Rename _print_errors to exit_if_errors.
OK, I'll fix them
Stop splitting the processing into the method, since it's harder to understand it.
This is intentional.
Right now, the validation logic is shared (with the record tests and the subset commands), so it may look like things are jut split for no reason.
However, the motivation is to make it easier to add method-specific validations in the future
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.
Got it. Thank you! I wanted to use @DataClass decorator, but it's only supported from Python 3.7 and above 😢
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.
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
Adds support for “fail fast mode” to the CLI so CI pipelines will stop immediately on certain API failures.
- Introduces
fail_fast_mode_validateand helpers to error out when invalid options are used in fast-fail mode. - Fetches workspace state (
isFailFastMode) via the newLaunchableClient.is_fail_fast_mode(), caches it, and wires it into commands. - Replaces many
click.echo(...), sys.exitpatterns withwarning_and_exit_if_fail_fast_modeto unify behavior.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| launchable/utils/fail_fast_mode.py | Implements global cache and validation for fail-fast. |
| launchable/utils/launchable_client.py | Adds is_fail_fast_mode() API and per-client cache. |
| launchable/commands/* | Wires in fail-fast checks and replaces warnings/exits. |
| tests/utils/test_fail_fast_mode.py | Adds tests for validation under fast-fail on/off. |
| tests/commands/**/* | Updates request‐call indices to account for state fetch. |
| launchable/utils/tracking.py | Switches to shared Command enum for tracking. |
launchable/utils/fail_fast_mode.py
Outdated
| return False | ||
|
|
||
|
|
||
| def warning_and_exit_if_fail_fast_mode(message: str): |
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.
nit: warning is a noun, so I guess the verb, warn is better?
| def warning_and_exit_if_fail_fast_mode(message: str): | |
| def warn_and_exit_if_fail_fast_mode(message: str): |
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.
based on feedback
|



The cli a.k.a launchable command is built for CI pipelines, so it doesn't stop pipelines when API calls fail.
But this means pipelines keep running even with wrong settings, making it hard to notice problems.
To fix this, we will make the pipeline stop when the workspace is in a special state.
We call this special state "fail fast mode".
This PR adds fail fast mode support to the CLI.