[Internal]: Replace Instance.termination_reason values with codes
#3187
Conversation
src/dstack/_internal/server/background/tasks/process_running_jobs.py
Outdated
Show resolved
Hide resolved
| # TODO: Migrate to EnumAsString(InstanceTerminationReason, 100) after enough releases to ensure backward compatibility | ||
| termination_reason: Mapped[Optional[str]] = mapped_column(String(4000)) |
There was a problem hiding this comment.
What happens after enough releases? Old instances continue to have non-enum values forever.
There was a problem hiding this comment.
Addressed by using InstanceTerminationReason straight away and converting old non-enum values to InstanceTerminationReason on reads
|
This PR is stale because it has been open for 14 days with no activity. |
|
This PR is stale because it has been open for 14 days with no activity. |
|
This PR was closed because it has been inactive for 7 days since being marked as stale. Please reopen the PR if it is still relevant. |
Use the `InstanceTerminationReason` enum as the type of `InstanceModel.termination_reason`. To handle old instances, automatically convert their legacy termination reason strings to relevant `InstanceTerminationReason` enum members on reads.
- Drop `starting_timeout`, use `provisioning_timeout` instead. These two termination reasons had similar semantics. The difference was that `starting_timeout` was used for cloud instances and `provisioning_timeout` for SSH. - Rename `termination_deadline` -> `unreachable`. The old name was based on implementation details rather than semantics, so it was not very informative to the user. - For `no_offers`, add a termination reason message to highlight the difference between failing to find offers and failing to provision offers.
To avoid extra testing and changes, limit the scope of the PR to instance termination reasons, as per the PR name. The introduction of `JobTerminationReason.NO_BALANCE` can be done in a separate PR. Its implementation may need to be updated to also use `JobTerminationReason.NO_BALANCE` on jobs with statuses other than `running`.
|
Updated the PR, addressed the pending review comment, and added small changes to termination reason naming. Also slightly reduced the scope of the PR. See commit messages for details and motivation |
Fixes #3182