-
Notifications
You must be signed in to change notification settings - Fork 186
[UX]: Make run status in UI and CLI easier to understand #2716
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
[UX]: Make run status in UI and CLI easier to understand #2716
Conversation
Commented ESLint (until warnigns are fixed)
def _get_error(termination_reason: Optional[JobTerminationReason]) -> Optional[str]: | ||
if termination_reason == JobTerminationReason.INSTANCE_UNREACHABLE: | ||
return "instance unreachable" | ||
elif termination_reason == JobTerminationReason.WAITING_INSTANCE_LIMIT_EXCEEDED: | ||
return "waiting instance limit exceeded" | ||
elif termination_reason == JobTerminationReason.VOLUME_ERROR: | ||
return "waiting runner limit exceeded" | ||
elif termination_reason == JobTerminationReason.GATEWAY_ERROR: | ||
return "gateway error" | ||
elif termination_reason == JobTerminationReason.SCALED_DOWN: | ||
return "scaled down" | ||
elif termination_reason == JobTerminationReason.INACTIVITY_DURATION_EXCEEDED: | ||
return "inactivity duration exceeded" | ||
elif termination_reason == JobTerminationReason.TERMINATED_DUE_TO_UTILIZATION_POLICY: | ||
return "utilization policy" | ||
elif termination_reason == JobTerminationReason.PORTS_BINDING_FAILED: | ||
return "ports binding failed" | ||
elif termination_reason == JobTerminationReason.CREATING_CONTAINER_ERROR: | ||
return "runner error" | ||
elif termination_reason == JobTerminationReason.EXECUTOR_ERROR: | ||
return "executor error" | ||
elif termination_reason == JobTerminationReason.MAX_DURATION_EXCEEDED: | ||
return "max duration exceeded" | ||
else: | ||
return None |
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.
Maybe define a mapping instead of a big if? Also I think we'll forget to update this when adding new termination reasons unless there is a test that checks all reasons are mapped.
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.
Fixed, also added tests
@@ -171,7 +166,12 @@ def apply_configuration( | |||
# We can attach to run multiple times if it goes from running to pending (retried). | |||
while True: | |||
with MultiItemStatus(f"Launching [code]{run.name}[/]...", console=console) as live: | |||
while not _is_ready_to_attach(run): |
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.
Breaks fix #2720
Fixes #2655
Status
column (both CLI and UI), show a human-readable status explaining the termination reason from the last job submission (ExampleFailed
->Exited (137)
;Failed
->No offers
, etc)Aborted
orNo offers
, use Yellow)Error
column (both CLI and UI), show a human-readable status only in case theStatus
information is not enough (e.g. do not show anything in case ofNo offers
orExited (137)
.Reservation
column into theInstance type
columnStatus
changes in the output ofdstack apply
in case the run has failed due toNo offers
orExited (X)