)]}'
{"glance/async_/flows/api_image_import.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"60199b359af4f1bcfd02e44534e6eb8018ea9e49","unresolved":false,"context_lines":[{"line_number":477,"context_line":"                     \"all_stores_must_succeed is set to false, continue.\") %"},{"line_number":478,"context_line":"                   {\u0027task_id\u0027: self.task_id, \u0027task_type\u0027: self.task_type})"},{"line_number":479,"context_line":"            LOG.warning(msg)"},{"line_number":480,"context_line":"            if self.backend is not None:"},{"line_number":481,"context_line":"                action.add_failed_stores([self.backend])"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"        if self.backend is not None:"},{"line_number":484,"context_line":"            action.remove_importing_stores([self.backend])"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_584fc791","side":"PARENT","line":481,"range":{"start_line":480,"start_character":12,"end_line":481,"end_character":56},"updated":"2020-08-12 17:06:21.000000000","message":"This needs to be here as it is as if all_stores_must_succeed is true it will never add the failure store to the list as we don\u0027t call revert in that case.","commit_id":"ec372377b1e69eb143e4d08b0041b8199459ee15"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"554084bb8af49d8d394695fd1a1eb2d0fb0b8d75","unresolved":false,"context_lines":[{"line_number":477,"context_line":"                     \"all_stores_must_succeed is set to false, continue.\") %"},{"line_number":478,"context_line":"                   {\u0027task_id\u0027: self.task_id, \u0027task_type\u0027: self.task_type})"},{"line_number":479,"context_line":"            LOG.warning(msg)"},{"line_number":480,"context_line":"            if self.backend is not None:"},{"line_number":481,"context_line":"                action.add_failed_stores([self.backend])"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"        if self.backend is not None:"},{"line_number":484,"context_line":"            action.remove_importing_stores([self.backend])"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_382db3a8","side":"PARENT","line":481,"range":{"start_line":480,"start_character":12,"end_line":481,"end_character":56},"in_reply_to":"9f560f44_584fc791","updated":"2020-08-12 17:08:03.000000000","message":"Ah, right, I made this change while tweaking the unit tests but forgot that we\u0027d skip in that case.","commit_id":"ec372377b1e69eb143e4d08b0041b8199459ee15"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"528e9588110b8e39db28bf4e02be9dcbe1ced3a8","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        with self.action_wrapper as action:"},{"line_number":516,"context_line":"            action.remove_location_for_store(self.backend)"},{"line_number":517,"context_line":"            action.remove_importing_stores([self.backend])"},{"line_number":518,"context_line":"            if isinstance(result, taskflow.types.failure.Failure):"},{"line_number":519,"context_line":"                # We are the store that failed, so add us to the failed list"},{"line_number":520,"context_line":"                action.add_failed_stores([self.backend])"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"class _VerifyImageState(task.Task):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_9fcbbca3","line":520,"range":{"start_line":518,"start_character":0,"end_line":520,"end_character":56},"updated":"2020-08-18 16:44:38.000000000","message":"We should be adding this to the failed stores anyways as we\u0027re removing the store on L: 516 as the data is not there even it was part of the request.\n\nI don\u0027t know what else might cause us calling this revert than the task itself failing and if the answer is nothing, the check in L 518 is redundant.\n\nOr what we likely should do is to move that L 516 under this check so we remove the location data only if we really failed.","commit_id":"4da88018bc2eb700952dbeac70bd9557ff963e2e"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"811062b8b231fc50f0352a935c8e73134bd5f48a","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        with self.action_wrapper as action:"},{"line_number":516,"context_line":"            action.remove_location_for_store(self.backend)"},{"line_number":517,"context_line":"            action.remove_importing_stores([self.backend])"},{"line_number":518,"context_line":"            if isinstance(result, taskflow.types.failure.Failure):"},{"line_number":519,"context_line":"                # We are the store that failed, so add us to the failed list"},{"line_number":520,"context_line":"                action.add_failed_stores([self.backend])"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"class _VerifyImageState(task.Task):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_1bb85fc6","line":520,"range":{"start_line":518,"start_character":0,"end_line":520,"end_character":56},"in_reply_to":"9f560f44_3f65501e","updated":"2020-08-18 17:52:02.000000000","message":"\u003e IMO user will able to understand that his import operation\n \u003e has been stopped when he  will see that all_stores_must_succeed is\n \u003e True and there is one store in the os_glance_failed_import list.\n\nThis was my take as well. I would expect if all must succeed, and I only see a few, then that would give me a clue what happened.","commit_id":"4da88018bc2eb700952dbeac70bd9557ff963e2e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"e749a540197526c8e4352634083bb7b7be72287b","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        with self.action_wrapper as action:"},{"line_number":516,"context_line":"            action.remove_location_for_store(self.backend)"},{"line_number":517,"context_line":"            action.remove_importing_stores([self.backend])"},{"line_number":518,"context_line":"            if isinstance(result, taskflow.types.failure.Failure):"},{"line_number":519,"context_line":"                # We are the store that failed, so add us to the failed list"},{"line_number":520,"context_line":"                action.add_failed_stores([self.backend])"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"class _VerifyImageState(task.Task):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_ff1598e6","line":520,"range":{"start_line":518,"start_character":0,"end_line":520,"end_character":56},"in_reply_to":"9f560f44_9fcbbca3","updated":"2020-08-18 16:51:40.000000000","message":"it was my suggestion to add only store to failed list which caused the actual failure (even dan was insisting to add all the stores which will be reverted). I insisted the same because when you go through the logs you can find that the image was imported to some stores before it was failed while importing to \u0027xyz\u0027 store which caused removing the data from previous stores.","commit_id":"4da88018bc2eb700952dbeac70bd9557ff963e2e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8fa604d46c5f2f49d23793de51254ca86c833d73","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        with self.action_wrapper as action:"},{"line_number":516,"context_line":"            action.remove_location_for_store(self.backend)"},{"line_number":517,"context_line":"            action.remove_importing_stores([self.backend])"},{"line_number":518,"context_line":"            if isinstance(result, taskflow.types.failure.Failure):"},{"line_number":519,"context_line":"                # We are the store that failed, so add us to the failed list"},{"line_number":520,"context_line":"                action.add_failed_stores([self.backend])"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"class _VerifyImageState(task.Task):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_3f65501e","line":520,"range":{"start_line":518,"start_character":0,"end_line":520,"end_character":56},"in_reply_to":"9f560f44_bff3209f","updated":"2020-08-18 17:17:04.000000000","message":"In the spec it is not mentioned after deleting the data from previous stores they will be shown in the os_glance_failed_import list. IMO user will able to understand that his import operation has been stopped when he  will see that all_stores_must_succeed is True and there is one store in the os_glance_failed_import list.\n\nI am also open for adding all the stores to failed list as dan and you find it more user friendly.","commit_id":"4da88018bc2eb700952dbeac70bd9557ff963e2e"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"37a9fd17c7c3ebfe0931ed1046c69da5be9504e0","unresolved":false,"context_lines":[{"line_number":515,"context_line":"        with self.action_wrapper as action:"},{"line_number":516,"context_line":"            action.remove_location_for_store(self.backend)"},{"line_number":517,"context_line":"            action.remove_importing_stores([self.backend])"},{"line_number":518,"context_line":"            if isinstance(result, taskflow.types.failure.Failure):"},{"line_number":519,"context_line":"                # We are the store that failed, so add us to the failed list"},{"line_number":520,"context_line":"                action.add_failed_stores([self.backend])"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"class _VerifyImageState(task.Task):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_bff3209f","line":520,"range":{"start_line":518,"start_character":0,"end_line":520,"end_character":56},"in_reply_to":"9f560f44_ff1598e6","updated":"2020-08-18 17:02:00.000000000","message":"But that\u0027s not what the spec [0] says. Unless I understood the failure differently than the rest.\n\nThe whole point of those two parameters were to track the progress which stores of the request are done and which are not as per of the store list. So if we add only one store into the failure list it may leave the user wondering what happened to the rest of the stores in for that import. It will be impossible to know if that import is still running or not.\n\n[0] https://specs.openstack.org/openstack/glance-specs/specs/ussuri/approved/glance/import-multi-stores.html#proposed-change","commit_id":"4da88018bc2eb700952dbeac70bd9557ff963e2e"}],"glance/tests/unit/async_/flows/test_api_image_import.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"aeb1597a83bfef9ea56969cd14e774f4464e57d0","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        image_import.revert(None)"},{"line_number":300,"context_line":"        self.assertEqual(\u0027store2\u0027, image.extra_properties[pend_key])"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"        try:"},{"line_number":303,"context_line":"            raise Exception(\u0027foo\u0027)"},{"line_number":304,"context_line":"        except Exception:"},{"line_number":305,"context_line":"            fake_exc_info \u003d sys.exc_info()"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"        extra_properties \u003d {\"os_glance_importing_to_stores\": \"store1,store2\"}"},{"line_number":308,"context_line":"        image_import.revert(taskflow.types.failure.Failure(fake_exc_info))"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_3f9d50bb","line":305,"range":{"start_line":302,"start_character":8,"end_line":305,"end_character":42},"updated":"2020-08-18 16:39:29.000000000","message":"Hah, that works.","commit_id":"4da88018bc2eb700952dbeac70bd9557ff963e2e"}]}