-
Notifications
You must be signed in to change notification settings - Fork 1
Description
The way alerts are set up and handled seems confusing to me and wanted to raise this ticket as a place to discuss and clarify the expected behaviour:
When you create a run, you can use the run.add_alert() method to create a new alert for this run. This implies to me that this alert only applies to this run, since other similar methods such as add_metrics(), add_event(), add_artifact() add things which are only stored under the run. However the alert is sort of set up and stored globally, and can be seen under the Alerts tab. That behaviour seems a bit confusing to me, but I can see how it would be beneficial to be able to add the same alert to multiple runs and compare whether it is triggered or not across these runs.
However if that is the intention, then it seems strange that there is no option to add an existing alert to a run. As far as I can tell, the only way to add alerts to a run is with add_alert(), which makes you define the whole alert again. Maybe this could be split into add_new_alert() (which basically does what add_alert() does now, raising an error if an alert with the same name already exists), and add_existing_alert(), which just takes the alert name and adds the existing definition to a run. This way you can reduce the number of times a user has to define the same thing, and can make sure that an alert is consistent across different runs.
Additionally, it looks like if you try to use add_alert() with a name which exists already, currently the code will ignore the parameters you try to pass in, and instead use the parameters stored under the existing alert. For example if on the first run I define I set up an outside range alert with the range between 4 and 6, and then on a subsequent run I edit these values to 9 and 10 but keep the name of the alert the same, the old alert definition is added. This seems like it will confuse the user (it definitely confused me), but on the other hand you may not want the user to be able to update the boundaries of an existing alert like this as then trying to compare the status of an alert between runs becomes impossible since the boundaries of the alert may have changed between runs.
@AbyAbraham21 @alahiff any opinions on this?