Skip to content
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

feat(models): Add model status message for k8s resource status #4834

Merged

Conversation

agrski
Copy link
Contributor

@agrski agrski commented May 9, 2023

What this PR does / why we need it:
This PR adds a message field for model statuses to indicate, at a high level, why a model is (not) ready. At present, models can only be ready or not ready, and may have a reason describing the underlying cause of that status. That reason is fine for humans, but is verbose and is not suitable for automated systems to parse easily.

This PR also updates the pipeline status fields to share the same semantics are for models:

  • Message = short-form description of the status, generally as a term from the Protobuf contracts.
  • Reason = free-form text describing the underlying cause of the status.

There are also a number of formatting changes to break long lines (>120 characters) and introduce some whitespace for logical grouping/separation.

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:

) {
modelStatus := schedulerAPI.ModelStatus_ModelFailed
if canRetry {
modelStatus = schedulerAPI.ModelStatus_ModelProgressing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ The equivalent pipeline code simply says the pipeline failed if there was any issue requesting it to be loaded. Which behaviour do we want -- indicate failure early, or be optimistic as transient glitches can happen?

Copy link
Contributor Author

@agrski agrski May 9, 2023

Choose a reason for hiding this comment

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

I've updated the pipeline status to not indicate failure from retryable errors: 8f16995

@agrski agrski force-pushed the add-model-status-message-for-k8s-resource-status branch from 558b0a6 to 8f16995 Compare May 9, 2023 13:24
@agrski agrski requested a review from ukclivecox May 9, 2023 13:24
@agrski agrski marked this pull request as ready for review May 9, 2023 13:24
@ukclivecox ukclivecox added the v2 label May 10, 2023
@agrski agrski merged commit 37cedf8 into SeldonIO:v2 May 10, 2023
@agrski agrski deleted the add-model-status-message-for-k8s-resource-status branch May 10, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants