)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"e7b6f66f9e55ffe5163d7f3bbab4e43d19b0fa86","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     d23c18f9 (Add AuditPipeline lifecycle notifications)"},{"line_number":2,"context_line":"Author:     Douglas Viroel \u003cviroel@gmail.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2026-08-05 09:54:13 -0300"},{"line_number":4,"context_line":"Commit:     Douglas Viroel \u003cviroel@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"676c5607_0e27237d","line":1,"updated":"2026-08-19 23:20:41.000000000","message":"The commit message bullet reads \u0027WeightedSchedulingMixin (planner/common.py): extracts the weight-based DAG scheduling logic...\u0027. No watcher/decision_engine/planner/common.py exists in the tree; the mixin was added to watcher/decision_engine/planner/weight.py, and no planner/common.py file was created in this change.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Impact**: Future readers searching the history for \u0027planner/common.py\u0027 will not find the module, making the refactor harder to trace; the meaning remains otherwise clear.\n\n**Recommendation**:\nAmend the commit message bullet to reference watcher/decision_engine/planner/weight.py (where WeightedSchedulingMixin now lives) before merge.","commit_id":"8f4f596cd1e0d59c5cc8685e324d63f4a59861ff"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e06217b1bd0c05ed2566c690a99dd741392cd53e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"13391104_d1ddb446","updated":"2026-09-03 16:34:55.000000000","message":"almost +2, I think we can merge this. The only remaining change is decide what to do with global efficacies in audit pipelines","commit_id":"231825c54173b1e36380c7f9fb305bac35d02aa8"}],"setup.cfg":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fb021810592f741ff38f54a26f893c56277cfff3","unresolved":true,"context_lines":[{"line_number":1,"context_line":"[metadata]"},{"line_number":2,"context_line":"name \u003d python-watcher"},{"line_number":3,"context_line":"summary \u003d OpenStack Watcher provides a flexible and scalable resource optimization service for multi-tenant OpenStack-based clouds."},{"line_number":4,"context_line":"description_file \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":14,"id":"5b176b2b_e077873b","line":1,"updated":"2026-09-01 17:06:48.000000000","message":"sign i tough we have moved watcher to use pyproject.tomle fo rthis already \n\nwe really shoudl be using that last cycle as setup.cfg and setup.py shoudl eb remvoed in the next relseae or two.","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fb021810592f741ff38f54a26f893c56277cfff3","unresolved":true,"context_lines":[{"line_number":107,"context_line":"    weight \u003d watcher.decision_engine.planner.weight:WeightPlanner"},{"line_number":108,"context_line":"    workload_stabilization  \u003d watcher.decision_engine.planner.workload_stabilization:WorkloadStabilizationPlanner"},{"line_number":109,"context_line":"    node_resource_consolidation  \u003d watcher.decision_engine.planner.node_resource_consolidation:NodeResourceConsolidationPlanner"},{"line_number":110,"context_line":"    pipeline \u003d watcher.decision_engine.planner.pipeline:PipelinePlanner"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"watcher_cluster_data_model_collectors \u003d"},{"line_number":113,"context_line":"    compute \u003d watcher.decision_engine.model.collector.nova:NovaClusterDataModelCollector"}],"source_content_type":"text/x-ttcn-cfg","patch_set":14,"id":"1f4c1516_b2dd1f96","line":110,"updated":"2026-09-01 17:06:48.000000000","message":"this is correct but i think im goint to create a patch to mvoe watcher to using pyproject.toml for this\n\ni tought i already had submitted one but maybe that was for a diffetn proejct","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58d83ae47d2804769f8a61e0cf2721afc47471db","unresolved":true,"context_lines":[{"line_number":107,"context_line":"    weight \u003d watcher.decision_engine.planner.weight:WeightPlanner"},{"line_number":108,"context_line":"    workload_stabilization  \u003d watcher.decision_engine.planner.workload_stabilization:WorkloadStabilizationPlanner"},{"line_number":109,"context_line":"    node_resource_consolidation  \u003d watcher.decision_engine.planner.node_resource_consolidation:NodeResourceConsolidationPlanner"},{"line_number":110,"context_line":"    pipeline \u003d watcher.decision_engine.planner.pipeline:PipelinePlanner"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"watcher_cluster_data_model_collectors \u003d"},{"line_number":113,"context_line":"    compute \u003d watcher.decision_engine.model.collector.nova:NovaClusterDataModelCollector"}],"source_content_type":"text/x-ttcn-cfg","patch_set":14,"id":"1fd5f819_7d52d310","line":110,"in_reply_to":"1f4c1516_b2dd1f96","updated":"2026-09-01 17:07:59.000000000","message":"nope we jsut didnt merge it at the start of the cycle\n\nhttps://review.opendev.org/c/openstack/watcher/+/981604","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"76aee911a130563bf70510f6db51e46aaa558010","unresolved":false,"context_lines":[{"line_number":107,"context_line":"    weight \u003d watcher.decision_engine.planner.weight:WeightPlanner"},{"line_number":108,"context_line":"    workload_stabilization  \u003d watcher.decision_engine.planner.workload_stabilization:WorkloadStabilizationPlanner"},{"line_number":109,"context_line":"    node_resource_consolidation  \u003d watcher.decision_engine.planner.node_resource_consolidation:NodeResourceConsolidationPlanner"},{"line_number":110,"context_line":"    pipeline \u003d watcher.decision_engine.planner.pipeline:PipelinePlanner"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"watcher_cluster_data_model_collectors \u003d"},{"line_number":113,"context_line":"    compute \u003d watcher.decision_engine.model.collector.nova:NovaClusterDataModelCollector"}],"source_content_type":"text/x-ttcn-cfg","patch_set":14,"id":"d0d60430_0b4f5c61","line":110,"in_reply_to":"1fd5f819_7d52d310","updated":"2026-09-02 15:49:35.000000000","message":"Done","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"}],"watcher/decision_engine/audit/pipeline.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c337a1e72c74907ad5beafaeac7d81029973eea0","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            # State already CANCELLED in DB"},{"line_number":162,"context_line":"            pass"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        except Exception as e:"},{"line_number":165,"context_line":"            LOG.exception(\"Pipeline %s failed: %s\", pipeline.uuid, e)"},{"line_number":166,"context_line":"            notifications.audit_pipeline.send_action_notification("},{"line_number":167,"context_line":"                context,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bfef014e_fc956454","line":164,"updated":"2026-08-06 19:24:16.000000000","message":"pre_execute raises exception.AuditPipelineCancelled when the pipeline is already CANCELLED, but execute() does not catch this exception explicitly. It falls through to the broad \u0027except Exception\u0027 handler which sets state to FAILED and sends an ERROR notification. This is inconsistent with the an...\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: A pipeline that was already cancelled by an administrator or concurrent process will be transitioned to FAILED state instead of remaining CANCELLED. This produces misleading status, incorrect ERROR notifications, and confuses operators who expect the pipeline to remain in its cancelled state.\n\n**Priority**: Before merge\n**Why This Matters**: A pipeline that was already cancelled by an administrator or concurrent process will be transitioned to FAILED state instead of remaining CANCELLED. This produces misleading status, incorrect ERROR notifications, and confuses operators who expect the pipeline to remain in its cancelled state.\n\n**Recommendation**:\nAdd an explicit \u0027except exception.AuditPipelineCancelled\u0027 handler before the \u0027except Exception\u0027 handler in execute(), mirroring the pattern in base.py\u0027s AuditHandler for AuditCancelled. For example: \u0027except exception.AuditPipelineCancelled: LOG.info(\"Pipeline %s was already cancelled\", pipeline.uuid)\u0027","commit_id":"811bd92cc4b9dab05660464ccfe2e143e2ef2d62"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c456c8681bc5769da9535c4c2d21b01ba25cb1aa","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":70,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":71,"context_line":"        )"},{"line_number":72,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":73,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":2,"id":"9cd8077b_0ef91c4d","line":72,"updated":"2026-08-07 20:09:52.000000000","message":"pre_execute checks fresh.state \u003d\u003d objects.audit.State.CANCELLED, using the State class from the Audit object rather than the AuditPipeline object. While both currently resolve to the string \u0027CANCELLED\u0027, referencing the wrong model\u0027s State enum is a semantic mismatch that will silently break if th...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Currently functionally equivalent since both resolve to \u0027CANCELLED\u0027. If audit.State.CANCELLED and audit_pipeline.State.CANCELLED ever diverge (e.g., different lifecycle requirements), the cancellation check in pre_execute would silently stop working.\n\n**Recommendation**:\nChange line 72 to use objects.audit_pipeline.State.CANCELLED for consistency with do_execute (line 102) and all other state references in the same file.","commit_id":"5000eb5318e616f8ecebd2b3cd8f9f8ae856cc34"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c456c8681bc5769da9535c4c2d21b01ba25cb1aa","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                phase\u003dwfields.NotificationPhase.END,"},{"line_number":153,"context_line":"            )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"        except exception.ActionPlanIsOngoing as e:"},{"line_number":156,"context_line":"            LOG.warning(\"Pipeline %s cancelled: %s\", pipeline.uuid, e)"},{"line_number":157,"context_line":"            pipeline.state \u003d objects.audit_pipeline.State.CANCELLED"},{"line_number":158,"context_line":"            pipeline.save()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ab02bd3_6e2b0642","line":155,"updated":"2026-08-07 20:09:52.000000000","message":"pre_execute() raises exception.AuditPipelineCancelled when it detects the pipeline is already in CANCELLED state. However, execute() only catches _PipelineCancelled (the internal sentinel) for the pass-through case. AuditPipelineCancelled is a WatcherException subclass, not _PipelineCancelled, so...\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: A pipeline cancelled by an operator before execution begins will have its state overwritten from CANCELLED to FAILED. This misleads operators about why the pipeline did not run, pollutes monitoring/dashboards with spurious FAILED pipelines, and may trigger failure-handling logic (notifications, r...\n\n**Priority**: Before merge\n**Why This Matters**: A pipeline cancelled by an operator before execution begins will have its state overwritten from CANCELLED to FAILED. This misleads operators about why the pipeline did not run, pollutes monitoring/dashboards with spurious FAILED pipelines, and may trigger failure-handling logic (notifications, r...\n\n**Recommendation**:\nAdd an explicit except clause for exception.AuditPipelineCancelled (or catch it alongside _PipelineCancelled) that preserves the CANCELLED state without setting FAILED. For example: except (exception.AuditPipelineCancelled, _PipelineCancelled): pass since the state is already CANCELLED in the database.","commit_id":"5000eb5318e616f8ecebd2b3cd8f9f8ae856cc34"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c456c8681bc5769da9535c4c2d21b01ba25cb1aa","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            pipeline.state \u003d objects.audit_pipeline.State.CANCELLED"},{"line_number":158,"context_line":"            pipeline.save()"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        except _PipelineCancelled:"},{"line_number":161,"context_line":"            # State already CANCELLED in DB"},{"line_number":162,"context_line":"            pass"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"37e067ad_bd02c393","line":160,"updated":"2026-08-07 20:09:52.000000000","message":"When _PipelineCancelled is raised from do_execute() during stage iteration, the EXECUTION START notification has already been sent, but neither an END nor an ERROR notification is emitted for the cancellation. Notification consumers will see an unpaired START event with no terminal signal.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: External systems consuming audit_pipeline notifications (e.g., telemetry, event processing, monitoring dashboards) will see a START without a matching END or ERROR for cancelled pipelines, making it impossible to distinguish a still-running pipeline from a cancelled one.\n\n**Suggestion**:\nIn the _PipelineCancelled except block, emit a terminal notification (either END or a dedicated CANCELLED/error notification) so consumers receive a lifecycle-complete event sequence. For example, send an ERROR or END notification with an appropriate priority.","commit_id":"5000eb5318e616f8ecebd2b3cd8f9f8ae856cc34"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4467278db8f5d4663bbbd79052588cb6781d55d9","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":70,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":71,"context_line":"        )"},{"line_number":72,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":73,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":3,"id":"1051aac8_39492062","line":72,"updated":"2026-08-12 13:17:55.000000000","message":"In pre_execute(), the cancellation check compares fresh.state against objects.audit.State.CANCELLED (the Audit enum) rather than objects.audit_pipeline.State.CANCELLED (the AuditPipeline enum). While both resolve to the same string \u0027CANCELLED\u0027 today, using the wrong enum is semantically incorrect...\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: No functional impact today since both enums use the string \u0027CANCELLED\u0027. Could cause subtle bugs if the enums diverge in the future and makes the code harder to read.\n\n**Recommendation**:\nChange objects.audit.State.CANCELLED to objects.audit_pipeline.State.CANCELLED on line 72 for consistency with the rest of the file.","commit_id":"481077a22ff81d8a1a93d25e2eaab23c60d63ada"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4467278db8f5d4663bbbd79052588cb6781d55d9","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                phase\u003dwfields.NotificationPhase.END,"},{"line_number":153,"context_line":"            )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"        except exception.ActionPlanIsOngoing as e:"},{"line_number":156,"context_line":"            LOG.warning(\"Pipeline %s cancelled: %s\", pipeline.uuid, e)"},{"line_number":157,"context_line":"            pipeline.state \u003d objects.audit_pipeline.State.CANCELLED"},{"line_number":158,"context_line":"            pipeline.save()"}],"source_content_type":"text/x-python","patch_set":3,"id":"f344d6d8_dfdba615","line":155,"updated":"2026-08-12 13:17:55.000000000","message":"When pre_execute() detects an already-CANCELLED pipeline it raises exception.AuditPipelineCancelled. However, execute() only catches ActionPlanIsOngoing, _PipelineCancelled, and generic Exception. AuditPipelineCancelled is a WatcherException subclass (not ActionPlanIsOngoing), so it falls through...\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: A pipeline cancelled externally before execution starts will have its state incorrectly overwritten from CANCELLED to FAILED, producing misleading status and notifications for operators.\n\n**Priority**: Before merge\n**Why This Matters**: A pipeline cancelled externally before execution starts will have its state incorrectly overwritten from CANCELLED to FAILED, producing misleading status and notifications for operators.\n\n**Recommendation**:\nAdd an explicit except clause for exception.AuditPipelineCancelled in execute() (before the generic except Exception) that does not change the pipeline state, similar to how the original AuditHandler catches exception.AuditCancelled. Fix the test assertion and docstring to verify CANCELLED state is preserved.","commit_id":"481077a22ff81d8a1a93d25e2eaab23c60d63ada"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4467278db8f5d4663bbbd79052588cb6781d55d9","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            pipeline.state \u003d objects.audit_pipeline.State.CANCELLED"},{"line_number":158,"context_line":"            pipeline.save()"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        except _PipelineCancelled:"},{"line_number":161,"context_line":"            # State already CANCELLED in DB"},{"line_number":162,"context_line":"            pass"},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f448e39_1832d475","line":160,"updated":"2026-08-12 13:17:55.000000000","message":"When _PipelineCancelled is raised during do_execute (detected between stages), the except handler simply does `pass` without updating the in-memory pipeline object state or sending an EXECUTION END notification. The pipeline.state remains ONGOING on the in-memory object. Contrast with the ActionP...\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: After mid-execution cancellation, the in-memory pipeline object has stale ONGOING state. If any code path uses the in-memory object after execute() returns (e.g., for notifications or logging), it sees incorrect state. Missing END notification leaves consumers waiting for a lifecycle event that n...\n\n**Priority**: Before merge\n**Why This Matters**: After mid-execution cancellation, the in-memory pipeline object has stale ONGOING state. If any code path uses the in-memory object after execute() returns (e.g., for notifications or logging), it sees incorrect state. Missing END notification leaves consumers waiting for a lifecycle event that n...\n\n**Recommendation**:\nIn the _PipelineCancelled handler, set pipeline.state \u003d objects.audit_pipeline.State.CANCELLED and save, and optionally send an END notification for consistency with the START notification. Alternatively, re-fetch the pipeline from DB to sync state. Add a test that verifies pipeline.state \u003d\u003d CANCELLED after mid-stage cancellation.","commit_id":"481077a22ff81d8a1a93d25e2eaab23c60d63ada"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"747b3bbb2f12c4d715c304a1fe49f9ad624a0e0e","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                phase\u003dwfields.NotificationPhase.END,"},{"line_number":153,"context_line":"            )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"        except exception.ActionPlanIsOngoing as e:"},{"line_number":156,"context_line":"            LOG.warning(\"Pipeline %s cancelled: %s\", pipeline.uuid, e)"},{"line_number":157,"context_line":"            pipeline.state \u003d objects.audit_pipeline.State.CANCELLED"},{"line_number":158,"context_line":"            pipeline.save()"}],"source_content_type":"text/x-python","patch_set":4,"id":"4dba439c_6e86fd20","line":155,"updated":"2026-08-12 23:13:29.000000000","message":"The execute() method sends START and END notifications for success, and ERROR notifications for failures, but cancellation paths (ActionPlanIsOngoing and _PipelineCancelled) do not emit any notification, breaking the notification lifecycle contract.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Consumers monitoring audit pipeline execution via notifications will not be informed when a pipeline is cancelled, leading to incomplete lifecycle tracking. A pipeline started notification will be emitted with no corresponding terminal notification, making it appear hung or still running.\n\n**Suggestion**:\nAdd a send_action_notification call with phase\u003dEND (or a CANCEL phase if appropriate) in both cancellation branches to ensure all terminal states emit a notification. Consider sending the notification before or after setting CANCELLED state for consistency with the SUCCESS and ERROR paths.","commit_id":"8d164d28924afe3c0bf428f7238bfdad88f5f312"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"773e3dc962934f27e757a374800c42510aab446f","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                phase\u003dwfields.NotificationPhase.END,"},{"line_number":153,"context_line":"            )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"        except exception.ActionPlanIsOngoing as e:"},{"line_number":156,"context_line":"            LOG.warning(\"Pipeline %s cancelled: %s\", pipeline.uuid, e)"},{"line_number":157,"context_line":"            pipeline.state \u003d objects.audit_pipeline.State.CANCELLED"},{"line_number":158,"context_line":"            pipeline.save()"}],"source_content_type":"text/x-python","patch_set":4,"id":"b764a7e4_1da7d664","line":155,"in_reply_to":"4dba439c_6e86fd20","updated":"2026-08-13 17:24:38.000000000","message":"Going to remove the check for ActionPlanIsOngoing, that only makes sense if we start to support CONTINUOUS audits, which would create multiple action plans, and one of them could be ONGOING when executing Audit again","commit_id":"8d164d28924afe3c0bf428f7238bfdad88f5f312"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"2e2407e7529e73873657064231709add3a5abe2c","unresolved":false,"context_lines":[{"line_number":154,"context_line":"            # State already CANCELLED in DB"},{"line_number":155,"context_line":"            pass"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        except Exception as e:"},{"line_number":158,"context_line":"            LOG.exception(\"Pipeline %s failed: %s\", pipeline.uuid, e)"},{"line_number":159,"context_line":"            notifications.audit_pipeline.send_action_notification("},{"line_number":160,"context_line":"                context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"82595419_be67e2f0","line":157,"updated":"2026-08-18 14:28:19.000000000","message":"PipelineHandler.pre_execute() raises exception.AuditPipelineCancelled when the re-fetched pipeline is already CANCELLED (line 71). PipelineHandler.execute() only special-cases the internal _PipelineCancelled exception; AuditPipelineCancelled is a WatcherException and is therefore caught by the generic \u0027except Exception\u0027 branch, which sets state\u003dFAILED, saves it (overwriting the CANCELLED state in the DB), and emits an ERROR execution notification.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Impact**: A pipeline cancelled between scheduling and execution start (a realistic window: trigger_audit_pipeline is a cast into the audit executor pool) ends FAILED instead of CANCELLED. The DB transition CANCELLED-\u003eFAILED is written, ERROR notifications and exception tracebacks are emitted, and API/monitoring consumers see a spurious failure for a deliberately cancelled pipeline. FAILED and CANCELLED are distinct terminal states in AuditPipelineStateTransitionManager (watcher/objects/audit_pipeline.py:37-41).\n\n**Priority**: Before merge\n**Recommendation**:\nCatch exception.AuditPipelineCancelled together with _PipelineCancelled in execute() (e.g. \u0027except (exception.AuditPipelineCancelled, _PipelineCancelled): pass\u0027) or have pre_execute raise _PipelineCancelled, leaving the CANCELLED state intact. Update test_pre_execute_already_cancelled_pipeline to assert the pipeline stays CANCELLED, matching its own docstring.","commit_id":"be29943003c0a0b3f9829b2fb40fb854734f0da7"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"a92dc29ba1c023c3869228b2ebbc94f7592ca9b7","unresolved":false,"context_lines":[{"line_number":154,"context_line":"            # State already CANCELLED in DB"},{"line_number":155,"context_line":"            pass"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        except Exception as e:"},{"line_number":158,"context_line":"            LOG.exception(\"Pipeline %s failed: %s\", pipeline.uuid, e)"},{"line_number":159,"context_line":"            notifications.audit_pipeline.send_action_notification("},{"line_number":160,"context_line":"                context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"36bce375_e354e3ec","line":157,"in_reply_to":"82595419_be67e2f0","updated":"2026-08-19 20:13:12.000000000","message":"Correct, fixed in next PS","commit_id":"be29943003c0a0b3f9829b2fb40fb854734f0da7"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"d810f1da1f3e269a991c165c604b3b375dd381de","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":64,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":65,"context_line":"        )"},{"line_number":66,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":67,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":10,"id":"b2b63aba_d04f3fd7","line":66,"updated":"2026-08-21 19:19:25.000000000","message":"PipelineHandler.pre_execute compares the AuditPipeline object\u0027s state against objects.audit.State.CANCELLED, while do_execute and the SUCCEEDED/FAILED assignments correctly use objects.audit_pipeline.State. The two State classes are independent definitions that currently happen to use identical strings, so today\u0027s behavior is accidentally correct, but the pipeline state machine is now interpreted through the audit\u0027s state class at one of its two cancellation decision points.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: If either State class ever changes (the audit class is the older, more volatile one), the pre_execute check silently stops matching real pipeline states and cancelled pipelines start executing; no test pins the two classes together, so the break would be silent.\n\n**Recommendation**:\nUse objects.audit_pipeline.State.CANCELLED in pre_execute so both cancellation checks read the pipeline\u0027s own state machine.","commit_id":"f0ab162b157e5d90396e781219caff2b8eba64f1"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8377a3c4843913d08f883360a48e36fd4780b4ad","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def __init__(self):"},{"line_number":53,"context_line":"        # So far the only one supported"},{"line_number":54,"context_line":"        self._strategy_context \u003d cascade_context.CascadePipelineContext()"},{"line_number":55,"context_line":"        self._planner_loader \u003d loader.DefaultPlannerLoader()"},{"line_number":56,"context_line":"        self.applier_client \u003d rpcapi.ApplierAPI()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"e9926780_f0edb3a6","line":54,"updated":"2026-08-27 21:38:04.000000000","message":"PipelineHandler is instantiated once in AuditEndpoint.__init__ and its CascadePipelineContext keeps per-pipeline execution state (_compute_cdm, _cache) as instance attributes. trigger_audit_pipeline submits executions to the futurist thread pool sized by max_audit_workers (default 2), so two pipelines executing concurrently interleave on the same context: setup() of the second pipeline replaces self._cache while the first is mid-flight, and self._compute_cdm is overwritten by the second pipeline\u0027s stage 0, so the first pipeline\u0027s later stages plan against the wrong projected CDM and metric cache.\n\n**Severity**: HIGH | **Confidence**: 0.75\n\n**Impact**: When two pipelines are triggered concurrently (the executor pool exists precisely for parallel audit work), stage strategies of one pipeline can consume another pipeline\u0027s projected cluster model and simulated metrics, producing incorrect solutions and an action plan that matches neither pipeline\u0027s intent. The result is silently wrong; no exception is raised.\n\n**Priority**: Before merge\n**Recommendation**:\nCreate a fresh execution context per pipeline execution, e.g. instantiate CascadePipelineContext (or a per-execution state holder) inside do_execute() or pass state through execute_stage(), and keep PipelineHandler stateless like AuditHandler/DefaultStrategyContext. Add a unit test running two interleaved executions asserting each stage sees only its own pipeline\u0027s CDM/cache.","commit_id":"28236f13fa9d599485b48ee391f2c7ebe71736b8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"8377a3c4843913d08f883360a48e36fd4780b4ad","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":64,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":65,"context_line":"        )"},{"line_number":66,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":67,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":11,"id":"a70dac35_f06ce56a","line":66,"updated":"2026-08-27 21:38:04.000000000","message":"PipelineHandler.pre_execute compares the re-fetched AuditPipeline state to objects.audit.State.CANCELLED (the Audit object enum) rather than objects.audit_pipeline.State.CANCELLED. The check currently works only because both enums happen to define the string \u0027CANCELLED\u0027. do_execute in the same file correctly uses objects.audit_pipeline.State.CANCELLED, so one rule (detect cancelled pipeline) now has two independent sources of truth that can diverge.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: No incorrect behavior today, but the cancellation gate in pre_execute silently stops detecting CANCELLED pipelines if the pipeline state vocabulary ever diverges from the audit one, and the mixed usage makes such divergence easy to miss in later review.\n\n**Suggestion**:\nChange line 66 to use objects.audit_pipeline.State.CANCELLED so both cancellation checks reference the pipeline enum.","commit_id":"28236f13fa9d599485b48ee391f2c7ebe71736b8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"86d05130ed3cc736e926c5ed78a7aacd81e8d56d","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def __init__(self):"},{"line_number":53,"context_line":"        # So far the only one supported"},{"line_number":54,"context_line":"        self._strategy_context \u003d cascade_context.CascadePipelineContext()"},{"line_number":55,"context_line":"        self._planner_loader \u003d loader.DefaultPlannerLoader()"},{"line_number":56,"context_line":"        self.applier_client \u003d rpcapi.ApplierAPI()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"aa80bf89_53cbda4c","line":54,"updated":"2026-08-27 22:25:54.000000000","message":"PipelineHandler is instantiated once in AuditEndpoint.__init__ and CascadePipelineContext keeps per-execution state (self._compute_cdm, self._cache) on that long-lived instance. AuditEndpoint submits trigger_audit_pipeline calls to a futurist pool sized by watcher_decision_engine.max_audit_workers (default 2), so two pipelines can interleave execute_stage() calls on the same context: pipeline B\u0027s stage would deepcopy pipeline A\u0027s mutated CDM and both pipelines share one metric cache (simulated host metrics keyed only by resource UUID), silently cross-contaminating planning inputs. Additionally, setup() resets the cache but never resets _compute_cdm, so if a later pipeline\u0027s lowest-position stage is not position 0 it silently plans against the previous pipeline\u0027s leftover mutated CDM.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Impact**: Concurrently triggered pipelines can produce action plans computed against another pipeline\u0027s projected cluster state and another pipeline\u0027s simulated metrics, yielding wrong migration decisions; sequential executions can leak a stale mutated CDM. Errors are silent (no exception, plausible-looking plans).\n\n**Priority**: Before merge\n**Recommendation**:\nMake the execution context per-execution: instantiate CascadePipelineContext (or reset both _compute_cdm\u003dNone and the cache) at the start of PipelineHandler.execute()/do_execute, mirroring how DefaultStrategyContext derives everything from the audit per call. Add a test that runs two interleaved executions and asserts each stage sees only its own pipeline\u0027s CDM.","commit_id":"0d346f3b90d2927a5d5e2db6eeca277e440256f6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"f6dcbc257631b856fa85f736d31c1a95a3937fa2","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def __init__(self):"},{"line_number":53,"context_line":"        # So far the only one supported"},{"line_number":54,"context_line":"        self._strategy_context \u003d cascade_context.CascadePipelineContext()"},{"line_number":55,"context_line":"        self._planner_loader \u003d loader.DefaultPlannerLoader()"},{"line_number":56,"context_line":"        self.applier_client \u003d rpcapi.ApplierAPI()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"628bc733_cd545253","line":54,"updated":"2026-08-28 14:18:41.000000000","message":"PipelineHandler owns one CascadePipelineContext for the whole decision-engine process, but that context stores per-pipeline mutable state (self._compute_cdm and self._cache). Pipeline triggers are submitted to a multi-worker futurist pool, so two pipelines executing at the same time read and write the same attributes.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Impact**: With two pipelines in flight (e.g. operator triggers a second pipeline while the first is between stages), stage N of pipeline A deep-copies whatever CDM pipeline B last stored and both pipelines\u0027 simulated metrics land in the same cache; solutions are computed against the wrong projected cluster state, producing silently incorrect merged action plans. A second pipeline\u0027s setup() can also drop the first pipeline\u0027s cached metrics mid-execution.\n\n**Priority**: Before merge\n**Recommendation**:\nMake the execution context per-run: construct a fresh CascadePipelineContext in PipelineHandler.execute()/do_execute (or in do_trigger_audit_pipeline) instead of in __init__, or key _compute_cdm/_cache by pipeline uuid. Keeping PipelineHandler stateless matches the existing AuditHandler/DefaultStrategyContext pattern.","commit_id":"dedc2f2148fdd3d759738a45f503cd373099ef73"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"901e32ed30dc6c7342a0ab9a3a4cae0debb46481","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def __init__(self):"},{"line_number":53,"context_line":"        # So far the only one supported"},{"line_number":54,"context_line":"        self._strategy_context \u003d cascade_context.CascadePipelineContext()"},{"line_number":55,"context_line":"        self._planner_loader \u003d loader.DefaultPlannerLoader()"},{"line_number":56,"context_line":"        self.applier_client \u003d rpcapi.ApplierAPI()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"0179d24a_feef4066","line":54,"in_reply_to":"628bc733_cd545253","updated":"2026-08-28 18:34:50.000000000","message":"Done, moved to execute(), it should supports now multiple concurrent audit pipelines.","commit_id":"dedc2f2148fdd3d759738a45f503cd373099ef73"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"1a6e9f2a747aa50db8b9f5e930b2ac6a7b285939","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":67,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":68,"context_line":"        )"},{"line_number":69,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":70,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":14,"id":"6aa1672f_fe83d897","line":69,"updated":"2026-08-28 18:51:09.000000000","message":"PipelineHandler.pre_execute compares the fresh pipeline state with objects.audit.State.CANCELLED (the Audit object\u0027s enum), while every other state reference in the same file uses objects.audit_pipeline.State. The check works today only because both enums happen to define CANCELLED \u003d \u0027CANCELLED\u0027.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: The cancellation decision for pipelines is owned by an unrelated enum; if either enum ever diverges (for example audit gains SUSPENDED-style states or pipeline states are renumbered), pre_execute silently stops detecting cancelled pipelines or misinterprets states, with no test failing.\n\n**Recommendation**:\nChange watcher/decision_engine/audit/pipeline.py:69 to objects.audit_pipeline.State.CANCELLED, matching the rest of the handler.","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"9fb0b2ecdcd202a5bf7c103f2d7d1824f2d5362d","unresolved":true,"context_lines":[{"line_number":66,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":67,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":68,"context_line":"        )"},{"line_number":69,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":70,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":14,"id":"f25278ef_2fdf27ae","line":69,"in_reply_to":"6aa1672f_fe83d897","updated":"2026-09-02 10:18:47.000000000","message":"this is minor but valid","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2fe2f09cfbbb6ad31b37cf2b5b5f078c6b40472d","unresolved":true,"context_lines":[{"line_number":66,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":67,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":68,"context_line":"        )"},{"line_number":69,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":70,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa2e1166_47158dbd","line":69,"in_reply_to":"f25278ef_2fdf27ae","updated":"2026-09-02 12:06:43.000000000","message":"True, I can fix that in the next PS or follow up. I will leave this as open","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"76aee911a130563bf70510f6db51e46aaa558010","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        fresh \u003d objects.AuditPipeline.get_by_uuid("},{"line_number":67,"context_line":"            context, pipeline.uuid, eager\u003dTrue"},{"line_number":68,"context_line":"        )"},{"line_number":69,"context_line":"        if fresh.state \u003d\u003d objects.audit.State.CANCELLED:"},{"line_number":70,"context_line":"            raise exception.AuditPipelineCancelled(uuid\u003dpipeline.uuid)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        pipeline.hostname \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":14,"id":"7e64da7c_c02636c2","line":69,"in_reply_to":"fa2e1166_47158dbd","updated":"2026-09-02 15:49:35.000000000","message":"Done","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"1a6e9f2a747aa50db8b9f5e930b2ac6a7b285939","unresolved":false,"context_lines":[{"line_number":150,"context_line":"                phase\u003dwfields.NotificationPhase.END,"},{"line_number":151,"context_line":"            )"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        except exception.AuditPipelineCancelled:"},{"line_number":154,"context_line":"            # State already CANCELLED in DB"},{"line_number":155,"context_line":"            pass"},{"line_number":156,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"c0952fc4_b4da1436","line":153,"updated":"2026-08-28 18:51:09.000000000","message":"PipelineHandler.execute sends an audit_pipeline EXECUTION notification with phase START, but the AuditPipelineCancelled handler only logs and returns without emitting any END, ERROR or CANCEL notification, so the started execution event is never terminated for consumers.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: Any consumer of versioned audit_pipeline notifications that waits for an execution end event (monitoring, event-driven tooling) sees cancelled pipelines stuck in \u0027execution started\u0027 forever, with no signal that execution actually stopped.\n\n**Suggestion**:\nIn the AuditPipelineCancelled handler call notifications.audit_pipeline.send_action_notification with phase\u003dEND (or a dedicated cancel action) and an appropriate priority, mirroring how the applier closes started executions and reports cancellation.","commit_id":"00b8043aec446bb0f6256ddf4882a386a556ed59"}],"watcher/decision_engine/planner/pipeline.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"773e3dc962934f27e757a374800c42510aab446f","unresolved":true,"context_lines":[{"line_number":236,"context_line":"    # ------------------------------------------------------------------"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _create_action_plan(self, context, pipeline_id):"},{"line_number":239,"context_line":"        # NOTE(dviroel): Do not copy global efficacy from"},{"line_number":240,"context_line":"        # solutions since they don\u0027t represent the complete"},{"line_number":241,"context_line":"        # action plan efficacy"},{"line_number":242,"context_line":"        action_plan_dict \u003d {"},{"line_number":243,"context_line":"            \u0027uuid\u0027: utils.generate_uuid(),"},{"line_number":244,"context_line":"            \u0027audit_pipeline_id\u0027: pipeline_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"6cd9fb50_9e94a209","line":241,"range":{"start_line":239,"start_character":0,"end_line":241,"end_character":30},"updated":"2026-08-13 17:24:38.000000000","message":"Note for reviewers: a new efficacy indicator and global efficacy need to be created for audit pipeline. Not all strategies provide similar efficacy indicators, which means that some indicator values don\u0027t properly represent the solution\u0027s actions. (e.g. 2 stages produce migration action, but only one has migration_count as indicator, providing  this indicator in the actionplan provides an incorrect information)\nA new global efficacy for audit pipeline will be required, based on actions. This may be a new Change or an ammend to this one.","commit_id":"8d164d28924afe3c0bf428f7238bfdad88f5f312"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"20c0b9796ee8afef0fa70f9b5d82b825c83567e2","unresolved":true,"context_lines":[{"line_number":236,"context_line":"    # ------------------------------------------------------------------"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _create_action_plan(self, context, pipeline_id):"},{"line_number":239,"context_line":"        # NOTE(dviroel): Do not copy global efficacy from"},{"line_number":240,"context_line":"        # solutions since they don\u0027t represent the complete"},{"line_number":241,"context_line":"        # action plan efficacy"},{"line_number":242,"context_line":"        action_plan_dict \u003d {"},{"line_number":243,"context_line":"            \u0027uuid\u0027: utils.generate_uuid(),"},{"line_number":244,"context_line":"            \u0027audit_pipeline_id\u0027: pipeline_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ea01d99_2659791f","line":241,"range":{"start_line":239,"start_character":0,"end_line":241,"end_character":30},"in_reply_to":"5e26a799_b433c937","updated":"2026-09-04 11:33:34.000000000","message":"The spec will need an update for next cycle. What happens is that 2 indicators with different names represent same or equal operations. So I created this new change that group similar indicators for audit pipeline[1].\nWe could instead of creating a new group, concatenate them yes, but still looks wierd. If you take the INSTANCE_MIGRATIONS_GROUP as an example, one strategy populates \"instance_migrations_count \u003d 1\" while other populates \"planned_live_migrate_instance_count \u003d 1\". It looks like the same migration, but are 2 different migrations from 2 different strategies.\nI think that in the end we would need to have common efficacy indicators shared among strategies instead of each strategy creates it own indicator.\nEven with this ^ refactoring, we could still rely on the proposal of [1], since we can update \"source_names\" there as needed.\n[1] https://review.opendev.org/c/openstack/watcher/+/1001009/13/watcher/decision_engine/goal/efficacy/pipeline.py#111","commit_id":"8d164d28924afe3c0bf428f7238bfdad88f5f312"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e06217b1bd0c05ed2566c690a99dd741392cd53e","unresolved":true,"context_lines":[{"line_number":236,"context_line":"    # ------------------------------------------------------------------"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _create_action_plan(self, context, pipeline_id):"},{"line_number":239,"context_line":"        # NOTE(dviroel): Do not copy global efficacy from"},{"line_number":240,"context_line":"        # solutions since they don\u0027t represent the complete"},{"line_number":241,"context_line":"        # action plan efficacy"},{"line_number":242,"context_line":"        action_plan_dict \u003d {"},{"line_number":243,"context_line":"            \u0027uuid\u0027: utils.generate_uuid(),"},{"line_number":244,"context_line":"            \u0027audit_pipeline_id\u0027: pipeline_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5e26a799_b433c937","line":241,"range":{"start_line":239,"start_character":0,"end_line":241,"end_character":30},"in_reply_to":"6cd9fb50_9e94a209","updated":"2026-09-03 16:34:55.000000000","message":"thinking about this, probably the best we can do is, as you mention, create new efficacy indicators per action type and produce them from the final action plane. However, in the spec we mentioned we\u0027d simply concatenate the indicators for all stages, so I think that would be ok for an initial version as well:\n```\n Indicators with the same name from different stages will be concatenated rather than\ndeduplicated, so that no information about individual stage contributions is\nlost. \n```","commit_id":"8d164d28924afe3c0bf428f7238bfdad88f5f312"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"285181c41d8a9e4ecfa03b7f3fddd1f97084492e","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    3. Ordering actions by weight and building a weighted DAG (via the"},{"line_number":54,"context_line":"       :class:`~watcher.decision_engine.planner.weight"},{"line_number":55,"context_line":"       .WeightedSchedulingMixin`)."},{"line_number":56,"context_line":"    4. Persisting all actions and aggregated efficacy indicators."},{"line_number":57,"context_line":"    \"\"\""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"e53f2f97_ad29b880","line":56,"updated":"2026-08-19 20:58:52.000000000","message":"The PipelinePlanner class docstring step 4 states the planner persists \u0027all actions and aggregated efficacy indicators\u0027. The implementation does the opposite: _create_action_plan explicitly sets global_efficacy\u003d[] with a NOTE saying solution efficacy is not copied, and no efficacy indicators are created anywhere in schedule(). Pipeline action plans therefore carry no efficacy data, and the docstring misleads readers/maintainers about a behavior the code intentionally omits.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Maintainers and API consumers reading the planner contract will expect efficacy indicators on pipeline action plans and find none, which obscures the intentional limitation and invites incorrect downstream assumptions.\n\n**Suggestion**:\nCorrect the docstring to state that global efficacy and efficacy indicators are intentionally left empty for pipeline action plans (with the NOTE rationale), or, if aggregation is intended for a later patch, mark it as a TODO referencing the audit-pipeline blueprint.","commit_id":"90a07f3854ee20d480c673f450230079f8b81f9b"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"901e32ed30dc6c7342a0ab9a3a4cae0debb46481","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    3. Ordering actions by weight and building a weighted DAG (via the"},{"line_number":54,"context_line":"       :class:`~watcher.decision_engine.planner.weight"},{"line_number":55,"context_line":"       .WeightedSchedulingMixin`)."},{"line_number":56,"context_line":"    4. Persisting all actions and aggregated efficacy indicators."},{"line_number":57,"context_line":"    \"\"\""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"98513c44_a8e86a3f","line":56,"in_reply_to":"e53f2f97_ad29b880","updated":"2026-08-28 18:34:50.000000000","message":"It is going to be implemented right in the next commit.","commit_id":"90a07f3854ee20d480c673f450230079f8b81f9b"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"d810f1da1f3e269a991c165c604b3b375dd381de","unresolved":false,"context_lines":[{"line_number":307,"context_line":"            \u0027audit_id\u0027: None,"},{"line_number":308,"context_line":"            \u0027strategy_id\u0027: None,"},{"line_number":309,"context_line":"            \u0027state\u0027: objects.action_plan.State.RECOMMENDED,"},{"line_number":310,"context_line":"            \u0027global_efficacy\u0027: [],"},{"line_number":311,"context_line":"        }"},{"line_number":312,"context_line":"        action_plan \u003d objects.ActionPlan(context, **action_plan_dict)"},{"line_number":313,"context_line":"        action_plan.create()"}],"source_content_type":"text/x-python","patch_set":10,"id":"9b056607_5ebd0dd2","line":310,"updated":"2026-08-21 19:19:25.000000000","message":"The PipelinePlanner class docstring lists \u0027Persisting all actions and aggregated efficacy indicators\u0027 as step 5 of schedule(), but _create_action_plan hardcodes global_efficacy to [] and no efficacy indicators are aggregated or persisted. In parallel, CascadePipelineContext calls strategy.execute(skip_pre_execute\u003dTrue) without skip_global_efficacy\u003dTrue, so every stage runs post_execute/compute_global_efficacy and produces a global efficacy that the planner then throws away.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Every pipeline action plan will report empty efficacy to users and operators despite the code claiming aggregation, and each stage pays for a global efficacy computation whose result is unconditionally discarded.\n\n**Suggestion**:\nEither aggregate the stage solutions\u0027 global_efficacy (or recompute efficacy over the merged action set) before persisting the action plan, or align the code with the current limitation: fix the docstring and pass skip_global_efficacy\u003dTrue in cascade.py so stages skip the discarded computation, and keep the inline NOTE so the API limitation is documented.","commit_id":"f0ab162b157e5d90396e781219caff2b8eba64f1"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"86d05130ed3cc736e926c5ed78a7aacd81e8d56d","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    4. Ordering actions by weight and building a weighted DAG (via the"},{"line_number":57,"context_line":"       :class:`~watcher.decision_engine.planner.weight"},{"line_number":58,"context_line":"       .WeightedSchedulingMixin`)."},{"line_number":59,"context_line":"    5. Persisting all actions and aggregated efficacy indicators."},{"line_number":60,"context_line":"    \"\"\""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":12,"id":"f744f792_e98c4f2b","line":59,"updated":"2026-08-27 22:25:54.000000000","message":"The PipelinePlanner class docstring documents step \u00275. Persisting all actions and aggregated efficacy indicators\u0027, but schedule() never creates EfficacyIndicator rows and _create_action_plan hard-codes global_efficacy to []. Each stage\u0027s solution does compute global_efficacy (CascadePipelineContext does not pass skip_global_efficacy), so per-stage efficacy data is computed and then silently discarded, leaving pipeline action plans with no efficacy information at all, unlike WeightPlanner which persists both indicators and global efficacy.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: Operators querying pipeline action plans (watcher actionplan show / efficacy indicator API) get empty efficacy data, making it impossible to judge plan quality; the docstring misleads maintainers into believing indicators are persisted, and the per-stage computation work is wasted.\n\n**Suggestion**:\nEither aggregate and persist efficacy indicators for the merged plan (e.g. reuse _create_efficacy_indicators per stage solution and merge global_efficacy) or, if efficacy is intentionally out of scope for this bp stage, correct the docstring and log a debug note; the test suite currently asserts empty global_efficacy, so update it together with the chosen behavior.","commit_id":"0d346f3b90d2927a5d5e2db6eeca277e440256f6"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"1871fca47a294bb07d8b7a76588b31bb52d136df","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                merged_actions"},{"line_number":102,"context_line":"            )"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        action_plan \u003d self._create_action_plan(context, pipeline_id)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        sorted_weighted_actions \u003d self._get_weighted_actions("},{"line_number":107,"context_line":"            context, action_plan, merged_actions"}],"source_content_type":"text/x-python","patch_set":15,"id":"ac93e8f5_40759e7a","line":104,"updated":"2026-09-01 22:42:22.000000000","message":"schedule() never persists efficacy indicators from the merged stage solutions, yet the class docstring step 5 states \u0027Persisting all actions and aggregated efficacy indicators\u0027. The reused WeightPlanner calls _create_efficacy_indicators for exactly this purpose.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Action plans created by pipelines expose no efficacy indicators through the API even though every stage strategy computed them, degrading audit result reporting for pipeline plans; the docstring also misleads future maintainers into believing indicators are stored.\n\n**Suggestion**:\nEither aggregate and persist the per-stage indicators (merge each solution\u0027s efficacy_indicators with stage attribution, as _create_efficacy_indicators does for single audits) or fix the docstring and document that pipeline plans do not report efficacy for now.","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"41ad1cd1784aacd0d8e8ceff64b081aee66247c6","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                merged_actions"},{"line_number":102,"context_line":"            )"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        action_plan \u003d self._create_action_plan(context, pipeline_id)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        sorted_weighted_actions \u003d self._get_weighted_actions("},{"line_number":107,"context_line":"            context, action_plan, merged_actions"}],"source_content_type":"text/x-python","patch_set":15,"id":"f9449d81_eb01e80f","line":104,"in_reply_to":"ac93e8f5_40759e7a","updated":"2026-09-02 11:35:56.000000000","message":"efficacy indicators are handled in the next patch in the chain https://review.opendev.org/c/openstack/watcher/+/1001009 so this is ok","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"}],"watcher/decision_engine/planner/weight.py":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"41ad1cd1784aacd0d8e8ceff64b081aee66247c6","unresolved":true,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def compute_action_graph(self, sorted_weighted_actions):"},{"line_number":106,"context_line":"        reverse_weights \u003d {v: k for k, v in self.config.weights.items()}"},{"line_number":107,"context_line":"        # leaf_groups contains a list of list of nodes called groups"},{"line_number":108,"context_line":"        # each group is a set of nodes from which a future node will"},{"line_number":109,"context_line":"        # branch off (parent nodes)."},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"47dcc76f_470e835c","side":"PARENT","line":107,"updated":"2026-09-02 11:35:56.000000000","message":"was this removal intentional or did the AI do it? I find that some tend to delete comments, and while some like the one from `chunkify` above are fine, this one is worth keeping as it helps understand what the method does","commit_id":"6f40745b58ffbc139e4d2dfddb194f6e04299e97"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2fe2f09cfbbb6ad31b37cf2b5b5f078c6b40472d","unresolved":true,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def compute_action_graph(self, sorted_weighted_actions):"},{"line_number":106,"context_line":"        reverse_weights \u003d {v: k for k, v in self.config.weights.items()}"},{"line_number":107,"context_line":"        # leaf_groups contains a list of list of nodes called groups"},{"line_number":108,"context_line":"        # each group is a set of nodes from which a future node will"},{"line_number":109,"context_line":"        # branch off (parent nodes)."},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"f6576356_023fbd51","side":"PARENT","line":107,"in_reply_to":"47dcc76f_470e835c","updated":"2026-09-02 12:06:43.000000000","message":"i moved a lot these methods around, so I may end up deleting them, I will bring the back in this case yeah. Thanks","commit_id":"6f40745b58ffbc139e4d2dfddb194f6e04299e97"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"76aee911a130563bf70510f6db51e46aaa558010","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def compute_action_graph(self, sorted_weighted_actions):"},{"line_number":106,"context_line":"        reverse_weights \u003d {v: k for k, v in self.config.weights.items()}"},{"line_number":107,"context_line":"        # leaf_groups contains a list of list of nodes called groups"},{"line_number":108,"context_line":"        # each group is a set of nodes from which a future node will"},{"line_number":109,"context_line":"        # branch off (parent nodes)."},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"49eb435e_b401ed4e","side":"PARENT","line":107,"in_reply_to":"f6576356_023fbd51","updated":"2026-09-02 15:49:35.000000000","message":"Done","commit_id":"6f40745b58ffbc139e4d2dfddb194f6e04299e97"}],"watcher/decision_engine/strategy/context/cascade.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c337a1e72c74907ad5beafaeac7d81029973eea0","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            strategy.input_parameters.update(parameters)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        # 3. Initializez the strategy by calling its pre_execute method"},{"line_number":143,"context_line":"        solution \u003d strategy.pre_execute()"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # 4. Sets metric data cache and run simulations from previous solutions"},{"line_number":146,"context_line":"        if strategy.get_datasource_metrics():"}],"source_content_type":"text/x-python","patch_set":1,"id":"adedffb7_d21a3e67","line":143,"updated":"2026-08-06 19:24:16.000000000","message":"In CascadePipelineContext.execute_stage, pre_execute() is called explicitly at line 143, then strategy.execute() is called at line 160. The base Strategy.execute() method internally calls self.pre_execute() before do_execute and post_execute. This results in pre_execute() being invoked twice per...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Calling pre_execute() twice wastes computation and may produce duplicate log messages, duplicate deprecation warnings, or inconsistent state for strategies whose pre_execute has non-idempotent side effects. The dead \u0027solution \u003d \u0027 assignment at line 143 reduces readability.\n\n**Recommendation**:\nEither remove the explicit pre_execute() call at line 143 (if the metric cache setup between lines 143-151 does not depend on pre_execute side effects), or replace strategy.execute() at line 160 with direct calls to do_execute() and post_execute() if the explicit pre_execute is intentional.","commit_id":"811bd92cc4b9dab05660464ccfe2e143e2ef2d62"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"c456c8681bc5769da9535c4c2d21b01ba25cb1aa","unresolved":false,"context_lines":[{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        try:"},{"line_number":279,"context_line":"            instance \u003d compute_model.get_instance_by_uuid(resource_id)"},{"line_number":280,"context_line":"        except Exception:"},{"line_number":281,"context_line":"            LOG.debug(\"CDM lookup failed for instance %s\", resource_id)"},{"line_number":282,"context_line":"            return"},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5c5d38c1_811fd013","line":280,"updated":"2026-08-07 20:09:52.000000000","message":"The _simulate_migration_metrics method uses four separate \u0027except Exception\u0027 clauses to guard CDM lookups and metric fetching. The same file already uses specific exceptions (InstanceNotFound, ComputeNodeNotFound) in _apply_migration (lines 216, 235), demonstrating that concrete exception types a...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Overly broad exception handling may silently swallow unexpected errors (e.g., programming errors, database connectivity issues) that should propagate or be logged differently. It also makes debugging harder since the actual exception type is not visible in the code structure.\n\n**Suggestion**:\nReplace \u0027except Exception\u0027 with specific exception types. For CDM lookups, use exception.InstanceNotFound and exception.ComputeNodeNotFound (matching the pattern in _apply_migration). For _fetch_metric, catch the datasource-specific aggregation error or at minimum use oslo-based exception types.","commit_id":"5000eb5318e616f8ecebd2b3cd8f9f8ae856cc34"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4467278db8f5d4663bbbd79052588cb6781d55d9","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        if parameters:"},{"line_number":140,"context_line":"            strategy.input_parameters.update(parameters)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        # 3. Initializez the strategy by calling its pre_execute method"},{"line_number":143,"context_line":"        solution \u003d strategy.pre_execute()"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # 4. Sets metric data cache and run simulations from previous solutions"}],"source_content_type":"text/x-python","patch_set":3,"id":"07017a53_e5eba171","line":142,"updated":"2026-08-12 13:17:55.000000000","message":"A code comment in cascade.py line 142 contains a spelling error: \u0027Initializez\u0027 instead of \u0027Initializes\u0027.\n\n**Severity**: SUGGESTION | **Confidence**: 1.0\n\n**Benefit**: Minor readability issue; no functional impact.\n\n**Recommendation**:\nChange \u0027Initializez\u0027 to \u0027Initializes\u0027 in the comment.","commit_id":"481077a22ff81d8a1a93d25e2eaab23c60d63ada"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"747b3bbb2f12c4d715c304a1fe49f9ad624a0e0e","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            strategy.input_parameters.update(parameters)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        # 3. Initializez the strategy by calling its pre_execute method"},{"line_number":143,"context_line":"        solution \u003d strategy.pre_execute()"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # 4. Sets metric data cache and run simulations from previous solutions"},{"line_number":146,"context_line":"        if strategy.get_datasource_metrics():"}],"source_content_type":"text/x-python","patch_set":4,"id":"4295e13e_76cd5f8a","line":143,"updated":"2026-08-12 23:13:29.000000000","message":"In execute_stage(), pre_execute() is called explicitly at line 143 and then called again inside strategy.execute() (base.py:291). This causes all pre_execute side effects to run twice for every stage execution.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Each stage strategy\u0027s pre_execute() runs twice, causing redundant work (re-fetching parameters, re-validating cluster model, duplicate log messages). Some strategies may have non-idempotent pre_execute side effects, potentially causing unexpected behavior or errors during pipeline execution.\n\n**Priority**: Before merge\n**Why This Matters**: Each stage strategy\u0027s pre_execute() runs twice, causing redundant work (re-fetching parameters, re-validating cluster model, duplicate log messages). Some strategies may have non-idempotent pre_execute side effects, potentially causing unexpected behavior or errors during pipeline execution.\n\n**Recommendation**:\nRemove the explicit `solution \u003d strategy.pre_execute()` call at line 143 if the intent was just to initialize before setting up the metric cache. Alternatively, if pre_execute needs to run before metric cache setup, call strategy.do_execute() and strategy.post_execute() separately instead of strategy.execute() to avoid the duplicate pre_execute invocation.","commit_id":"8d164d28924afe3c0bf428f7238bfdad88f5f312"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"773e3dc962934f27e757a374800c42510aab446f","unresolved":true,"context_lines":[{"line_number":140,"context_line":"            strategy.input_parameters.update(parameters)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        # 3. Initializez the strategy by calling its pre_execute method"},{"line_number":143,"context_line":"        solution \u003d strategy.pre_execute()"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # 4. Sets metric data cache and run simulations from previous solutions"},{"line_number":146,"context_line":"        if strategy.get_datasource_metrics():"}],"source_content_type":"text/x-python","patch_set":4,"id":"b0f94216_cb1c500e","line":143,"in_reply_to":"4295e13e_76cd5f8a","updated":"2026-08-13 17:24:38.000000000","message":"true, it is going to call it twice, which isn\u0027t really a issue since pre_execute should do only initializations. I plan to add an option to skip phases in strategy.execute() so we can skip wherever we want.","commit_id":"8d164d28924afe3c0bf428f7238bfdad88f5f312"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a4c228e8435bd47eae70994affd01a1edfc3c4e7","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    # CDM mutation"},{"line_number":170,"context_line":"    # ------------------------------------------------------------------"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def _mutate_cdm(self, solution):"},{"line_number":173,"context_line":"        \"\"\"Apply the stage\u0027s actions to a copy of the CDM and store it.\"\"\""},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        for action in solution.actions:"}],"source_content_type":"text/x-python","patch_set":5,"id":"f2126aa5_47830d4f","line":172,"updated":"2026-08-13 20:11:48.000000000","message":"The _mutate_cdm method docstring states \u0027Apply the stage\u0027s actions to a copy of the CDM and store it\u0027 but the implementation calls self._apply_migration(self._compute_cdm, ...) and self._apply_nova_service_state(self._compute_cdm, ...), mutating the shared CDM object directly without copying.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: The misleading docstring could cause a future developer to believe the original CDM is preserved, leading to incorrect assumptions about data isolation between stages.\n\n**Recommendation**:\nUpdate the docstring to reflect the actual behavior, e.g., \u0027Apply the stage\u0027s actions to the shared CDM in place so subsequent stages see the projected cluster state.\u0027","commit_id":"b2c6a46879481f4ced03e1f320f2689fb3554511"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"285181c41d8a9e4ecfa03b7f3fddd1f97084492e","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        source_host \u003d params.get(\u0027source_node\u0027)"},{"line_number":208,"context_line":"        dest_host \u003d params.get(\u0027destination_node\u0027)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        if not dest_host:"},{"line_number":211,"context_line":"            try:"},{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"}],"source_content_type":"text/x-python","patch_set":8,"id":"2f4a8d7b_f8ea1a8d","line":210,"updated":"2026-08-19 20:58:52.000000000","message":"In _apply_migration, the branch handling migrations without destination_node catches only exception.InstanceNotFound. cdm.get_node_by_name(source_host) on the next line raises ComputeNodeNotFound, which is not caught in this branch, while the with-destination branch below (cascade.py:235-239) correctly catches both. The exception propagates through execute_stage -\u003e PipelineHandler.do_execute -\u003e execute(), where the generic handler marks the entire pipeline FAILED, even though the sibling branch treats the same condition as a warning-level simulation miss.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: A single migration action without an explicit destination whose source node is missing from the CDM aborts all remaining pipeline stages and marks the pipeline FAILED, instead of degrading the simulation the way the sibling branch does.\n\n**Suggestion**:\nCatch both exception.InstanceNotFound and exception.ComputeNodeNotFound in the no-destination branch (mirroring lines 235-239) and log a warning, or factor the shared try/except so both branches share one lookup helper.","commit_id":"90a07f3854ee20d480c673f450230079f8b81f9b"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"901e32ed30dc6c7342a0ab9a3a4cae0debb46481","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        source_host \u003d params.get(\u0027source_node\u0027)"},{"line_number":208,"context_line":"        dest_host \u003d params.get(\u0027destination_node\u0027)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        if not dest_host:"},{"line_number":211,"context_line":"            try:"},{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"}],"source_content_type":"text/x-python","patch_set":8,"id":"72faaef8_60c2903c","line":210,"in_reply_to":"2f4a8d7b_f8ea1a8d","updated":"2026-08-28 18:34:50.000000000","message":"ack, we can include the additional exception handling","commit_id":"90a07f3854ee20d480c673f450230079f8b81f9b"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"d810f1da1f3e269a991c165c604b3b375dd381de","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self._cache \u003d None"},{"line_number":63,"context_line":"        LOG.debug(\"Initializing Cascade Pipeline Context\")"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def setup(self):"},{"line_number":66,"context_line":"        \"\"\"Initialise the shared CDM, metric cache, and compute collector.\"\"\""},{"line_number":67,"context_line":"        self._cache \u003d metric_cache.MetricDataCache()"},{"line_number":68,"context_line":"        LOG.debug(\"Cascade context initialised: metric cache ready\")"}],"source_content_type":"text/x-python","patch_set":10,"id":"b5e6ab7f_72357519","line":65,"updated":"2026-08-21 19:19:25.000000000","message":"AuditEndpoint builds one PipelineHandler at startup, which builds one CascadePipelineContext, and submits trigger_audit_pipeline jobs to the audit-endpoint worker pool (watcher_decision_engine.max_audit_workers, default 2, explicitly documented to \u0027execute audits in parallel\u0027). CascadePipelineContext keeps per-execution state on that shared instance: self._cache (replaced by setup(), mutated by cache.put/clear_simulated) and self._compute_cdm (reassigned whenever a position-0 stage runs). Two pipelines running concurrently interleave these writes.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Impact**: If a second pipeline is triggered while a first is mid-run, the first pipeline\u0027s metric cache is swapped out and its _compute_cdm is replaced by the second pipeline\u0027s cluster model. Later stages of the first pipeline then plan migrations and service state changes against the wrong projected cluster state and consume metrics simulated for the other pipeline, producing an action plan that does not match either pipeline\u0027s intent.\n\n**Priority**: Before merge\n**Recommendation**:\nMake the execution state per-run: instantiate a fresh CascadePipelineContext (or a per-execution state object holding the cache and CDM) inside PipelineHandler.execute, or thread the cache and CDM through execute_stage parameters instead of instance attributes. A regression test can submit two execute() calls concurrently against one context and assert each stage sees its own model/cache.","commit_id":"f0ab162b157e5d90396e781219caff2b8eba64f1"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"86d05130ed3cc736e926c5ed78a7aacd81e8d56d","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"},{"line_number":214,"context_line":"                if instance and source_node:"},{"line_number":215,"context_line":"                    cdm.remove_instance(instance, source_node)"},{"line_number":216,"context_line":"            except ("},{"line_number":217,"context_line":"                exception.InstanceNotFound,"},{"line_number":218,"context_line":"                exception.ComputeNodeNotFound,"}],"source_content_type":"text/x-python","patch_set":12,"id":"de033136_5095961c","line":215,"updated":"2026-08-27 22:25:54.000000000","message":"In the defensive branch of CascadePipelineContext._apply_migration (migration action with no destination_node), the code calls cdm.remove_instance(instance, source_node), but ModelRoot.remove_instance(self, instance) accepts a single argument (it re-derives the node via get_node_by_instance_uuid). If that branch is ever taken it raises TypeError instead of removing the instance, and the TypeError is not caught by the surrounding except (InstanceNotFound, ComputeNodeNotFound), so the whole pipeline would be marked FAILED rather than degrading gracefully.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: Any migration action lacking destination_node (malformed or future strategy output) turns an intended best-effort CDM adjustment into an unhandled TypeError that fails the pipeline; the unit test asserts the broken contract, so the defect survives future refactors.\n\n**Suggestion**:\nChange the call to cdm.remove_instance(instance) (the source node is resolved internally), and update test_cascade.py to assert the one-argument call; optionally also catch TypeError alongside the existing exception tuple.","commit_id":"0d346f3b90d2927a5d5e2db6eeca277e440256f6"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"41ad1cd1784aacd0d8e8ceff64b081aee66247c6","unresolved":true,"context_lines":[{"line_number":63,"context_line":"        LOG.debug(\"Initializing Cascade Pipeline Context\")"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def setup(self):"},{"line_number":66,"context_line":"        \"\"\"Initialise the shared CDM, metric cache, and compute collector.\"\"\""},{"line_number":67,"context_line":"        self._cache \u003d metric_cache.MetricDataCache()"},{"line_number":68,"context_line":"        LOG.debug(\"Cascade context initialised: metric cache ready\")"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7fba04a9_33e55121","line":66,"updated":"2026-09-02 11:35:56.000000000","message":"nit the docstring is not really accurate since we only initiliase the metric cache here","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"76aee911a130563bf70510f6db51e46aaa558010","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        LOG.debug(\"Initializing Cascade Pipeline Context\")"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def setup(self):"},{"line_number":66,"context_line":"        \"\"\"Initialise the shared CDM, metric cache, and compute collector.\"\"\""},{"line_number":67,"context_line":"        self._cache \u003d metric_cache.MetricDataCache()"},{"line_number":68,"context_line":"        LOG.debug(\"Cascade context initialised: metric cache ready\")"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7db6782f_5e66396d","line":66,"in_reply_to":"404ad7bf_3057edc5","updated":"2026-09-02 15:49:35.000000000","message":"Done","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2fe2f09cfbbb6ad31b37cf2b5b5f078c6b40472d","unresolved":true,"context_lines":[{"line_number":63,"context_line":"        LOG.debug(\"Initializing Cascade Pipeline Context\")"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def setup(self):"},{"line_number":66,"context_line":"        \"\"\"Initialise the shared CDM, metric cache, and compute collector.\"\"\""},{"line_number":67,"context_line":"        self._cache \u003d metric_cache.MetricDataCache()"},{"line_number":68,"context_line":"        LOG.debug(\"Cascade context initialised: metric cache ready\")"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"404ad7bf_3057edc5","line":66,"in_reply_to":"7fba04a9_33e55121","updated":"2026-09-02 12:06:43.000000000","message":"yeah, i changed that during the developement and this got outdated, will fix.","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"41ad1cd1784aacd0d8e8ceff64b081aee66247c6","unresolved":true,"context_lines":[{"line_number":139,"context_line":"        if parameters:"},{"line_number":140,"context_line":"            strategy.input_parameters.update(parameters)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        # 3. Initializez the strategy by calling its pre_execute method"},{"line_number":143,"context_line":"        solution \u003d strategy.pre_execute()"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # 4. Sets metric data cache and run simulations from previous solutions"}],"source_content_type":"text/x-python","patch_set":15,"id":"e26f1788_c2cb5432","line":142,"updated":"2026-09-02 11:35:56.000000000","message":"```suggestion\n        # 3. Initializes the strategy by calling its pre_execute method\n```","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"41ad1cd1784aacd0d8e8ceff64b081aee66247c6","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        if strategy.get_datasource_metrics():"},{"line_number":147,"context_line":"            strategy.datasource_backend.metric_cache \u003d self._cache"},{"line_number":148,"context_line":"            if solutions and self._strategy_uses_simulatable_metrics(strategy):"},{"line_number":149,"context_line":"                self._simulate_metrics_from_solutions("},{"line_number":150,"context_line":"                    solutions, self._cache, strategy, self._compute_cdm"},{"line_number":151,"context_line":"                )"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"f0020cec_b9775348","line":149,"updated":"2026-09-02 11:35:56.000000000","message":"I\u0027m not sure I understand why we need to simulate all the solutions at the beginning of each stage for the metrics instead of simply doing it at the end of each stage like we do for the cdm","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e06217b1bd0c05ed2566c690a99dd741392cd53e","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        if strategy.get_datasource_metrics():"},{"line_number":147,"context_line":"            strategy.datasource_backend.metric_cache \u003d self._cache"},{"line_number":148,"context_line":"            if solutions and self._strategy_uses_simulatable_metrics(strategy):"},{"line_number":149,"context_line":"                self._simulate_metrics_from_solutions("},{"line_number":150,"context_line":"                    solutions, self._cache, strategy, self._compute_cdm"},{"line_number":151,"context_line":"                )"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"91e46baa_98a9c9d5","line":149,"in_reply_to":"7df3f5d5_d09c7268","updated":"2026-09-03 16:34:55.000000000","message":"Acknowledged","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2fe2f09cfbbb6ad31b37cf2b5b5f078c6b40472d","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        if strategy.get_datasource_metrics():"},{"line_number":147,"context_line":"            strategy.datasource_backend.metric_cache \u003d self._cache"},{"line_number":148,"context_line":"            if solutions and self._strategy_uses_simulatable_metrics(strategy):"},{"line_number":149,"context_line":"                self._simulate_metrics_from_solutions("},{"line_number":150,"context_line":"                    solutions, self._cache, strategy, self._compute_cdm"},{"line_number":151,"context_line":"                )"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7df3f5d5_d09c7268","line":149,"in_reply_to":"f0020cec_b9775348","updated":"2026-09-02 12:06:43.000000000","message":"Yeah, what happens is that not all stages use the same metrics or same datasources/fetch parameters, so for each strategy we need to simulate how previous solutions affects the topology (only metrics in this case since for CDM this will not happen)","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"1871fca47a294bb07d8b7a76588b31bb52d136df","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"},{"line_number":214,"context_line":"                if instance and source_node:"},{"line_number":215,"context_line":"                    cdm.remove_instance(instance, source_node)"},{"line_number":216,"context_line":"            except ("},{"line_number":217,"context_line":"                exception.InstanceNotFound,"},{"line_number":218,"context_line":"                exception.ComputeNodeNotFound,"}],"source_content_type":"text/x-python","patch_set":15,"id":"198889a5_0fd18004","line":215,"updated":"2026-09-01 22:42:22.000000000","message":"CascadePipelineContext._apply_migration calls cdm.remove_instance(instance, source_node) when a migrate action has no destination_node, but ModelRoot.remove_instance(self, instance) accepts only one argument. The resulting TypeError escapes the (InstanceNotFound, ComputeNodeNotFound) handler and fails the stage.\n\n**Severity**: HIGH | **Confidence**: 0.85\n\n**Impact**: Any cascade pipeline stage whose strategy emits a migrate action without destination_node (e.g. host_maintenance evacuation) raises TypeError during CDM mutation; PipelineHandler.execute catches it as a generic failure and marks the pipeline FAILED even though the strategy produced a valid solution.\n\n**Priority**: Before merge\n**Recommendation**:\nCall cdm.delete_instance(instance, source_node) or cdm.remove_instance(instance); consider unmap_instance() so the migrating instance stays visible to later stages. Add a test using a real ModelRoot rather than a MagicMock to pin the call signature.","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"41ad1cd1784aacd0d8e8ceff64b081aee66247c6","unresolved":true,"context_lines":[{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"},{"line_number":214,"context_line":"                if instance and source_node:"},{"line_number":215,"context_line":"                    cdm.remove_instance(instance, source_node)"},{"line_number":216,"context_line":"            except ("},{"line_number":217,"context_line":"                exception.InstanceNotFound,"},{"line_number":218,"context_line":"                exception.ComputeNodeNotFound,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fa76532_2c74bff5","line":215,"in_reply_to":"198889a5_0fd18004","updated":"2026-09-02 11:35:56.000000000","message":"this seems valid, if this is the method being called, the number of parameters passed is wrong https://github.com/openstack/watcher/blob/8599261212fa5eab6444a1b2ea9d55af6e6d3207/watcher/decision_engine/model/model_root.py#L175","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2fe2f09cfbbb6ad31b37cf2b5b5f078c6b40472d","unresolved":true,"context_lines":[{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"},{"line_number":214,"context_line":"                if instance and source_node:"},{"line_number":215,"context_line":"                    cdm.remove_instance(instance, source_node)"},{"line_number":216,"context_line":"            except ("},{"line_number":217,"context_line":"                exception.InstanceNotFound,"},{"line_number":218,"context_line":"                exception.ComputeNodeNotFound,"}],"source_content_type":"text/x-python","patch_set":15,"id":"e898fcc9_25441f3b","line":215,"in_reply_to":"9fa76532_2c74bff5","updated":"2026-09-02 12:06:43.000000000","message":"hum, I thought that I had handle this scenario, i will check it again..","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e06217b1bd0c05ed2566c690a99dd741392cd53e","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"},{"line_number":214,"context_line":"                if instance and source_node:"},{"line_number":215,"context_line":"                    cdm.remove_instance(instance, source_node)"},{"line_number":216,"context_line":"            except ("},{"line_number":217,"context_line":"                exception.InstanceNotFound,"},{"line_number":218,"context_line":"                exception.ComputeNodeNotFound,"}],"source_content_type":"text/x-python","patch_set":15,"id":"c2907af3_93b3279e","line":215,"in_reply_to":"a356d5e9_11152182","updated":"2026-09-03 16:34:55.000000000","message":"Done","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"76aee911a130563bf70510f6db51e46aaa558010","unresolved":true,"context_lines":[{"line_number":212,"context_line":"                instance \u003d cdm.get_instance_by_uuid(resource_id)"},{"line_number":213,"context_line":"                source_node \u003d cdm.get_node_by_name(source_host)"},{"line_number":214,"context_line":"                if instance and source_node:"},{"line_number":215,"context_line":"                    cdm.remove_instance(instance, source_node)"},{"line_number":216,"context_line":"            except ("},{"line_number":217,"context_line":"                exception.InstanceNotFound,"},{"line_number":218,"context_line":"                exception.ComputeNodeNotFound,"}],"source_content_type":"text/x-python","patch_set":15,"id":"a356d5e9_11152182","line":215,"in_reply_to":"e898fcc9_25441f3b","updated":"2026-09-02 15:49:35.000000000","message":"right, now i see the issue, i do cover the dest_node\u003dNone scenario, but it is calling the wrong method.","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"1871fca47a294bb07d8b7a76588b31bb52d136df","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            )"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"            if source_ram is not None:"},{"line_number":407,"context_line":"                new_source_ram \u003d max(0, source_ram - instance_ram)"},{"line_number":408,"context_line":"                cache.put("},{"line_number":409,"context_line":"                    source_obj.uuid,"},{"line_number":410,"context_line":"                    \u0027host_ram_usage\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"30e02910_4df78987","line":407,"updated":"2026-09-01 22:42:22.000000000","message":"_simulate_instance_migration subtracts/adds instance_ram_usage directly to/from host_ram_usage with no unit conversion, unlike the CPU branch which converts instance-relative percent into a host fraction. Under the default gnocchi backend the two metrics are not in the same unit.\n\n**Severity**: HIGH | **Confidence**: 0.75\n\n**Impact**: For pipelines whose later stages consume host_ram_usage (e.g. workload_stabilization), simulated values written into the shared metric cache are wrong by ~1024x on gnocchi and ~1e6x on prometheus; later stages then plan migrations against badly wrong projected host memory and can recommend overloading destination hosts.\n\n**Priority**: Before merge\n**Recommendation**:\nNormalize before combining: convert host values to MB using the same convention strategies use (units.Ki on gnocchi) or compute deltas from CDM capacities (node.memory_mb_capacity, instance.memory), and gate the simulation on backends whose units are known. Add a test with realistic gnocchi-scale values for both metrics (current tests only cover CPU).","commit_id":"edb25130091259a5bc69637b4031faff1b6fdf95"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"bf07a8769ad947fb876679a8a5f7b5f31067233a","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            )"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"            if source_ram is not None:"},{"line_number":407,"context_line":"                new_source_ram \u003d max(0, source_ram - instance_ram)"},{"line_number":408,"context_line":"                cache.put("},{"line_number":409,"context_line":"                    source_obj.uuid,"},{"line_number":410,"context_line":"                    \u0027host_ram_usage\u0027,"}],"source_content_type":"text/x-python","patch_set":16,"id":"f38e910a_7808ec05","line":407,"updated":"2026-09-02 16:29:15.000000000","message":"In _simulate_instance_migration the RAM branch computes new_source_ram \u003d max(0, source_ram - instance_ram) and new_dest_ram \u003d max(0, dest_ram + instance_ram), mixing host_ram_usage (kibibytes) with instance_ram_usage (megabytes) with no unit conversion. The CPU branch scales the instance contribution by vcpus/host ratios; the RAM branch has no equivalent scaling.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Impact**: Simulated host_ram_usage changes by ~1024x less than intended, so later stages of a cascade pipeline effectively see unchanged RAM usage. RAM-based strategies (workload_stabilization, vm_workload_consolidation) will overcommit destination hosts or miss freed capacity, and the merged action plan executes those real migrations. Added tests (test_ram_updates_source_and_dest) encode the unconverted subtraction, so the error is locked in.\n\n**Priority**: Before merge\n**Recommendation**:\nConvert the instance contribution to the host metric\u0027s unit before subtracting (e.g. instance_ram_mb * 1024 for KiB backends, ideally scaling like the CPU branch using memory ratios), or document and enforce a single unit convention in the cache. Update the RAM tests to assert converted values.","commit_id":"ba279c28b9cbb1e3ba6bcf33eb594aa1ff581398"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"97a0348c08e1de2fe2de154d6a6e8def9062a309","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            )"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"            if source_ram is not None:"},{"line_number":407,"context_line":"                new_source_ram \u003d max(0, source_ram - instance_ram)"},{"line_number":408,"context_line":"                cache.put("},{"line_number":409,"context_line":"                    source_obj.uuid,"},{"line_number":410,"context_line":"                    \u0027host_ram_usage\u0027,"}],"source_content_type":"text/x-python","patch_set":16,"id":"8383da85_4c20e82a","line":407,"in_reply_to":"f38e910a_7808ec05","updated":"2026-09-02 23:28:58.000000000","message":"this now should be fixed","commit_id":"ba279c28b9cbb1e3ba6bcf33eb594aa1ff581398"}]}