Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Create multiple actions on alert #302

Merged
merged 11 commits into from
Jan 13, 2025

Conversation

brandon-groundlight
Copy link
Collaborator

A followup for later, we will completely replace 'rule' with 'alert'. Once renamed we can move alert functions into the standard client. This requires a bit of backend modelling updates.

I'm not a huge fan of root as the attribute of ActionList, but that matches the style of datamodel-codegen.

@@ -37,7 +37,8 @@ def is_valid_display_result(result: Any) -> bool:
and not isinstance(result, MultiClassificationResult)
):
return False
if not is_valid_display_label(result.label):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

separate change that needed to get in to support different result types

Copy link
Contributor

@f-wright f-wright left a comment

Choose a reason for hiding this comment

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

LGTM! I left a few suggestions to remove references to rules in the new create_alert function. If we're switching to use exclusively "alert", that seems like it could become confusing.


:param channel: The notification channel to use. One of "EMAIL" or "TEXT"
:param recipient: The email address or phone number to send notifications to
:param include_image: Whether to include the triggering image in notifications
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:param include_image: Whether to include the triggering image in notifications
:param include_image: Whether to include the triggering image in alerts

such as when a detector's prediction changes or maintains a particular state.

.. note::
Currently, only binary mode detectors (YES/NO answers) are supported for notification rules.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Currently, only binary mode detectors (YES/NO answers) are supported for notification rules.
Currently, only binary mode detectors (YES/NO answers) are supported for alerts.


:param detector: The detector ID or Detector object to add the rule to
:param name: A unique name to identify this rule
:param enabled: Whether the rule should be active when created (default True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:param enabled: Whether the rule should be active when created (default True)
:param enabled: Whether the alert should be active when created (default True)

)

:param detector: The detector ID or Detector object to add the rule to
:param name: A unique name to identify this rule
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:param name: A unique name to identify this rule
:param name: A unique name to identify this alert


gl = ExperimentalApi()

# Create an action for a rule
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Create an action for a rule
# Create an action

@brandon-groundlight brandon-groundlight merged commit c849543 into main Jan 13, 2025
8 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants