)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"5a76c5fb4f0b5ce0306df5c6eb7d07e57075d1f8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"01e257a1_3e1282d0","updated":"2026-11-08 05:56:06.000000000","message":"IIUC the proposed solution doesn\u0027t fix existing alarms unless these are explicitly updated or re-created by users. Can we update the evaluator logic so that the existing alarms are fixed without update by users ?\n\nTechnically we can add db migration script to fill the column, though backporting it may be a bit tricky.","commit_id":"42118fe7738c3aa776748010d9e0f9d8865cbf73"},{"author":{"_account_id":39493,"name":"Daniel Gines","display_name":"Daniel Gines","email":"dangines@gmail.com","username":"danielgines"},"change_message_id":"b5e49eef3608d7c1b3c23a7664d8e9e830a3be98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2bd65bfd_d1ef16f5","in_reply_to":"01e257a1_3e1282d0","updated":"2026-11-08 20:45:11.000000000","message":"Good point. PS2 also updates the evaluator: _assigned_alarms() selects the enabled alarms with a NULL evaluate_timestamp as well (a second query, evaluate_timestamp IS NULL), and the field is filled in when the alarm is evaluated. Without coordination this is done by the existing compare-and-swap update (filter_by(evaluate_timestamp\u003dNone) is rendered as IS NULL, covered by a new storage test); with coordination, by a conditional update of the alarms whose evaluate_timestamp is NULL. So the existing alarms are fixed by the next evaluation cycle, without any update by their owners, and this works in the stable branches too.\n\nI proposed the database migration as a separate change, https://review.opendev.org/c/openstack/aodh/+/1014711, which sets the NULL values, and also the future ones that the API accepted until now, to the time of the upgrade. It is kept separate so that this change can be backported without it.","commit_id":"42118fe7738c3aa776748010d9e0f9d8865cbf73"}],"aodh/api/controllers/v2/alarms.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"5a76c5fb4f0b5ce0306df5c6eb7d07e57075d1f8","unresolved":true,"context_lines":[{"line_number":664,"context_line":"        # evaluate_timestamp is managed by the evaluator: ignore the value"},{"line_number":665,"context_line":"        # sent by the client and keep the current one. An alarm with a NULL"},{"line_number":666,"context_line":"        # (or future) evaluate_timestamp is never selected for evaluation."},{"line_number":667,"context_line":"        data.evaluate_timestamp \u003d alarm_in.evaluate_timestamp or now"},{"line_number":668,"context_line":"        if alarm_in.state !\u003d data.state:"},{"line_number":669,"context_line":"            data.state_timestamp \u003d now"},{"line_number":670,"context_line":"            data.state_reason \u003d ALARM_REASON_MANUAL"}],"source_content_type":"text/x-python","patch_set":1,"id":"cde99a22_49f9e92c","line":667,"range":{"start_line":667,"start_character":13,"end_line":667,"end_character":31},"updated":"2026-11-08 05:56:06.000000000","message":"I wonder why create/update api ever exposes this internal field. We may probably want to hide this or return error in case the field is explicitly set by a user.(but this can be done separately)","commit_id":"42118fe7738c3aa776748010d9e0f9d8865cbf73"},{"author":{"_account_id":39493,"name":"Daniel Gines","display_name":"Daniel Gines","email":"dangines@gmail.com","username":"danielgines"},"change_message_id":"b5e49eef3608d7c1b3c23a7664d8e9e830a3be98","unresolved":false,"context_lines":[{"line_number":664,"context_line":"        # evaluate_timestamp is managed by the evaluator: ignore the value"},{"line_number":665,"context_line":"        # sent by the client and keep the current one. An alarm with a NULL"},{"line_number":666,"context_line":"        # (or future) evaluate_timestamp is never selected for evaluation."},{"line_number":667,"context_line":"        data.evaluate_timestamp \u003d alarm_in.evaluate_timestamp or now"},{"line_number":668,"context_line":"        if alarm_in.state !\u003d data.state:"},{"line_number":669,"context_line":"            data.state_timestamp \u003d now"},{"line_number":670,"context_line":"            data.state_reason \u003d ALARM_REASON_MANUAL"}],"source_content_type":"text/x-python","patch_set":1,"id":"68cd2299_76df986b","line":667,"in_reply_to":"cde99a22_49f9e92c","updated":"2026-11-08 20:45:11.000000000","message":"evaluate_timestamp is in the block of attributes documented as \"ignored in the PUT or POST operations, but are filled in for GET\", together with timestamp and state_timestamp, so PS2 makes the API honour that: the value sent by the client is ignored on POST and PUT, the stored value is kept on PUT (or set to the current time if it is NULL), and it is no longer recorded as a rule change in the history.\n\nI did not make the API return an error when the field is set, because python-aodhclient\u0027s AlarmManager.update() does a GET, merges the changes and PUTs the whole alarm back, so the body of every \"openstack alarm update\" contains the evaluate_timestamp returned by GET (Heat uses the same client).\n\nI agree that the field should not be returned at all, and proposed it in a follow-up change on top of this one, https://review.opendev.org/c/openstack/aodh/+/1014710 (bug 2173707): the field is no longer returned by GET, by the responses of POST and PUT, and by the alarm query API, and is no longer recorded in the creation events of the history. The API keeps accepting and ignoring it, so old clients and saved alarm bodies keep working. It depends on this change, because without it a PUT built from a GET without the field would store NULL again.","commit_id":"42118fe7738c3aa776748010d9e0f9d8865cbf73"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"5a76c5fb4f0b5ce0306df5c6eb7d07e57075d1f8","unresolved":true,"context_lines":[{"line_number":840,"context_line":"        data.timestamp \u003d now"},{"line_number":841,"context_line":"        data.state_timestamp \u003d now"},{"line_number":842,"context_line":"        data.state_reason \u003d ALARM_REASON_DEFAULT"},{"line_number":843,"context_line":"        data.evaluate_timestamp \u003d now"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        ALARMS_RULES[data.type].plugin.create_hook(data)"},{"line_number":846,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"02b7be7c_12aa0e37","line":843,"range":{"start_line":843,"start_character":8,"end_line":843,"end_character":37},"updated":"2026-11-08 05:56:06.000000000","message":"So does this mean the same problem exists in creation API ?","commit_id":"42118fe7738c3aa776748010d9e0f9d8865cbf73"},{"author":{"_account_id":39493,"name":"Daniel Gines","display_name":"Daniel Gines","email":"dangines@gmail.com","username":"danielgines"},"change_message_id":"b5e49eef3608d7c1b3c23a7664d8e9e830a3be98","unresolved":false,"context_lines":[{"line_number":840,"context_line":"        data.timestamp \u003d now"},{"line_number":841,"context_line":"        data.state_timestamp \u003d now"},{"line_number":842,"context_line":"        data.state_reason \u003d ALARM_REASON_DEFAULT"},{"line_number":843,"context_line":"        data.evaluate_timestamp \u003d now"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        ALARMS_RULES[data.type].plugin.create_hook(data)"},{"line_number":846,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"eb720d1d_121359a1","line":843,"in_reply_to":"02b7be7c_12aa0e37","updated":"2026-11-08 20:45:11.000000000","message":"Partly. POST never stores NULL: when the client does not send the field, or sends null, the storage model gets None, the ORM leaves None values out of the INSERT and the column default (timeutils.utcnow) applies. But POST stored the value sent by the client as is: on master, a POST with \"evaluate_timestamp\": \"2099-01-01T00:00:00\" stores 2099-01-01, and the alarm is not evaluated before that date. PS2 sets it to the creation time, like timestamp and state_timestamp. The new test_post_alarm_ignores_evaluate_timestamp fails on master with \"datetime.datetime(2099, 1, 1, 0, 0) not less than or equal to datetime.datetime(2026, 11, 8, ...)\".","commit_id":"42118fe7738c3aa776748010d9e0f9d8865cbf73"}],"aodh/evaluator/__init__.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a025cd8aafb967fbc2ee4d8f5e4f4db3e898a1f7","unresolved":true,"context_lines":[{"line_number":285,"context_line":"                    \u0027Alarm %s has been already handled by another evaluator\u0027,"},{"line_number":286,"context_line":"                    alarm.alarm_id"},{"line_number":287,"context_line":"                )"},{"line_number":288,"context_line":"                return"},{"line_number":289,"context_line":"        elif alarm.evaluate_timestamp is None:"},{"line_number":290,"context_line":"            self.storage_conn.conditional_update("},{"line_number":291,"context_line":"                sql_models.Alarm,"},{"line_number":292,"context_line":"                {\u0027evaluate_timestamp\u0027: timeutils.utcnow()},"},{"line_number":293,"context_line":"                {\u0027alarm_id\u0027: alarm.alarm_id, \u0027evaluate_timestamp\u0027: None},"},{"line_number":294,"context_line":"            )"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        LOG.debug(\u0027Evaluating alarm %s\u0027, alarm.alarm_id)"},{"line_number":297,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a6310b41_e6740869","line":294,"range":{"start_line":288,"start_character":22,"end_line":294,"end_character":13},"updated":"2026-11-09 06:33:52.000000000","message":"Could you add reference to bug 2173654 here to record the reason we need this.","commit_id":"c3b12109c6a055b2adab0a3ef2dca1253345642d"},{"author":{"_account_id":39493,"name":"Daniel Gines","display_name":"Daniel Gines","email":"dangines@gmail.com","username":"danielgines"},"change_message_id":"b6d317a562258a4fc14b4e4a2f248f61cbd8a79b","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                sql_models.Alarm,"},{"line_number":292,"context_line":"                {\u0027evaluate_timestamp\u0027: timeutils.utcnow()},"},{"line_number":293,"context_line":"                {\u0027alarm_id\u0027: alarm.alarm_id, \u0027evaluate_timestamp\u0027: None},"},{"line_number":294,"context_line":"            )"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        LOG.debug(\u0027Evaluating alarm %s\u0027, alarm.alarm_id)"},{"line_number":297,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"253a1d81_1d9c8262","line":294,"in_reply_to":"a6310b41_e6740869","updated":"2026-11-09 18:13:07.000000000","message":"Done in PS3: added a NOTE with the reference to bug #2173654. I kept this handling in this change, because a backport of this change without the migration of 1014711 still needs it; 1014711 removes it on master.","commit_id":"c3b12109c6a055b2adab0a3ef2dca1253345642d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a025cd8aafb967fbc2ee4d8f5e4f4db3e898a1f7","unresolved":true,"context_lines":[{"line_number":302,"context_line":"    def _assigned_alarms(self):"},{"line_number":303,"context_line":"        before \u003d (timeutils.utcnow() - datetime.timedelta("},{"line_number":304,"context_line":"            seconds\u003dself.conf.evaluator.evaluation_interval / 2))"},{"line_number":305,"context_line":"        selected \u003d {}"},{"line_number":306,"context_line":"        # NOTE: also select the alarms without evaluate_timestamp, which"},{"line_number":307,"context_line":"        # the API stored when an alarm was updated without this field."},{"line_number":308,"context_line":"        for evaluate_timestamp in ({\u0027lt\u0027: before}, None):"},{"line_number":309,"context_line":"            for alarm in self.storage_conn.get_alarms("},{"line_number":310,"context_line":"                    enabled\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"608d1dbc_a8b9e35e","line":307,"range":{"start_line":305,"start_character":21,"end_line":307,"end_character":70},"updated":"2026-11-09 06:33:52.000000000","message":"Could you add reference to bug 2173654 here to record the reason we need this.","commit_id":"c3b12109c6a055b2adab0a3ef2dca1253345642d"},{"author":{"_account_id":39493,"name":"Daniel Gines","display_name":"Daniel Gines","email":"dangines@gmail.com","username":"danielgines"},"change_message_id":"b6d317a562258a4fc14b4e4a2f248f61cbd8a79b","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            seconds\u003dself.conf.evaluator.evaluation_interval / 2))"},{"line_number":305,"context_line":"        selected \u003d {}"},{"line_number":306,"context_line":"        # NOTE: also select the alarms without evaluate_timestamp, which"},{"line_number":307,"context_line":"        # the API stored when an alarm was updated without this field."},{"line_number":308,"context_line":"        for evaluate_timestamp in ({\u0027lt\u0027: before}, None):"},{"line_number":309,"context_line":"            for alarm in self.storage_conn.get_alarms("},{"line_number":310,"context_line":"                    enabled\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ce99e2d_6821a94a","line":307,"in_reply_to":"608d1dbc_a8b9e35e","updated":"2026-11-09 18:13:07.000000000","message":"Done in PS3, same here.","commit_id":"c3b12109c6a055b2adab0a3ef2dca1253345642d"}]}