-
Notifications
You must be signed in to change notification settings - Fork 241
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
[Model Monitoring] Avoid failing on missing predictions TSDB table #5706
Conversation
[ML-6628](https://iguazio.atlassian.net/browse/ML-6628) Return an empty dataframe instead.
) | ||
except v3io_frames.ReadError as err: | ||
if "No TSDB schema file found" in str(err): | ||
return pd.DataFrame() |
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.
What is the /metrics
output on a new model endpoint?
If I follow correctly, it will always show the "invocations" metric, which is incorrect.
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.
It will return an empty dataframe, and hence the expected ModelEndpointMonitoringMetricNoData
.
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 refer to the read_prediction_metric_for_endpoint_if_exists
method, which is used in the /metrics
endpoint.
What is the /metrics
(not /metrics-values
) output in the mentioned case?
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.
You're right, that's an issue in development. I pushed a 1-liner commit to fix that.
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.
What is the
/metrics
(not/metrics-values
) output in the mentioned case?
👆
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.
This question remains unanswered - it was the primary motivation for this fix.
It should be ok now. Addressing it could reduce the review time for both sides.
ML-6628
Return an empty dataframe instead.
Fixes system test:
TestRecordResults::test_inference_feature_set