)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e10b57a1f27b43d68fe1af217577b7d4a01b4885","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"26663905_1669a017","updated":"2025-05-16 16:52:28.000000000","message":"LGTM, thanks!","commit_id":"63626d6fc3e9668c586bb172e036533d41570ecf"}],"watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"56517ebb6f4b3e878e3523eb7a032f663647da4c","unresolved":true,"context_lines":[{"line_number":318,"context_line":"        self.m_n_helper.get_instance_list.return_value \u003d []"},{"line_number":319,"context_line":"        solution \u003d self.strategy.execute()"},{"line_number":320,"context_line":"        migration_params \u003d solution.actions[0][\u0027input_parameters\u0027]"},{"line_number":321,"context_line":"        # since we have not passed \u0027dst_pool\u0027 in the input, we should not have"},{"line_number":322,"context_line":"        # a destination_node in the generated migration action"},{"line_number":323,"context_line":"        # self.assertNotIn(\u0027destination_node\u0027, migration_params)"},{"line_number":324,"context_line":"        # temporarily make the test pass, delete and use the above assert in"},{"line_number":325,"context_line":"        # followup"},{"line_number":326,"context_line":"        self.assertIsNone(migration_params[\u0027destination_node\u0027])"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def test_execute_retype_volume(self):"},{"line_number":329,"context_line":"        volume_on_src2 \u003d self.fake_volume(host\u003d\"src2@back1#pool1\","}],"source_content_type":"text/x-python","patch_set":5,"id":"05e8babe_f4b541fa","line":326,"range":{"start_line":321,"start_character":0,"end_line":326,"end_character":63},"updated":"2025-05-16 07:11:00.000000000","message":"I understant this is irelevant for this test, right? anyway, we are modifying the test in the fixing one, so probably not a big problem.","commit_id":"63626d6fc3e9668c586bb172e036533d41570ecf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"b59d0cf2ae7537eca7495f416e863a09dcab1b27","unresolved":true,"context_lines":[{"line_number":318,"context_line":"        self.m_n_helper.get_instance_list.return_value \u003d []"},{"line_number":319,"context_line":"        solution \u003d self.strategy.execute()"},{"line_number":320,"context_line":"        migration_params \u003d solution.actions[0][\u0027input_parameters\u0027]"},{"line_number":321,"context_line":"        # since we have not passed \u0027dst_pool\u0027 in the input, we should not have"},{"line_number":322,"context_line":"        # a destination_node in the generated migration action"},{"line_number":323,"context_line":"        # self.assertNotIn(\u0027destination_node\u0027, migration_params)"},{"line_number":324,"context_line":"        # temporarily make the test pass, delete and use the above assert in"},{"line_number":325,"context_line":"        # followup"},{"line_number":326,"context_line":"        self.assertIsNone(migration_params[\u0027destination_node\u0027])"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def test_execute_retype_volume(self):"},{"line_number":329,"context_line":"        volume_on_src2 \u003d self.fake_volume(host\u003d\"src2@back1#pool1\","}],"source_content_type":"text/x-python","patch_set":5,"id":"36b4a0de_d99f792d","line":326,"range":{"start_line":321,"start_character":0,"end_line":326,"end_character":63},"in_reply_to":"05e8babe_f4b541fa","updated":"2025-05-16 07:29:31.000000000","message":"This is relevant, although not very obvious why. The actual problem of the bug is that when the parameters are omitted in the input, they are added to the migration_params dict, with a value of None, which causes the problem, so this assert is checking that the root cause of the problem occurs","commit_id":"63626d6fc3e9668c586bb172e036533d41570ecf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0313fb3af07b195eee25fc922f990d71f64f2ab4","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        self.m_n_helper.get_instance_list.return_value \u003d []"},{"line_number":319,"context_line":"        solution \u003d self.strategy.execute()"},{"line_number":320,"context_line":"        migration_params \u003d solution.actions[0][\u0027input_parameters\u0027]"},{"line_number":321,"context_line":"        # since we have not passed \u0027dst_pool\u0027 in the input, we should not have"},{"line_number":322,"context_line":"        # a destination_node in the generated migration action"},{"line_number":323,"context_line":"        # self.assertNotIn(\u0027destination_node\u0027, migration_params)"},{"line_number":324,"context_line":"        # temporarily make the test pass, delete and use the above assert in"},{"line_number":325,"context_line":"        # followup"},{"line_number":326,"context_line":"        self.assertIsNone(migration_params[\u0027destination_node\u0027])"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def test_execute_retype_volume(self):"},{"line_number":329,"context_line":"        volume_on_src2 \u003d self.fake_volume(host\u003d\"src2@back1#pool1\","}],"source_content_type":"text/x-python","patch_set":5,"id":"b6ec42f7_bf7c01f4","line":326,"range":{"start_line":321,"start_character":0,"end_line":326,"end_character":63},"in_reply_to":"193c21d4_644e2f8f","updated":"2025-05-16 10:47:15.000000000","message":"hum, that\u0027s unfonate  \nhttps://github.com/openstack/watcher/blob/86a260a2c703cd6f84a5b4aee104ab2b79cef50f/watcher/applier/actions/volume_migration.py#L98-L103\n\n\ni suspect that we are going ot turn the storag pool case into a 400 error isntead.\nwith that said cinder does have a schduler i just dont know if it makes sense to allow it to choose and if it supprot chooing when we are doign a volume migration.\n\nso we can revisit what the final behavior should be in the fix patch.\n\n\nself.assertNotIn(\u0027destination_node\u0027, migration_params) might be ok if cinder can make a good choice itself. otherwise if it cant we shoudl make the dest_pool a required parmater and return a 400 if its not present.","commit_id":"63626d6fc3e9668c586bb172e036533d41570ecf"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9198536e12b776185ff192b90ca9eed81f6b63bd","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        self.m_n_helper.get_instance_list.return_value \u003d []"},{"line_number":319,"context_line":"        solution \u003d self.strategy.execute()"},{"line_number":320,"context_line":"        migration_params \u003d solution.actions[0][\u0027input_parameters\u0027]"},{"line_number":321,"context_line":"        # since we have not passed \u0027dst_pool\u0027 in the input, we should not have"},{"line_number":322,"context_line":"        # a destination_node in the generated migration action"},{"line_number":323,"context_line":"        # self.assertNotIn(\u0027destination_node\u0027, migration_params)"},{"line_number":324,"context_line":"        # temporarily make the test pass, delete and use the above assert in"},{"line_number":325,"context_line":"        # followup"},{"line_number":326,"context_line":"        self.assertIsNone(migration_params[\u0027destination_node\u0027])"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    def test_execute_retype_volume(self):"},{"line_number":329,"context_line":"        volume_on_src2 \u003d self.fake_volume(host\u003d\"src2@back1#pool1\","}],"source_content_type":"text/x-python","patch_set":5,"id":"193c21d4_644e2f8f","line":326,"range":{"start_line":321,"start_character":0,"end_line":326,"end_character":63},"in_reply_to":"36b4a0de_d99f792d","updated":"2025-05-16 08:13:00.000000000","message":"right, actually, it\u0027s not irrelevant 😊 . What i was missing is that the parameter name for the destination pool in the volume_migrate action is also `destination_node`. That\u0027s tricky.","commit_id":"63626d6fc3e9668c586bb172e036533d41570ecf"}]}