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

[Alerts] Verify entity and notifications exist before storing alert #5711

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

Yacouby
Copy link
Member

@Yacouby Yacouby commented Jun 4, 2024

provide more user-friendly error messages in case the user tries to store an alert without providing entity or at least one notification.

resolves:
https://iguazio.atlassian.net/browse/ML-6676

@@ -74,11 +74,17 @@ def validate_required_fields(self):
def to_dict(self, fields: list = None, exclude: list = None, strip: bool = False):
data = super().to_dict(self._dict_fields)

if self.entities is None:
Copy link
Member

Choose a reason for hiding this comment

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

I don't see an issue with the error but the way you use to_dict to parse the entities and notifications is not really the mlrun way. you should use _serialize_field see ModelObj._fields_to_serialize

@Yacouby Yacouby requested a review from alonmr June 5, 2024 15:06
@alonmr alonmr merged commit 244c829 into mlrun:development Jun 6, 2024
11 checks passed
@Yacouby Yacouby deleted the alert_verify branch June 6, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants