)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d048fcb8f0c2c4e5219a97558fab822aaa778ad7","unresolved":true,"context_lines":[{"line_number":16,"context_line":"On other contitions the action will be FAILED in the pre_condition"},{"line_number":17,"context_line":"check:"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"- Destination node (if specified) does exists or is disabled"},{"line_number":20,"context_line":"- Instance is not ACTIVE and migration_type is live."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Implements: blueprint skip-actions-in-pre-condition"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1956642a_ffdf80c2","line":19,"updated":"2025-11-25 15:58:35.000000000","message":"I think this is missing a `not`:\n`Destination node (if specified) does not exist or is disabled`","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ac0eae1184926bef971af1aee671333f70291bd0","unresolved":false,"context_lines":[{"line_number":16,"context_line":"On other contitions the action will be FAILED in the pre_condition"},{"line_number":17,"context_line":"check:"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"- Destination node (if specified) does exists or is disabled"},{"line_number":20,"context_line":"- Instance is not ACTIVE and migration_type is live."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Implements: blueprint skip-actions-in-pre-condition"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5db1ea0a_ce5f48de","line":19,"in_reply_to":"1956642a_ffdf80c2","updated":"2025-11-26 14:48:20.000000000","message":"Done","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"16d68bdb47f072e3b449ac02a5135cbebea1e0ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"edfbe429_3a2e130c","updated":"2025-11-14 12:18:55.000000000","message":"ill review this properly again on monday","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d048fcb8f0c2c4e5219a97558fab822aaa778ad7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3a5ceb5a_8ff8fd52","updated":"2025-11-25 15:58:35.000000000","message":"the current state transitions on pre_execute lgtm, I just left a comment with a possible small improvement for the tests","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"66a003997f3ba1d4e83b9def8543436d4a25b6ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"994c98ce_6cb6fc0d","updated":"2025-11-27 10:59:23.000000000","message":"I think this is good to go, there is just a small issue in the release note","commit_id":"ca2161b1b07603a11de01c6db89b7dcd4c331981"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"b89af7ab0031d811d060f6ae98c3a9c6c9bb18d6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"70650e4a_657d9b57","updated":"2025-11-28 11:30:46.000000000","message":"recheck","commit_id":"b1d045facc06ed2152189525263f5b776a5383a4"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c39f92ef1bd2f33336cd2b92b0184322b7135acb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"219cbc02_fc8f7477","updated":"2025-12-09 09:43:26.000000000","message":"recheck","commit_id":"2ec812f4e232c0cfaf7a37dc733e283a0d9c0863"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"0a15c4de256af37dbbf360870e7fa8e44ac6da16","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"fa3ce0f1_53127e7d","updated":"2025-12-10 07:41:22.000000000","message":"recheck","commit_id":"2ec812f4e232c0cfaf7a37dc733e283a0d9c0863"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"deb4eb72517a16816d3bbd5319cdd6357dc4a9aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a563afed_7d9e0093","updated":"2025-12-15 10:42:55.000000000","message":"readding vote after rebase","commit_id":"c14f9098a608fb013a710dc9c4ca77fab96d148f"}],"doc/source/actions/migrate.rst":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":48,"context_line":"On other conditions the action will be FAILED in the pre_condition check:"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"- Destination node (if specified) does not exist or is disabled"},{"line_number":51,"context_line":"- The server status is not ACTIVE for live migration"}],"source_content_type":"text/x-rst","patch_set":9,"id":"29a6a94d_f4e2f885","line":51,"updated":"2026-01-15 13:08:30.000000000","message":"+1","commit_id":"c14f9098a608fb013a710dc9c4ca77fab96d148f"}],"releasenotes/notes/migration-precondition-check-ff55be4.yaml":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"66a003997f3ba1d4e83b9def8543436d4a25b6ea","unresolved":true,"context_lines":[{"line_number":11,"context_line":"    On other contitions the action will be FAILED in the pre_condition"},{"line_number":12,"context_line":"    check:"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"    - Destination node (if specified) does exists or is disabled"},{"line_number":15,"context_line":"    - Instance is not ACTIVE and migration_type is live."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"    This improves safety and correctness of migrations."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"b2249495_0891f6e9","line":14,"updated":"2025-11-27 10:59:23.000000000","message":"```suggestion\n    - Destination node (if specified) does not exist or is disabled\n```","commit_id":"5266332e3ca37b8957f42276e9ffb5207873a806"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"0cb7443e4ee04376d3b57a07b013ec5594ab4d53","unresolved":false,"context_lines":[{"line_number":11,"context_line":"    On other contitions the action will be FAILED in the pre_condition"},{"line_number":12,"context_line":"    check:"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"    - Destination node (if specified) does exists or is disabled"},{"line_number":15,"context_line":"    - Instance is not ACTIVE and migration_type is live."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"    This improves safety and correctness of migrations."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"f6b93c41_95f8f789","line":14,"in_reply_to":"b2249495_0891f6e9","updated":"2025-11-27 13:11:47.000000000","message":"Done","commit_id":"5266332e3ca37b8957f42276e9ffb5207873a806"}],"watcher/applier/actions/migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":148,"context_line":"    def _abort_cold_migrate(self, nova):"},{"line_number":149,"context_line":"        # TODO(adisky): currently watcher uses its own version of cold migrate"},{"line_number":150,"context_line":"        # implement cold migrate using nova dependent on the blueprint"},{"line_number":151,"context_line":"        # https://blueprints.launchpad.net/nova/+spec/cold-migration-with-target"},{"line_number":152,"context_line":"        # Abort operation for cold migrate is dependent on blueprint"},{"line_number":153,"context_line":"        # https://blueprints.launchpad.net/nova/+spec/abort-cold-migration"},{"line_number":154,"context_line":"        LOG.warning(\"Abort operation for cold migration is not implemented\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"2832f69a_96fbdcef","line":151,"in_reply_to":"fab1e544_9d04b006","updated":"2026-01-15 13:08:30.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":216,"context_line":"            raise exception.ActionSkipped("},{"line_number":217,"context_line":"                _(\"Instance %s not found\") % self.instance_uuid)"},{"line_number":218,"context_line":"        except Exception:"},{"line_number":219,"context_line":"            raise"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        # Check that the instance is running on source_node"},{"line_number":222,"context_line":"        instance_host \u003d getattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9afadcb5_edb7903b","line":219,"updated":"2025-11-13 14:54:51.000000000","message":"we generally treat `except Exception` the same as a bare except\n\n```\n[H201] Do not write except:, use except Exception: at the very least.\nWhen catching an exception you should be as specific so you don\u0027t\nmistakenly catch unexpected exceptions.\n```\n\nhttps://github.com/openstack/hacking/blob/master/HACKING.rst?plain\u003d1#L28-L30\nhttps://github.com/SeanMooney/openstack-ai-style-guide/blob/master/docs/comprehensive-guide.md#never-use-bare-except-h201\n\nthere are times where you want to exictly catch any excption so it is allwoed by hackign but its never corerct to do\n\n```\nexcept Exception:\n    raise\n```\n\nif you jsut goting to directly re raise then dont add an except block at all.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":216,"context_line":"            raise exception.ActionSkipped("},{"line_number":217,"context_line":"                _(\"Instance %s not found\") % self.instance_uuid)"},{"line_number":218,"context_line":"        except Exception:"},{"line_number":219,"context_line":"            raise"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        # Check that the instance is running on source_node"},{"line_number":222,"context_line":"        instance_host \u003d getattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"759e08b9_6fd9d789","line":219,"in_reply_to":"6b8cfa02_1207ce30","updated":"2026-01-15 13:08:30.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":216,"context_line":"            raise exception.ActionSkipped("},{"line_number":217,"context_line":"                _(\"Instance %s not found\") % self.instance_uuid)"},{"line_number":218,"context_line":"        except Exception:"},{"line_number":219,"context_line":"            raise"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        # Check that the instance is running on source_node"},{"line_number":222,"context_line":"        instance_host \u003d getattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"6b8cfa02_1207ce30","line":219,"in_reply_to":"9afadcb5_edb7903b","updated":"2025-11-14 11:59:42.000000000","message":"Yes, that was the intent, any non-NotFound exception should be raised and the Action will be failed. I\u0027ll remove it.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":232,"context_line":"        if self.destination_node:"},{"line_number":233,"context_line":"            try:"},{"line_number":234,"context_line":"                # Find the compute node and check if service is enabled"},{"line_number":235,"context_line":"                compute_nodes \u003d nova.get_compute_node_by_name("},{"line_number":236,"context_line":"                    self.destination_node, detailed\u003dTrue)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"                dest_node \u003d None"},{"line_number":239,"context_line":"                for cn in compute_nodes:"},{"line_number":240,"context_line":"                    if cn.hypervisor_hostname \u003d\u003d self.destination_node:"},{"line_number":241,"context_line":"                        dest_node \u003d cn"},{"line_number":242,"context_line":"                        break"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"                if not dest_node:"},{"line_number":245,"context_line":"                    raise exception.ActionSkipped("},{"line_number":246,"context_line":"                        _(\"Destination node %s not found\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"aa80105a_0f2bd7d1","line":243,"range":{"start_line":235,"start_character":0,"end_line":243,"end_character":1},"updated":"2025-11-13 14:54:51.000000000","message":"why are you passing detailed\u003dTrue?\n\nis this actully doing a hypervisor detail list?\n\nalso this is not really who nova api shoudl be used ....\n\nhttps://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.py#L56-L96\n\nget_compute_node_list, get_compute_node_by_name  and get_compute_node_by_hostname \n\nare all subtelly incorect.\n\nfirst ironci nodes are also compute nodes, second  get_compute_node_by_hostname is actuly matching on the service host value not the hypervisor hostname\n\nso this should be get_compute_node_by_host\n\nthird comptue service not compute nodes are what can be disabled however because that is very often wanted when lookign at the hyperviorus api we do include that info in that as well.\nhttps://docs.openstack.org/api-ref/compute/#id293\n\nwe also include that without passing detail\u003dTrue so we can optimize this call\n\n\nwhich is why the statsu is not on the hypervior table.\nfinally get_compute_node_by_hostname is assumign that the hypervisor_hostname is a subset of the comptue service host value which is not requried or generally correct\n\nfor ironic for example the hypervior_hostname is the ironic node uuid.\n\ni assume you are doing this becae you have the hypervisor hostname? not the host value so cant use get_compute_node_by_hostname as a result.\n\nget_compute_node_by_name should return a single value not a list but its clear the implementation that that is not ow it wors today. we shoudl fix the name to be \n`get_compute_nodes_by_name` or `get_hypervisors_by_name` to relfect the api endpoint its hitting.\n\nanyway i belive we can optimize this to \n\n```suggestion\n                compute_nodes \u003d nova.get_compute_node_by_name(\n                    self.destination_node)\n                dest_node \u003d next((\n                  cn for cn in compute_nodes\n                  if cn.hypervisor_hostname \u003d\u003d self.destination_node)\n                  , None)\n\n```","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        if self.destination_node:"},{"line_number":233,"context_line":"            try:"},{"line_number":234,"context_line":"                # Find the compute node and check if service is enabled"},{"line_number":235,"context_line":"                compute_nodes \u003d nova.get_compute_node_by_name("},{"line_number":236,"context_line":"                    self.destination_node, detailed\u003dTrue)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"                dest_node \u003d None"},{"line_number":239,"context_line":"                for cn in compute_nodes:"},{"line_number":240,"context_line":"                    if cn.hypervisor_hostname \u003d\u003d self.destination_node:"},{"line_number":241,"context_line":"                        dest_node \u003d cn"},{"line_number":242,"context_line":"                        break"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"                if not dest_node:"},{"line_number":245,"context_line":"                    raise exception.ActionSkipped("},{"line_number":246,"context_line":"                        _(\"Destination node %s not found\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"96222e86_2540b2c4","line":243,"range":{"start_line":235,"start_character":0,"end_line":243,"end_character":1},"in_reply_to":"49874cfa_50f23fa6","updated":"2026-01-15 13:08:30.000000000","message":"this si something we shoudl dig into but not in this review so resolving for now","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"16d68bdb47f072e3b449ac02a5135cbebea1e0ac","unresolved":true,"context_lines":[{"line_number":232,"context_line":"        if self.destination_node:"},{"line_number":233,"context_line":"            try:"},{"line_number":234,"context_line":"                # Find the compute node and check if service is enabled"},{"line_number":235,"context_line":"                compute_nodes \u003d nova.get_compute_node_by_name("},{"line_number":236,"context_line":"                    self.destination_node, detailed\u003dTrue)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"                dest_node \u003d None"},{"line_number":239,"context_line":"                for cn in compute_nodes:"},{"line_number":240,"context_line":"                    if cn.hypervisor_hostname \u003d\u003d self.destination_node:"},{"line_number":241,"context_line":"                        dest_node \u003d cn"},{"line_number":242,"context_line":"                        break"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"                if not dest_node:"},{"line_number":245,"context_line":"                    raise exception.ActionSkipped("},{"line_number":246,"context_line":"                        _(\"Destination node %s not found\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"49874cfa_50f23fa6","line":243,"range":{"start_line":235,"start_character":0,"end_line":243,"end_character":1},"in_reply_to":"9e27abc5_399e86b3","updated":"2025-11-14 12:18:55.000000000","message":"so thats the thing i bleive in nova we use the hypervior hostname for the destiona no thte service host.\n\nthose are often the same but not required to be but we would have to dig deeper into nova to confirm that.\n\nwatcher is not nessisarly doing the writh thing inthe model today so we might need to fix that at some point.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":232,"context_line":"        if self.destination_node:"},{"line_number":233,"context_line":"            try:"},{"line_number":234,"context_line":"                # Find the compute node and check if service is enabled"},{"line_number":235,"context_line":"                compute_nodes \u003d nova.get_compute_node_by_name("},{"line_number":236,"context_line":"                    self.destination_node, detailed\u003dTrue)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"                dest_node \u003d None"},{"line_number":239,"context_line":"                for cn in compute_nodes:"},{"line_number":240,"context_line":"                    if cn.hypervisor_hostname \u003d\u003d self.destination_node:"},{"line_number":241,"context_line":"                        dest_node \u003d cn"},{"line_number":242,"context_line":"                        break"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"                if not dest_node:"},{"line_number":245,"context_line":"                    raise exception.ActionSkipped("},{"line_number":246,"context_line":"                        _(\"Destination node %s not found\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"9e27abc5_399e86b3","line":243,"range":{"start_line":235,"start_character":0,"end_line":243,"end_character":1},"in_reply_to":"aa80105a_0f2bd7d1","updated":"2025-11-14 11:59:42.000000000","message":"Actually, there may be consistencies across Watcher, but in the nova model collector the `hostname` of the compute_nodes is taken from service[\u0027host\u0027], so I think that\u0027s what we should check.\n\nhttps://github.com/openstack/watcher/blob/44472b9479fff6a03fa8a847fa0ed03aa84a5b46/watcher/decision_engine/model/collector/nova.py#L408\n\nso I think using get_compute_node_by_hostname would be the best way to go:\n\nhttps://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.py#L91\n\nwhich returns just an element or exception.\n\nI will update.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                        dest_node \u003d cn"},{"line_number":242,"context_line":"                        break"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"                if not dest_node:"},{"line_number":245,"context_line":"                    raise exception.ActionSkipped("},{"line_number":246,"context_line":"                        _(\"Destination node %s not found\") %"},{"line_number":247,"context_line":"                        self.destination_node)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"                # Check if compute service is enabled"},{"line_number":250,"context_line":"                if dest_node.status !\u003d \u0027enabled\u0027:"},{"line_number":251,"context_line":"                    raise exception.ActionSkipped("},{"line_number":252,"context_line":"                        _(\"Destination node %s is not in enabled state\") %"},{"line_number":253,"context_line":"                        self.destination_node)"},{"line_number":254,"context_line":"            except nova_helper.nvexceptions.NotFound:"},{"line_number":255,"context_line":"                raise exception.ActionSkipped("},{"line_number":256,"context_line":"                    _(\"Destination node %s not found\") %"},{"line_number":257,"context_line":"                    self.destination_node)"},{"line_number":258,"context_line":"            except exception.ActionSkipped:"},{"line_number":259,"context_line":"                raise"},{"line_number":260,"context_line":"            except Exception:"},{"line_number":261,"context_line":"                raise"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        # Check instance status based on migration type"},{"line_number":264,"context_line":"        instance_status \u003d instance.status"}],"source_content_type":"text/x-python","patch_set":2,"id":"1c3d075e_c1067b26","line":261,"range":{"start_line":244,"start_character":0,"end_line":261,"end_character":21},"updated":"2025-11-13 14:54:51.000000000","message":"```suggestion\n                if dest_node is None:\n                    raise exception.ActionSkipped(\n                        _(\"Destination node %s not found\") %\n                        self.destination_node)\n\n                # Check if compute service is enabled\n                if dest_node.status !\u003d \u0027enabled\u0027:\n                    raise exception.ActionFailed(\n                        _(\"Destination node %s is not in enabled state\") %\n                        self.destination_node)\n            except nova_helper.nvexceptions.NotFound:\n                raise exception.ActionSkipped(\n                    _(\"Destination node %s not found\") %\n                    self.destination_node)\n\n```\n\nso as above you shoudl not cactch ecptions just to raise them again witout doign anythign else so we shoudl not ahve \n\n```\n           except exception.ActionSkipped:\n                raise\n            except Exception:\n                raise\n```\n\n\nalso if we selected a destioanto nod e and it disabeld when we try to migrate to it we shoudl not skip we shoudl recored the action as failed.\n\nso we dont want to call execut but hte end state of the action i dont think shoudl be skipped in this case.\nwe may need wider discusson on that but i woudl only consdier it valied to skip if the instance did not exsit.\n\nactully thinkin about htis more im not sure it vaoid to skip it the destination is not fouhnd either.\n\nwe might want to have a wider dicusson on what exactly the pre conditon for skiping a migration is before going much futher.\n\nto me on reflection the only precondition that warrents skipign is that the instance nolonger extis the action is an action on the instanc enot the comptue hots so we shoudl not skip based on the status of the ocmptue host.\n\n\ni didnt check if we have a ActionFailed exepction we can use that will properly move the action to failed but we shoudl likely add one if we dont.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                        dest_node \u003d cn"},{"line_number":242,"context_line":"                        break"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"                if not dest_node:"},{"line_number":245,"context_line":"                    raise exception.ActionSkipped("},{"line_number":246,"context_line":"                        _(\"Destination node %s not found\") %"},{"line_number":247,"context_line":"                        self.destination_node)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"                # Check if compute service is enabled"},{"line_number":250,"context_line":"                if dest_node.status !\u003d \u0027enabled\u0027:"},{"line_number":251,"context_line":"                    raise exception.ActionSkipped("},{"line_number":252,"context_line":"                        _(\"Destination node %s is not in enabled state\") %"},{"line_number":253,"context_line":"                        self.destination_node)"},{"line_number":254,"context_line":"            except nova_helper.nvexceptions.NotFound:"},{"line_number":255,"context_line":"                raise exception.ActionSkipped("},{"line_number":256,"context_line":"                    _(\"Destination node %s not found\") %"},{"line_number":257,"context_line":"                    self.destination_node)"},{"line_number":258,"context_line":"            except exception.ActionSkipped:"},{"line_number":259,"context_line":"                raise"},{"line_number":260,"context_line":"            except Exception:"},{"line_number":261,"context_line":"                raise"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        # Check instance status based on migration type"},{"line_number":264,"context_line":"        instance_status \u003d instance.status"}],"source_content_type":"text/x-python","patch_set":2,"id":"721dc36e_b7227b70","line":261,"range":{"start_line":244,"start_character":0,"end_line":261,"end_character":21},"in_reply_to":"1c3d075e_c1067b26","updated":"2025-11-14 11:59:42.000000000","message":"Raising any exception different that ActionSkipped or ActionPlanCancelled will move the action to FAILED. I\u0027m using ActionExecutionFailure in next PS:\n\nhttps://github.com/openstack/watcher/blob/44472b9479fff6a03fa8a847fa0ed03aa84a5b46/watcher/applier/workflow_engine/base.py#L187-L193\n\nSo, I\u0027m expecting that any issue different that the pre-defined ones will make the action to fail.\n\nWRT the skipping conditions, I think it\u0027s a good discussions, my opinion:\n\n- The instance do not exist -\u003e SKIPPED\n- source_node !\u003d current_host. In most (or all) cases, the instance is moved because of the host where it is running, i.e. host_maintenance, zone_migrations or consolidating strategies are migrating the instances because of the host where they are running. workload_balance also create migration to decrease the resouces usage in the source node. In all those cases, there is no point in migrating the instance if it is running in a different host (i.e. it has been manually migrated before). That\u0027s why I think it should be SKIPPED in that case.\n- destination_host does not exist or it is disabled. I agree with you here after rethinking it, it should be probably FAILED, not SKIPPED.\n- Instance status !\u003d ACTIVE in migration_type \u003d Live. Moving to FAILED seems fine to me. Other option would be to automatically be cold migrated in that case, but that\u0027s out of the scope of this review.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":241,"context_line":"                        dest_node \u003d cn"},{"line_number":242,"context_line":"                        break"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"                if not dest_node:"},{"line_number":245,"context_line":"                    raise exception.ActionSkipped("},{"line_number":246,"context_line":"                        _(\"Destination node %s not found\") %"},{"line_number":247,"context_line":"                        self.destination_node)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"                # Check if compute service is enabled"},{"line_number":250,"context_line":"                if dest_node.status !\u003d \u0027enabled\u0027:"},{"line_number":251,"context_line":"                    raise exception.ActionSkipped("},{"line_number":252,"context_line":"                        _(\"Destination node %s is not in enabled state\") %"},{"line_number":253,"context_line":"                        self.destination_node)"},{"line_number":254,"context_line":"            except nova_helper.nvexceptions.NotFound:"},{"line_number":255,"context_line":"                raise exception.ActionSkipped("},{"line_number":256,"context_line":"                    _(\"Destination node %s not found\") %"},{"line_number":257,"context_line":"                    self.destination_node)"},{"line_number":258,"context_line":"            except exception.ActionSkipped:"},{"line_number":259,"context_line":"                raise"},{"line_number":260,"context_line":"            except Exception:"},{"line_number":261,"context_line":"                raise"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        # Check instance status based on migration type"},{"line_number":264,"context_line":"        instance_status \u003d instance.status"}],"source_content_type":"text/x-python","patch_set":2,"id":"decd3d71_e48dbb0d","line":261,"range":{"start_line":244,"start_character":0,"end_line":261,"end_character":21},"in_reply_to":"721dc36e_b7227b70","updated":"2026-01-15 13:08:30.000000000","message":"Acknowledged","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":268,"context_line":"                    _(\"Live migration requires instance %(instance)s to be \""},{"line_number":269,"context_line":"                      \"in ACTIVE status (current status: %(status)s)\") %"},{"line_number":270,"context_line":"                    {\u0027instance\u0027: self.instance_uuid,"},{"line_number":271,"context_line":"                     \u0027status\u0027: instance_status})"},{"line_number":272,"context_line":"        elif self.migration_type \u003d\u003d self.COLD_MIGRATION:"},{"line_number":273,"context_line":"            if instance_status !\u003d \u0027SHUTOFF\u0027:"},{"line_number":274,"context_line":"                raise exception.ActionSkipped("}],"source_content_type":"text/x-python","patch_set":2,"id":"a49c0678_7b3e45e3","line":271,"updated":"2025-11-13 14:54:51.000000000","message":"this should also restul in the action going to failed\n\nso we shoudl not exectue the action because the precondtion failed but we should mark it as failed not skipped.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":268,"context_line":"                    _(\"Live migration requires instance %(instance)s to be \""},{"line_number":269,"context_line":"                      \"in ACTIVE status (current status: %(status)s)\") %"},{"line_number":270,"context_line":"                    {\u0027instance\u0027: self.instance_uuid,"},{"line_number":271,"context_line":"                     \u0027status\u0027: instance_status})"},{"line_number":272,"context_line":"        elif self.migration_type \u003d\u003d self.COLD_MIGRATION:"},{"line_number":273,"context_line":"            if instance_status !\u003d \u0027SHUTOFF\u0027:"},{"line_number":274,"context_line":"                raise exception.ActionSkipped("}],"source_content_type":"text/x-python","patch_set":2,"id":"b26ba9f1_87e68c39","line":271,"in_reply_to":"a49c0678_7b3e45e3","updated":"2025-11-14 11:59:42.000000000","message":"See my comment below.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e44eaa3028606d2eb78cf343f87c1e6900e5e51d","unresolved":true,"context_lines":[{"line_number":268,"context_line":"                    _(\"Live migration requires instance %(instance)s to be \""},{"line_number":269,"context_line":"                      \"in ACTIVE status (current status: %(status)s)\") %"},{"line_number":270,"context_line":"                    {\u0027instance\u0027: self.instance_uuid,"},{"line_number":271,"context_line":"                     \u0027status\u0027: instance_status})"},{"line_number":272,"context_line":"        elif self.migration_type \u003d\u003d self.COLD_MIGRATION:"},{"line_number":273,"context_line":"            if instance_status !\u003d \u0027SHUTOFF\u0027:"},{"line_number":274,"context_line":"                raise exception.ActionSkipped("}],"source_content_type":"text/x-python","patch_set":2,"id":"ca31ceb7_6b4f03bf","line":271,"in_reply_to":"b26ba9f1_87e68c39","updated":"2025-11-14 12:01:06.000000000","message":"I meant my previous comment about skipping conditions.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                    _(\"Live migration requires instance %(instance)s to be \""},{"line_number":269,"context_line":"                      \"in ACTIVE status (current status: %(status)s)\") %"},{"line_number":270,"context_line":"                    {\u0027instance\u0027: self.instance_uuid,"},{"line_number":271,"context_line":"                     \u0027status\u0027: instance_status})"},{"line_number":272,"context_line":"        elif self.migration_type \u003d\u003d self.COLD_MIGRATION:"},{"line_number":273,"context_line":"            if instance_status !\u003d \u0027SHUTOFF\u0027:"},{"line_number":274,"context_line":"                raise exception.ActionSkipped("}],"source_content_type":"text/x-python","patch_set":2,"id":"5bfbd3a0_1f8d9a8a","line":271,"in_reply_to":"ca31ceb7_6b4f03bf","updated":"2026-01-15 13:08:30.000000000","message":"Acknowledged","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":270,"context_line":"                    {\u0027instance\u0027: self.instance_uuid,"},{"line_number":271,"context_line":"                     \u0027status\u0027: instance_status})"},{"line_number":272,"context_line":"        elif self.migration_type \u003d\u003d self.COLD_MIGRATION:"},{"line_number":273,"context_line":"            if instance_status !\u003d \u0027SHUTOFF\u0027:"},{"line_number":274,"context_line":"                raise exception.ActionSkipped("},{"line_number":275,"context_line":"                    _(\"Cold migration requires instance %(instance)s to be \""},{"line_number":276,"context_line":"                      \"in SHUTOFF status (current status: %(status)s)\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa33f17_a1da2d81","line":273,"updated":"2025-11-13 14:54:51.000000000","message":"this is incorrect.\n\nyou can cold migratie if the instnace is in Active state\nwe will just stop it internally and start it in the resize verify state\n\nthis is actually the recommend way to cold migrate instances.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":270,"context_line":"                    {\u0027instance\u0027: self.instance_uuid,"},{"line_number":271,"context_line":"                     \u0027status\u0027: instance_status})"},{"line_number":272,"context_line":"        elif self.migration_type \u003d\u003d self.COLD_MIGRATION:"},{"line_number":273,"context_line":"            if instance_status !\u003d \u0027SHUTOFF\u0027:"},{"line_number":274,"context_line":"                raise exception.ActionSkipped("},{"line_number":275,"context_line":"                    _(\"Cold migration requires instance %(instance)s to be \""},{"line_number":276,"context_line":"                      \"in SHUTOFF status (current status: %(status)s)\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"e4ced23f_aafde9f0","line":273,"in_reply_to":"7aa33f17_a1da2d81","updated":"2025-11-14 11:59:42.000000000","message":"Thanks for clarifying.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                    {\u0027instance\u0027: self.instance_uuid,"},{"line_number":271,"context_line":"                     \u0027status\u0027: instance_status})"},{"line_number":272,"context_line":"        elif self.migration_type \u003d\u003d self.COLD_MIGRATION:"},{"line_number":273,"context_line":"            if instance_status !\u003d \u0027SHUTOFF\u0027:"},{"line_number":274,"context_line":"                raise exception.ActionSkipped("},{"line_number":275,"context_line":"                    _(\"Cold migration requires instance %(instance)s to be \""},{"line_number":276,"context_line":"                      \"in SHUTOFF status (current status: %(status)s)\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"324aedb7_34acae4b","line":273,"in_reply_to":"e4ced23f_aafde9f0","updated":"2026-01-15 13:08:30.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        else:"},{"line_number":199,"context_line":"            raise exception.InstanceNotFound(name\u003dself.instance_uuid)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def pre_condition(self):"},{"line_number":202,"context_line":"        \"\"\"Check migration preconditions"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        Skipping conditions:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9bdf453e_784dc35d","line":201,"in_reply_to":"208ad04a_0cfa6e88","updated":"2026-01-15 13:08:30.000000000","message":"\u003e Add logging for successful pre_condition checks\n\u003e \n\u003e **Severity**: SUGGESTION | **Confidence**: 0.7\n\u003e \n\u003e **Benefit**: Improves observability and debugging by logging when pre_conditions pass\n\u003e \n\u003e **Recommendation**:\n\u003e Add LOG.info at the end of pre_condition() method to indicate all checks passed successfully\n\nmaybe at debug but info is too noisy\n\nthe code is ok as is.","commit_id":"c14f9098a608fb013a710dc9c4ca77fab96d148f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        # Check that the instance is running on source_node"},{"line_number":221,"context_line":"        instance_host \u003d getattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, None)"},{"line_number":222,"context_line":"        if instance_host !\u003d self.source_node:"},{"line_number":223,"context_line":"            raise exception.ActionSkipped("},{"line_number":224,"context_line":"                _(\"Instance %(instance)s is not running on source node \""},{"line_number":225,"context_line":"                  \"%(source)s (currently on %(current)s)\") %"},{"line_number":226,"context_line":"                {\u0027instance\u0027: self.instance_uuid,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1c1ca7c8_21bb147d","line":223,"in_reply_to":"25ddd656_6af5e2a3","updated":"2026-01-15 13:08:30.000000000","message":"done","commit_id":"c14f9098a608fb013a710dc9c4ca77fab96d148f"}],"watcher/tests/applier/actions/test_migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":13,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from unittest import mock"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import jsonschema"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from novaclient.v2.hypervisors import Hypervisor"},{"line_number":21,"context_line":"from novaclient.v2.servers import Server"},{"line_number":22,"context_line":"from watcher.applier.actions import base as baction"},{"line_number":23,"context_line":"from watcher.applier.actions import migration"},{"line_number":24,"context_line":"from watcher.common import clients"},{"line_number":25,"context_line":"from watcher.common import exception"},{"line_number":26,"context_line":"from watcher.common import nova_helper"},{"line_number":27,"context_line":"from watcher.tests import base"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class TestMigration(base.TestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3cc40686_08da1ec0","line":27,"range":{"start_line":16,"start_character":0,"end_line":27,"end_character":30},"updated":"2025-11-13 14:54:51.000000000","message":"```suggestion\nfrom unittest import mock\n\nimport jsonschema\n\nfrom novaclient.v2 import hypervisors\nfrom novaclient.v2 import servers\n\nfrom watcher.applier.actions import base as baction\nfrom watcher.applier.actions import migration\nfrom watcher.common import clients\nfrom watcher.common import exception\nfrom watcher.common import nova_helper\nfrom watcher.tests import base\n```\n\nwe do not allow importing anyting other then a toplevel pacakge or a modules\n\nso importing a class `from novaclient.v2.hypervisors import Hypervisor`\nos incorrect\n\nour openstack codeing conveetion also requrie that we spit improt in a specific way\n\nhttps://github.com/openstack/hacking/blob/master/HACKING.rst#real-world-import-order-examples\n\nalthough i thend to be a littel strciter then the minium requried\n\nhttps://github.com/SeanMooney/openstack-ai-style-guide/blob/master/docs/comprehensive-guide.md#2-import-organization-critical-for-ai\n\nspecificlly i do not like mignt import and from import\n\nso while hacking required 4 groups i prefer 8\nwtih serperate \n\nimport\n\nand \n\nfrom ... import ...\n\n\nfor each of Standard library, Third-party libraries, OpenStack libraries and  Local project imports","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from unittest import mock"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import jsonschema"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from novaclient.v2.hypervisors import Hypervisor"},{"line_number":21,"context_line":"from novaclient.v2.servers import Server"},{"line_number":22,"context_line":"from watcher.applier.actions import base as baction"},{"line_number":23,"context_line":"from watcher.applier.actions import migration"},{"line_number":24,"context_line":"from watcher.common import clients"},{"line_number":25,"context_line":"from watcher.common import exception"},{"line_number":26,"context_line":"from watcher.common import nova_helper"},{"line_number":27,"context_line":"from watcher.tests import base"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"class TestMigration(base.TestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"f3ca3dbe_02f08f04","line":27,"range":{"start_line":16,"start_character":0,"end_line":27,"end_character":30},"in_reply_to":"3cc40686_08da1ec0","updated":"2025-11-14 11:59:42.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def test_migration_pre_condition_success(self):"},{"line_number":139,"context_line":"        \"\"\"Test successful pre_condition with all checks passing\"\"\""},{"line_number":140,"context_line":"        instance \u003d mock.Mock("},{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2e0a1faa_ec40baea","line":140,"in_reply_to":"055e6b49_fca0c0fb","updated":"2025-11-14 11:59:42.000000000","message":"Thanks for the long explanation.\n\nI like the idea of avoid using mock at all here when possible. Checking at the novaclient code, I understand the right way of defining a Server is:\n\n```\ninstance_info \u003d {\n    \u0027id\u0027: self.INSTANCE_UUID,\n    \u0027status\u0027: \u0027ACTIVE\u0027,\n    \u0027OS-EXT-SRV-ATTR:host\u0027: \u0027compute1-hostname\u0027\n}\ninstance \u003d servers.Server(servers.ServerManager, info\u003dinstance_info)\n```\n\nIn current state of watcher, wdyt would be more appropiate? I\u0027ll go with ^ in next PS.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"16d68bdb47f072e3b449ac02a5135cbebea1e0ac","unresolved":true,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def test_migration_pre_condition_success(self):"},{"line_number":139,"context_line":"        \"\"\"Test successful pre_condition with all checks passing\"\"\""},{"line_number":140,"context_line":"        instance \u003d mock.Mock("},{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bcadbf3d_782b860d","line":140,"in_reply_to":"2e0a1faa_ec40baea","updated":"2025-11-14 12:18:55.000000000","message":"if that works then go for it.\ni was not quite shorut how tot deal with \u0027OS-EXT-SRV-ATTR:host\u0027 but if we can just pass info as a dict directly then lets do that\n\nits much cleaner and simplifies the code.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def test_migration_pre_condition_success(self):"},{"line_number":139,"context_line":"        \"\"\"Test successful pre_condition with all checks passing\"\"\""},{"line_number":140,"context_line":"        instance \u003d mock.Mock("},{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"055e6b49_fca0c0fb","line":140,"in_reply_to":"846889a2_51434c1f","updated":"2025-11-13 14:54:51.000000000","message":"so in this case you are not creatign the patch in this test fucntion that is being done elsehwere. but teh logical exteneion of its suggetion is that you would do something like this\n\n```\nmock_servers \u003d create_autospec(servers)\ninstance \u003d servers.Server(id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)\n```\n\nhowever an even more correct approch is not to use a mock at all.\n\nwe shoud not use mock for data  only for behavior.\n\nso you shoudl jsut do\n```\ninstance \u003d servers.Server(id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)\n```\n\nthe only thing that makes me hesitent here is we generally shoudl not be passing around server objects form nova client as retrun values form our helper methods like this in the first pace. we shoudl be converting this into our own instance object instead and the client classes and exections shoudl not leak out of the helper classes. so yet again this si existign tech debt.\n\n\n```\ninstance \u003d servers.Server(id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)\n```\n\nis exactly what you woudl do if this was a watcher class\n\nthe only reaosn you woudl ever make that a mock is if you wanted to assert that actions were performed on it i.e. fucntions were called on it.\n\nwe may want to dicsus this more.\n\nyou usage of mocks is better then watcher typically does but i would still not say its necessarily correct, its just a lot more corect then mock.Mock().","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def test_migration_pre_condition_success(self):"},{"line_number":139,"context_line":"        \"\"\"Test successful pre_condition with all checks passing\"\"\""},{"line_number":140,"context_line":"        instance \u003d mock.Mock("},{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"6d3da1cf_21cc1a14","line":140,"in_reply_to":"bcadbf3d_782b860d","updated":"2026-01-15 13:08:30.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":139,"context_line":"        \"\"\"Test successful pre_condition with all checks passing\"\"\""},{"line_number":140,"context_line":"        instance \u003d mock.Mock("},{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        compute_node \u003d mock.Mock(spec\u003dHypervisor,"},{"line_number":145,"context_line":"                                 hypervisor_hostname\u003d\u0027compute2-hostname\u0027,"},{"line_number":146,"context_line":"                                 status\u003d\u0027enabled\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"35fe154f_ee665d5f","line":143,"range":{"start_line":142,"start_character":6,"end_line":143,"end_character":1},"updated":"2025-11-13 14:54:51.000000000","message":"this si also not really correct\n\nagain if the value you are settin is constn usign setattr is almost alwasy incorect\n\nin this case i belive the correct way to set this woudl be \n\n```suggestion\n        instance.set_info(\u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)\n\n```\nhowever i belive this can also jsut be storaged as\n\ninstance \u003d servers.Server(id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027, host\u003d\u0027compute1-hostname\u0027)","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":139,"context_line":"        \"\"\"Test successful pre_condition with all checks passing\"\"\""},{"line_number":140,"context_line":"        instance \u003d mock.Mock("},{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        compute_node \u003d mock.Mock(spec\u003dHypervisor,"},{"line_number":145,"context_line":"                                 hypervisor_hostname\u003d\u0027compute2-hostname\u0027,"},{"line_number":146,"context_line":"                                 status\u003d\u0027enabled\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"c6b7433d_f528c59c","line":143,"range":{"start_line":142,"start_character":6,"end_line":143,"end_character":1},"in_reply_to":"35fe154f_ee665d5f","updated":"2025-11-14 11:59:42.000000000","message":"see my preview comment about how to create the instance.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        \"\"\"Test successful pre_condition with all checks passing\"\"\""},{"line_number":140,"context_line":"        instance \u003d mock.Mock("},{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        compute_node \u003d mock.Mock(spec\u003dHypervisor,"},{"line_number":145,"context_line":"                                 hypervisor_hostname\u003d\u0027compute2-hostname\u0027,"},{"line_number":146,"context_line":"                                 status\u003d\u0027enabled\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5639a1d9_6160d5e3","line":143,"range":{"start_line":142,"start_character":6,"end_line":143,"end_character":1},"in_reply_to":"c6b7433d_f528c59c","updated":"2026-01-15 13:08:30.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        compute_node \u003d mock.Mock(spec\u003dHypervisor,"},{"line_number":145,"context_line":"                                 hypervisor_hostname\u003d\u0027compute2-hostname\u0027,"},{"line_number":146,"context_line":"                                 status\u003d\u0027enabled\u0027)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":149,"context_line":"        self.m_helper.get_compute_node_by_name.return_value \u003d [compute_node]"}],"source_content_type":"text/x-python","patch_set":2,"id":"969f59bd_b69ec2d2","line":146,"range":{"start_line":144,"start_character":8,"end_line":146,"end_character":50},"updated":"2025-11-13 14:54:51.000000000","message":"```suggestion\n        comptue_node \u003d hypervisors.Hypervisor(\n            hypervisor_hostname\u003d\u0027compute2-hostname\u0027, status\u003d\u0027enabled\u0027)\n```","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        compute_node \u003d mock.Mock(spec\u003dHypervisor,"},{"line_number":145,"context_line":"                                 hypervisor_hostname\u003d\u0027compute2-hostname\u0027,"},{"line_number":146,"context_line":"                                 status\u003d\u0027enabled\u0027)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":149,"context_line":"        self.m_helper.get_compute_node_by_name.return_value \u003d [compute_node]"}],"source_content_type":"text/x-python","patch_set":2,"id":"4bc87fc6_62eb5f1c","line":146,"range":{"start_line":144,"start_character":8,"end_line":146,"end_character":50},"in_reply_to":"2e14dfee_0bb814c4","updated":"2026-01-15 13:08:30.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        compute_node \u003d mock.Mock(spec\u003dHypervisor,"},{"line_number":145,"context_line":"                                 hypervisor_hostname\u003d\u0027compute2-hostname\u0027,"},{"line_number":146,"context_line":"                                 status\u003d\u0027enabled\u0027)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":149,"context_line":"        self.m_helper.get_compute_node_by_name.return_value \u003d [compute_node]"}],"source_content_type":"text/x-python","patch_set":2,"id":"e3a2563c_082ba6d0","line":146,"range":{"start_line":144,"start_character":8,"end_line":146,"end_character":50},"in_reply_to":"969f59bd_b69ec2d2","updated":"2025-11-14 11:59:42.000000000","message":"I think this is the right way?:\n\n```\ncompute_node_info \u003d {\n    \u0027id\u0027: \u0027compute2-hostname\u0027,\n    \u0027status\u0027: \u0027enabled\u0027,\n    \u0027hypervisor_hostname\u0027: \u0027compute1-hostname\u0027,\n    \u0027service\u0027: {\n         \u0027host\u0027: \u0027compute1-hostname\u0027\n    }\n}\ncompute_node \u003d hypervisors.Hypervisor(\n    hypervisors.HypervisorManager, info\u003dcompute_node_info)\n```","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"16d68bdb47f072e3b449ac02a5135cbebea1e0ac","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            spec\u003dServer, id\u003dself.INSTANCE_UUID, status\u003d\u0027ACTIVE\u0027)"},{"line_number":142,"context_line":"        setattr(instance, \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027compute1-hostname\u0027)"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        compute_node \u003d mock.Mock(spec\u003dHypervisor,"},{"line_number":145,"context_line":"                                 hypervisor_hostname\u003d\u0027compute2-hostname\u0027,"},{"line_number":146,"context_line":"                                 status\u003d\u0027enabled\u0027)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":149,"context_line":"        self.m_helper.get_compute_node_by_name.return_value \u003d [compute_node]"}],"source_content_type":"text/x-python","patch_set":2,"id":"2e14dfee_0bb814c4","line":146,"range":{"start_line":144,"start_character":8,"end_line":146,"end_character":50},"in_reply_to":"e3a2563c_082ba6d0","updated":"2025-11-14 12:18:55.000000000","message":"yep that sound possible\n\nnova client has some weird indirection though a common Resouce mixing wich makes it less obvious how to manually init it becuase its expectign ot be inited forma a requests reponce object, efffectivly the api body if we can just init inof and then access the data nomrlaly then lets do that.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":148,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":149,"context_line":"        self.m_helper.get_compute_node_by_name.return_value \u003d [compute_node]"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        try:"},{"line_number":152,"context_line":"            self.action.pre_condition()"},{"line_number":153,"context_line":"        except Exception as exc:"},{"line_number":154,"context_line":"            self.fail(exc)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def test_pre_condition_instance_not_found(self):"},{"line_number":157,"context_line":"        \"\"\"Test pre_condition fails when instance doesn\u0027t exist\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"909eef2a_d97c3b83","line":154,"range":{"start_line":151,"start_character":7,"end_line":154,"end_character":26},"updated":"2025-11-13 14:54:51.000000000","message":"```suggestion\n        self.action.pre_condition()\n```\n\nfirstly all uncaught exception in test cause the test to fail automaticaly\nsecond we shoudl not do `except Exception`\n\nthis was wrogn before you change but since you basiclly reqrote the funciton you shoudl fix this as well.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c58b66c98d3cc253239bf66fa2c5b53ca2bc0fd2","unresolved":false,"context_lines":[{"line_number":148,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":149,"context_line":"        self.m_helper.get_compute_node_by_name.return_value \u003d [compute_node]"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        try:"},{"line_number":152,"context_line":"            self.action.pre_condition()"},{"line_number":153,"context_line":"        except Exception as exc:"},{"line_number":154,"context_line":"            self.fail(exc)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def test_pre_condition_instance_not_found(self):"},{"line_number":157,"context_line":"        \"\"\"Test pre_condition fails when instance doesn\u0027t exist\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"50d22e34_c298c669","line":154,"range":{"start_line":151,"start_character":7,"end_line":154,"end_character":26},"in_reply_to":"909eef2a_d97c3b83","updated":"2025-11-14 11:59:42.000000000","message":"Done","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"525a2d5d6e893503aac17eb58b00fd1795b23e80","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        exc \u003d self.assertRaises("},{"line_number":162,"context_line":"            exception.ActionSkipped, self.action.pre_condition)"},{"line_number":163,"context_line":"        self.assertIn(self.INSTANCE_UUID, str(exc))"},{"line_number":164,"context_line":"        self.assertIn(\u0027not found\u0027, str(exc))"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def test_pre_condition_instance_on_wrong_host(self):"},{"line_number":167,"context_line":"        \"\"\"Test pre_condition fails when instance is on wrong host\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"7e7f7942_5d82613c","line":164,"updated":"2025-11-13 14:54:51.000000000","message":"this is a vaild skip reason in my opion but given i quetion some of the other logic in the patch im going to stop reviewing the test here.","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76a5a02c6f42ba6a17c1aebbdd8bec945422f4df","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        exc \u003d self.assertRaises("},{"line_number":162,"context_line":"            exception.ActionSkipped, self.action.pre_condition)"},{"line_number":163,"context_line":"        self.assertIn(self.INSTANCE_UUID, str(exc))"},{"line_number":164,"context_line":"        self.assertIn(\u0027not found\u0027, str(exc))"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def test_pre_condition_instance_on_wrong_host(self):"},{"line_number":167,"context_line":"        \"\"\"Test pre_condition fails when instance is on wrong host\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"606462d8_7e50bfa6","line":164,"in_reply_to":"7e7f7942_5d82613c","updated":"2026-01-15 13:08:30.000000000","message":"Acknowledged","commit_id":"29285117bbdcb0d687b8d8bf2a2669636c71c70f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d048fcb8f0c2c4e5219a97558fab822aaa778ad7","unresolved":true,"context_lines":[{"line_number":171,"context_line":"        self.m_helper.find_instance.side_effect \u003d ("},{"line_number":172,"context_line":"            nova_helper.nvexceptions.NotFound(\u0027404\u0027))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        exc \u003d self.assertRaises("},{"line_number":175,"context_line":"            exception.ActionSkipped, self.action.pre_condition)"},{"line_number":176,"context_line":"        self.assertIn(self.INSTANCE_UUID, str(exc))"},{"line_number":177,"context_line":"        self.assertIn(\u0027not found\u0027, str(exc))"}],"source_content_type":"text/x-python","patch_set":4,"id":"6143f8c7_39fe875f","line":174,"updated":"2025-11-25 15:58:35.000000000","message":"the three asserts can be combined into one using `self.assertRaisesRegex`: \n\n```suggestion\n        self.assertRaisesRegex(\n            exception.ActionSkipped,\n            f\"Instance {self.INSTANCE_UUID} not found\",\n            self.action.pre_condition)\n```","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ac0eae1184926bef971af1aee671333f70291bd0","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        self.m_helper.find_instance.side_effect \u003d ("},{"line_number":172,"context_line":"            nova_helper.nvexceptions.NotFound(\u0027404\u0027))"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        exc \u003d self.assertRaises("},{"line_number":175,"context_line":"            exception.ActionSkipped, self.action.pre_condition)"},{"line_number":176,"context_line":"        self.assertIn(self.INSTANCE_UUID, str(exc))"},{"line_number":177,"context_line":"        self.assertIn(\u0027not found\u0027, str(exc))"}],"source_content_type":"text/x-python","patch_set":4,"id":"be183950_1a5da4af","line":174,"in_reply_to":"6143f8c7_39fe875f","updated":"2025-11-26 14:48:20.000000000","message":"Thanks for the suggestion!","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d048fcb8f0c2c4e5219a97558fab822aaa778ad7","unresolved":true,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        exc \u003d self.assertRaises("},{"line_number":191,"context_line":"            exception.ActionSkipped, self.action.pre_condition)"},{"line_number":192,"context_line":"        self.assertIn(\u0027not running on source node\u0027, str(exc))"},{"line_number":193,"context_line":"        self.assertIn(\u0027compute1-hostname\u0027, str(exc))"}],"source_content_type":"text/x-python","patch_set":4,"id":"3c7331a3_8f62a7c0","line":190,"updated":"2025-11-25 15:58:35.000000000","message":"same point about `assertRaises` and `assertRaisesRegex` applies to all added tests","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ac0eae1184926bef971af1aee671333f70291bd0","unresolved":false,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        self.m_helper.find_instance.return_value \u003d instance"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        exc \u003d self.assertRaises("},{"line_number":191,"context_line":"            exception.ActionSkipped, self.action.pre_condition)"},{"line_number":192,"context_line":"        self.assertIn(\u0027not running on source node\u0027, str(exc))"},{"line_number":193,"context_line":"        self.assertIn(\u0027compute1-hostname\u0027, str(exc))"}],"source_content_type":"text/x-python","patch_set":4,"id":"d41ae104_854c1b69","line":190,"in_reply_to":"3c7331a3_8f62a7c0","updated":"2025-11-26 14:48:20.000000000","message":"Done","commit_id":"08987418c1c5aa059923334348ff5e7228bd4c86"}]}