)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d9a05db88e3e667c31c6e5a3a82156f1bcc19df7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ad2b0cd9_fef236ba","updated":"2026-09-10 14:03:48.000000000","message":"LGTM, if you send a new PS, please fix the duplicated comment issue.\n\nTest outputs:\n\ntest_delete_active_instance[actuator,id-b3f2a1c4-8e7d-4a90-bc12-\n5f6e3d9a0b47,strategy]\npass\ntest_delete_skipped_when_instance_already_deleted[actuator,id-4070c0ec-43b1-41fc-9363-a5e8458cb87c,strategy]\npass\ntest_shelve_active_instance[actuator,id-ef156ca9-50d3-45e6-83f4-d97bfa92e168,strategy]\npass\ntest_shelve_skipped_when_instance_already_shelved[actuator,id-752d84da-bb38-4cf2-8fb0-7bd1963dab21,strategy]\npass\ntest_shelve_skipped_when_instance_not_found[actuator,id-72c68eec-3223-4216-a984-902caca3cfb4,strategy]","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e75ef334c8afe0ada640bf310fd90d69660baa3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"87c61ddb_d9e8b0f7","updated":"2026-09-17 13:48:15.000000000","message":"recheck","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"484bdb09dc613f11e342e624711cc55f4212fe62","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"07fd81bb_9cd5f3d7","updated":"2026-09-03 14:31:30.000000000","message":"recheck\n\ndepends-on patch is updated","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"38098f1f68b16f7471a2416324c0b2f7f3834b23","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ebec8bea_aee07fef","updated":"2026-08-28 13:47:36.000000000","message":"recheck\n\nfailure in aetos job is not related to this change","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d4677af5fb35833c7e1a1cf4f07d1556398df38a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8f456b4e_abb7139d","updated":"2026-08-27 15:09:30.000000000","message":"recheck \n\ndepends-on patch was updated with the new flag to true","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ef13c2fe30eb6fdda32e2f18de158f15174b1af8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"175664e0_f99d6915","updated":"2026-09-29 16:12:07.000000000","message":"tests lgtm, thanks","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"}],"watcher_tempest_plugin/tests/scenario/test_execute_actuator.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":"c53ef20df20ee86408705865e3320972e4c456c7","unresolved":false,"context_lines":[{"line_number":303,"context_line":"            }"},{"line_number":304,"context_line":"        ]"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        self.wait_for_all_action_plans_to_finish()"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        audit_template \u003d self.create_audit_template_for_strategy()"},{"line_number":309,"context_line":"        audit \u003d self.create_audit_and_wait("}],"source_content_type":"text/x-python","patch_set":2,"id":"8394a604_870685e7","line":306,"updated":"2026-08-24 18:47:34.000000000","message":"test_delete_skipped_when_instance_already_deleted and test_shelve_skipped_when_instance_not_found each inline a verbatim ~35-line copy of the audit-template creation, audit-and-wait, action plan lookup, start, poll, and state-assertion flow. This duplicates the _run_actuator_action helper added in the same patch and the pre-existing execute_actions_and_validate_states(pre_execution_hook\u003d...) helper in test_execute_skipped_actions.py that exists precisely for mutating resources between audit completion and action plan start. The invariant \u0027start the plan, wait for completion, assert finished state\u0027 now has three independent implementations in the added code.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: A future change to the start/wait contract (timeout handling, finished-state assertions, action listing) must be applied in three places in this file plus the sibling module; missing a copy makes the skip-path tests silently diverge from the postcondition tests they are meant to mirror.\n\n**Recommendation**:\nExtend _run_actuator_action with an optional pre_execution_hook callable invoked after show_action_plan and before start_action_plan, and have both skip tests call it with functools.partial(self._delete_instance, instance[\u0027id\u0027]), mirroring execute_actions_and_validate_states in test_execute_skipped_actions.py.","commit_id":"cea9ddb0acc6ce7142bcbb38c6c405b9aef48d71"},{"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":"2ef2d77243c31739ca37ed05504472f7527827f7","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":216,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":217,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":218,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":3,"id":"854e2901_ecd9e6d9","line":217,"updated":"2026-08-25 15:07:49.000000000","message":"The class sets two microversion attributes with the identical comment \u0027# Minimal version required for _create_instance with a specific host\u0027. The comment is correct for compute_min_microversion \u003d NOVA_API_VERSION_CREATE_WITH_HOST (\u00272.74\u0027) but wrong for min_microversion \u003d \u00271.3\u0027, which is the Watcher infra-optim API microversion and unrelated to host-specific instance creation.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: A future maintainer reading the comment may change or remove min_microversion assuming it is a Nova-side constraint (or vice versa), breaking the watcher API version contract these tests rely on.\n\n**Recommendation**:\nReplace the second comment with one that states the actual purpose, e.g. \u0027# Minimal Watcher (infra-optim) API microversion required by these tests\u0027, and note why 1.3 is needed.","commit_id":"766a7ba0ba1db12cd8256e64e8fc2679cb5b01cd"},{"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":"2ef2d77243c31739ca37ed05504472f7527827f7","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027actuator\u0027])"},{"line_number":343,"context_line":"    @decorators.idempotent_id(\u0027ef156ca9-50d3-45e6-83f4-d97bfa92e168\u0027)"},{"line_number":344,"context_line":"    def test_shelve_active_instance(self):"},{"line_number":345,"context_line":"        \"\"\"Test that the shelve action shelves an ACTIVE instance."},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        Creates an instance, submits a shelve action through the actuator"}],"source_content_type":"text/x-python","patch_set":3,"id":"6e013496_43fba042","line":344,"updated":"2026-08-25 15:07:49.000000000","message":"Three of the new tests (test_shelve_active_instance, test_shelve_skipped_when_instance_already_shelved, test_shelve_skipped_when_instance_not_found) rely on the Nova shelve API, but the class never checks the tempest compute_feature_enabled.shelve capability flag, unlike tempest\u0027s own shelve tests and this plugin\u0027s consistent use of capability skip checks for other features.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: On deployments where shelve is not available or disabled in tempest configuration, these tests fail with API errors instead of skipping, producing false negatives for the watcher shelve action coverage.\n\n**Suggestion**:\nAdd a skip check in the class (or per-test) mirroring the plugin convention, e.g. in skip_checks: if not CONF.compute_feature_enabled.shelve: raise cls.skipException(\u0027Shelve is not enabled\u0027) for the shelve tests, or split shelve tests into a subclass with that gate.","commit_id":"766a7ba0ba1db12cd8256e64e8fc2679cb5b01cd"},{"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":"2ef2d77243c31739ca37ed05504472f7527827f7","unresolved":false,"context_lines":[{"line_number":393,"context_line":"        # Shelve the instance before the action plan runs so the"},{"line_number":394,"context_line":"        # pre_condition finds it already shelved and skips the action."},{"line_number":395,"context_line":"        self.mgr.servers_client.shelve_server(instance[\u0027id\u0027])"},{"line_number":396,"context_line":"        waiters.wait_for_server_status("},{"line_number":397,"context_line":"            self.mgr.servers_client,"},{"line_number":398,"context_line":"            instance[\u0027id\u0027],"},{"line_number":399,"context_line":"            (\u0027SHELVED\u0027, \u0027SHELVED_OFFLOADED\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"0751b397_bda20d27","line":396,"updated":"2026-08-25 15:07:49.000000000","message":"test_shelve_skipped_when_instance_already_shelved passes (\u0027SHELVED\u0027, \u0027SHELVED_OFFLOADED\u0027) as the status argument to tempest.common.waiters.wait_for_server_status. That waiter compares the server status with plain equality (if server_status \u003d\u003d status), so a tuple never matches a string status and the call can only end in TimeoutException.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Impact**: The new shelve-skip test cannot pass: after shelving the server, the wait always raises TimeoutException once build_timeout elapses, so every run of test_shelve_skipped_when_instance_already_shelved fails and burns the full timeout before doing so.\n\n**Priority**: Before merge\n**Recommendation**:\nWait for a single concrete status, e.g. waiters.wait_for_server_status(self.mgr.servers_client, instance[\u0027id\u0027], \u0027SHELVED\u0027, ready_wait\u003dFalse), or poll both states with test_utils.call_until_true checking show_server()[\u0027server\u0027][\u0027status\u0027] in (\u0027SHELVED\u0027, \u0027SHELVED_OFFLOADED\u0027) using CONF.optimize.resource_timeout/resource_check_interval.","commit_id":"766a7ba0ba1db12cd8256e64e8fc2679cb5b01cd"},{"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":"611e5db577b61ad3aff0ccfeb1e54ad1fc62072e","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":217,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":218,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":219,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":4,"id":"6172c2a8_de87850f","line":218,"updated":"2026-08-25 17:43:42.000000000","message":"The new class sets two attributes with the identical comment \u0027# Minimal version required for _create_instance with a specific host\u0027. The first applies to compute_min_microversion (Nova 2.74 for host-pinned boot), but min_microversion \u003d \u00271.3\u0027 is the Watcher (optimize) API microversion and is unrelated to _create_instance. The duplicated comment misattributes a constraint to the wrong version knob.\n\n**Severity**: SUGGESTION | **Confidence**: 0.95\n\n**Impact**: A reader debugging microversion skips will assume the Watcher 1.3 requirement comes from host-pinned instance creation and may adjust or remove the wrong knob, silently changing test skip behavior.\n\n**Recommendation**:\nReplace the second comment with one stating the actual constraint, e.g. \u0027# Watcher API min microversion required by these tests\u0027, naming the API feature that needs 1.3.","commit_id":"7ba5c3a062b4621f72abb19022d05dd76a226441"},{"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":"611e5db577b61ad3aff0ccfeb1e54ad1fc62072e","unresolved":false,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    @decorators.attr(type\u003d[\u0027strategy\u0027, \u0027actuator\u0027])"},{"line_number":286,"context_line":"    @decorators.idempotent_id(\u00274070c0ec-43b1-41fc-9363-a5e8458cb87c\u0027)"},{"line_number":287,"context_line":"    def test_delete_skipped_when_instance_already_deleted(self):"},{"line_number":288,"context_line":"        \"\"\"Test that the delete action is skipped when instance is gone."},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        Deletes the instance before executing the action plan. The"}],"source_content_type":"text/x-python","patch_set":4,"id":"1b18cb3b_3ea3098c","line":287,"updated":"2026-08-25 17:43:42.000000000","message":"test_delete_skipped_when_instance_already_deleted (287-340) and test_shelve_skipped_when_instance_not_found (422-475) reproduce, statement for statement, the audit creation, action-plan listing/show, start, state assertion, call_until_true polling and final show sequence that _run_actuator_action (224-253) already encapsulates in the same class. The only inserted step is _delete_instance() between showing and starting the plan, giving three independent implementations of the same execution invariant.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: A future fix to polling, timeouts or the PENDING/ONGOING assertion must be applied in three places; missing one leaves the skipped-variant tests with divergent execution semantics that yield inconsistent behavior across tests of the same feature.\n\n**Recommendation**:\nAdd an optional pre-start callback to _run_actuator_action (invoked just before start_action_plan), or split it into _create_pending_action_plan(actions) and _start_and_wait_action_plan(uuid), and have both skipped tests wrap their single _delete_instance call with the shared helpers.","commit_id":"7ba5c3a062b4621f72abb19022d05dd76a226441"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ef13c2fe30eb6fdda32e2f18de158f15174b1af8","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            self._execute_actions(actions)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"class TestExecuteDeleteAndShelveActions("},{"line_number":208,"context_line":"        base.BaseInfraOptimScenarioTest):"},{"line_number":209,"context_line":"    \"\"\"Scenario tests for the delete and shelve actions."},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fc71fb9f_051846ee","line":207,"updated":"2026-09-29 16:12:07.000000000","message":"nit I think this would fit in a single line. I\u0027m guessing this is the AI model formatting, they can be a bit weird sometimes. Unrelated to this patch, but in the main watcher repo we use ruff format to standarize this kind of things, I don\u0027t think we\u0027ve started this kind of work, but it might be a good idea IMO","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"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":"a8e8699504ff8ff9683bdd768bb5636a118a36a7","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":217,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":218,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":219,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":5,"id":"5104f811_8e12c4d9","line":218,"updated":"2026-08-27 15:47:22.000000000","message":"Line 218 repeats the comment \u0027# Minimal version required for _create_instance with a specific host\u0027 above min_microversion \u003d \u00271.3\u0027. That comment describes the Nova compute microversion on lines 216-217 (NOVA_API_VERSION_CREATE_WITH_HOST \u003d \u00272.74\u0027, see base.py:52-57), but min_microversion here is the Watcher infra-optim API microversion (base.py:110-112, 144-146 select it for the optimize client), matching the \u00271.3\u0027 convention in every other scenario test class (test_continuous_audit.py:30, test_data_model.py:33, etc.).\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Impact**: A maintainer reading the class may conclude the Watcher API min_microversion is dictated by Nova 2.74 host pinning, or fear that raising/lowering Nova requirements also changes the Watcher API floor, making future microversion changes in this class error-prone.\n\n**Recommendation**:\nDelete the duplicated comment on line 218, or replace it with an accurate one such as \u0027# Minimal Watcher infra-optim API microversion required by these tests\u0027.","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"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":"3abf13ce3d16511579cf28fedfdccf4d6b3c885f","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":217,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":218,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":219,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":5,"id":"d6b28ddf_72491958","line":218,"updated":"2026-09-17 14:04:27.000000000","message":"Lines 216-219 carry two identical comments, \u0027Minimal version required for _create_instance with a specific host\u0027, one on compute_min_microversion (Nova 2.74) and one on min_microversion \u003d \u00271.3\u0027. min_microversion is the Watcher API microversion checked in BaseInfraOptimScenarioTest.skip_checks against CONF.optimize.min/max_microversion and used to select the optimize request microversion; it is not related to _create_instance host placement, which requires Nova 2.74 governed by the compute_min_microversion line directly above.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: The comment misleads maintainers into tying the Watcher microversion requirement to instance host placement. A future edit that changes host-placement requirements could remove or bump the wrong attribute, silently changing the Watcher API version these tests negotiate.\n\n**Recommendation**:\nReplace the comment on line 218 with an accurate one matching sibling classes, e.g. \u0027# Minimal version required for list data models\u0027, or state the actual Watcher feature that requires 1.3.","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"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":"a5f3010ad6ab04fd4d4024246bd1c413fac58509","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":217,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":218,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":219,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":5,"id":"9261149e_92e50d58","line":218,"updated":"2026-08-28 15:01:59.000000000","message":"The new TestExecuteDeleteAndShelveActions class repeats the comment \u0027# Minimal version required for _create_instance with a specific host\u0027 above both compute_min_microversion and min_microversion. The first is correct (Nova 2.74 for host-on-create), but min_microversion \u003d \u00271.3\u0027 is the Watcher infra-optim API microversion and is unrelated to _create_instance or Nova.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Impact**: A maintainer adjusting microversion floors will believe the Watcher optimize min_microversion is tied to Nova host-on-create support and may bump the wrong knob or omit a needed Watcher API version bump, producing silently wrong skip behavior.\n\n**Recommendation**:\nReplace the second comment with an accurate one, e.g. \u0027# Minimum Watcher (optimize) API microversion needed by these tests\u0027, or remove it if no specific 1.3 feature is actually required.","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"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":"9800c367ad6bb84f4e6fb5b3cf8731f645def04c","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":217,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":218,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":219,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":5,"id":"3dda9fae_a529191c","line":218,"updated":"2026-09-03 14:40:13.000000000","message":"The new class annotates both compute_min_microversion and min_microversion \u003d \u00271.3\u0027 with the identical comment \u0027# Minimal version required for _create_instance with a specific host\u0027. min_microversion 1.3 is the Watcher (optimize) API microversion; the Nova host-pinning requirement is carried by compute_min_microversion \u003d NOVA_API_VERSION_CREATE_WITH_HOST (\u00272.74\u0027). Sibling scenario files (test_execute_zone_migration.py lines 27-29, test_execute_basic_optim.py, test_execute_host_maintenance.py) use the conventional \u0027# Minimal version required for list data models\u0027 for min_microversion \u003d \"1.3\".\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Impact**: A future maintainer adding a watcher API call to this class could wrongly believe min_microversion is tied to Nova 2.74 behavior and avoid raising it when needed, or misdiagnose which microversion gates a skip error; the wrong comment conceals which API each attribute pins.\n\n**Recommendation**:\nChange the comment above min_microversion to reflect the actual watcher API requirement (conventionally \u0027# Minimal version required for list data models\u0027) and keep the host-pinning comment only above compute_min_microversion.","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ef13c2fe30eb6fdda32e2f18de158f15174b1af8","unresolved":true,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":217,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":218,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":219,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":5,"id":"bbe1db92_4edd45b5","line":218,"in_reply_to":"1f9c661b_11610020","updated":"2026-09-29 16:12:07.000000000","message":"+1 this is a minor thing but I think we should fix it before merging, I\u0027ve pushed https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/1007916 to fix it so I think we can proceed with this patch","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d9a05db88e3e667c31c6e5a3a82156f1bcc19df7","unresolved":true,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":217,"context_line":"    compute_min_microversion \u003d base.NOVA_API_VERSION_CREATE_WITH_HOST"},{"line_number":218,"context_line":"    # Minimal version required for _create_instance with a specific host"},{"line_number":219,"context_line":"    min_microversion \u003d \u00271.3\u0027"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"    GOAL \u003d \"unclassified\""}],"source_content_type":"text/x-python","patch_set":5,"id":"1f9c661b_11610020","line":218,"in_reply_to":"5104f811_8e12c4d9","updated":"2026-09-10 14:03:48.000000000","message":"Correct, the second comment is not needed, you actually need 1.3 as min because of the datamodel list api, so we can wait for instances in the model","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"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":"3abf13ce3d16511579cf28fedfdccf4d6b3c885f","unresolved":false,"context_lines":[{"line_number":314,"context_line":"            }"},{"line_number":315,"context_line":"        ]"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"        self.wait_for_all_action_plans_to_finish()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        audit_template \u003d self.create_audit_template_for_strategy()"},{"line_number":320,"context_line":"        audit \u003d self.create_audit_and_wait("}],"source_content_type":"text/x-python","patch_set":5,"id":"4ba05a61_8205ea7b","line":317,"updated":"2026-09-17 14:04:27.000000000","message":"The new class duplicates the audit-to-action-plan execution flow three times. _run_actuator_action (lines 232-260) reimplements the flow already provided by execute_actions_and_validate_states in test_execute_skipped_actions.py, and the two \u0027skipped\u0027 tests inline the entire flow again (lines 317-346 and 452-480), differing only by inserting self._delete_instance(instance[\u0027id\u0027]) before start_action_plan. That insert-point is exactly what the existing helper\u0027s pre_execution_hook parameter was designed for, as used by test_migrate_instance_deleted.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: The same procedural invariant now has three independently editable implementations across two modules. A fix to the flow (e.g. timeout handling, action-plan state checks, or audit-parameter handling) applied to one copy will not propagate to the others, producing inconsistent validation behavior between the actuator and skipped-actions scenario suites.\n\n**Recommendation**:\nSmallest fix: add an optional pre_execution_hook parameter to _run_actuator_action and call it between show_action_plan and start_action_plan, collapsing the two inline copies into helper calls. Better alignment: hoist execute_actions_and_validate_states into BaseInfraOptimScenarioTest and reuse it here with expected_actions_dict\u003d{\u0027SKIPPED\u0027: [\u0027delete\u0027|\u0027shelve\u0027], \u0027SUCCEEDED\u0027: []}.","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"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":"a8e8699504ff8ff9683bdd768bb5636a118a36a7","unresolved":false,"context_lines":[{"line_number":314,"context_line":"            }"},{"line_number":315,"context_line":"        ]"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"        self.wait_for_all_action_plans_to_finish()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        audit_template \u003d self.create_audit_template_for_strategy()"},{"line_number":320,"context_line":"        audit \u003d self.create_audit_and_wait("}],"source_content_type":"text/x-python","patch_set":5,"id":"dfa02747_1c1bc871","line":317,"updated":"2026-08-27 15:47:22.000000000","message":"test_delete_skipped_when_instance_already_deleted (lines 317-349) and test_shelve_skipped_when_instance_not_found (lines 452-483) each inline an identical sequence: wait_for_all_action_plans_to_finish, create_audit_template_for_strategy, create_audit_and_wait with actions parameters, list/show the action plan, delete the instance, start the plan, assert PENDING/ONGOING, poll has_action_plan_finished, show the finished plan, list actions, and assert SUCCEEDED/SUPERSEDED plus SKIPPED. The same patch already adds _run_actuator_action (lines 232-262) implementing most of this flow; the inline copies exist only because the instance must be deleted between plan discovery and start.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: The action-plan execution/assertion invariant now has three owners in one file. A future edit applied to only one copy (for example tightening the terminal-state assertion or the skip assertion) silently leaves the other tests enforcing the old behavior, producing inconsistent coverage of the same pre_condition contract.\n\n**Recommendation**:\nExtend _run_actuator_action with an optional pre-start hook, e.g. def _run_actuator_action(self, actions, before_start\u003dNone): ... if before_start: before_start() ... placed after action-plan discovery and before start_action_plan, and have both tests call it with before_start\u003dlambda: self._delete_instance(instance[\u0027id\u0027]). The action-state assertions can also be folded into the helper or a small _assert_action_skipped helper.","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"},{"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":"a5f3010ad6ab04fd4d4024246bd1c413fac58509","unresolved":false,"context_lines":[{"line_number":314,"context_line":"            }"},{"line_number":315,"context_line":"        ]"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"        self.wait_for_all_action_plans_to_finish()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        audit_template \u003d self.create_audit_template_for_strategy()"},{"line_number":320,"context_line":"        audit \u003d self.create_audit_and_wait("}],"source_content_type":"text/x-python","patch_set":5,"id":"d29ce49a_99dca384","line":317,"updated":"2026-08-28 15:01:59.000000000","message":"test_delete_skipped_when_instance_already_deleted and test_shelve_skipped_when_instance_not_found each duplicate roughly 30 lines of the submit-audit/list-plan/start/wait/show sequence that this same patch implements as _run_actuator_action. The only difference between the copies is the _delete_instance call inserted between show_action_plan and start_action_plan.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: A future fix to the execution sequence (polling timeouts, plan selection, state assertions) applied to _run_actuator_action will silently not reach the two inlined copies, letting the skipped-action tests drift from the other delete/shelve tests and potentially validate stale behavior.\n\n**Suggestion**:\nAdd an optional pre-start hook to the helper, e.g. def _run_actuator_action(self, actions, before_start\u003dNone): invoking before_start(action_plan) immediately before start_action_plan, and convert both skip tests to use it with a small lambda/callback performing the deletion.","commit_id":"5bfc0787e5a1923cb2f7933c58308045e8d41247"}]}