)]}'
{"nova/exception.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee1760780cbbc9fbda40cbed87d1f3376674175f","unresolved":false,"context_lines":[{"line_number":1230,"context_line":"                \"host node %(host)s.%(node)s.\")"},{"line_number":1231,"context_line":""},{"line_number":1232,"context_line":""},{"line_number":1233,"context_line":"class FileNotFound(NotFound):"},{"line_number":1234,"context_line":"    msg_fmt \u003d _(\"File %(file_path)s could not be found.\")"},{"line_number":1235,"context_line":""},{"line_number":1236,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3f4b6375_adc5733e","line":1233,"range":{"start_line":1233,"start_character":6,"end_line":1233,"end_character":18},"updated":"2017-10-20 20:36:28.000000000","message":"Why not just use this?","commit_id":"71a733a88453f77042cf8070ec4cb767e27741cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee1760780cbbc9fbda40cbed87d1f3376674175f","unresolved":false,"context_lines":[{"line_number":2183,"context_line":""},{"line_number":2184,"context_line":""},{"line_number":2185,"context_line":"class HyperVFileNotExists(NovaException):"},{"line_number":2186,"context_line":"    msg_fmt \u003d _(\"The dir does not exist: %(msg)s\")"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f4b6375_eda24b9a","line":2186,"range":{"start_line":2186,"start_character":21,"end_line":2186,"end_character":24},"updated":"2017-10-20 20:36:28.000000000","message":"directory - consider non-native English speakers (such as yourself) :)","commit_id":"71a733a88453f77042cf8070ec4cb767e27741cd"}],"nova/tests/unit/virt/hyperv/test_livemigrationops.py":[{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"3e05204457fcec7bcd14a65dcbc5916782864ab7","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        self.context \u003d \u0027fake_context\u0027"},{"line_number":35,"context_line":"        self._livemigrops \u003d livemigrationops.LiveMigrationOps()"},{"line_number":36,"context_line":"        self._livemigrops._livemigrutils \u003d mock.MagicMock()"},{"line_number":37,"context_line":"        self._livemigrops._pathutils \u003d mock.MagicMock()"},{"line_number":38,"context_line":"        self._livemigrops._block_dev_man \u003d mock.MagicMock()"},{"line_number":39,"context_line":"        self._pathutils \u003d self._livemigrops._pathutils"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @mock.patch.object(serialconsoleops.SerialConsoleOps,"},{"line_number":42,"context_line":"                       \u0027stop_console_handler\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f201791_e5753433","side":"PARENT","line":39,"range":{"start_line":37,"start_character":8,"end_line":39,"end_character":54},"updated":"2017-06-28 14:20:24.000000000","message":"I don\u0027t see why this change is needed, it induces some unrelated changes.","commit_id":"6e64cb032e56b279acdbee7ae9a0404e7aea99f2"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"553a1dea58e1f0db6d6d2bb0aecf958becfdbbf0","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            mock.sentinel.context, mock_instance, mock.sentinel.src_comp_info,"},{"line_number":220,"context_line":"            mock.sentinel.dest_comp_info)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        self.assertTrue(mock_path.called)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        self.assertEqual(mock_migr_data_cls.return_value, migr_data)"},{"line_number":225,"context_line":"        self.assertEqual(mock_shared.return_value,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f287b81_a62b5380","line":222,"updated":"2017-08-24 12:36:42.000000000","message":"Using \u0027called\u0027 on a central library like os.path is almost always a bad idea as you can\u0027t be sure that it was your code that did the call, or something else somewhere in the code path.\n\nif you must mock os.path.exists and want to assert that it is called, then assert that it has been called with the expected arguments.","commit_id":"7bedf9aaf7f5d93a9bcef725edfa75de71969fd3"}],"nova/virt/hyperv/livemigrationops.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"5d199b0eae2d03517408ca23984f343a11482143","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":136,"context_line":"                    instance_ref.host))"},{"line_number":137,"context_line":"        except OSError as e:"},{"line_number":138,"context_line":"            reason \u003d \u0027Failed to precheck live migration: %s\u0027 % e"},{"line_number":139,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        return migrate_data"}],"source_content_type":"text/x-python","patch_set":2,"id":"df140735_65e76442","line":138,"range":{"start_line":138,"start_character":21,"end_line":138,"end_character":60},"updated":"2017-06-01 13:16:36.000000000","message":"It should be:\n\n_(\u0027Failed to precheck live migration: %s\u0027)","commit_id":"4b2af411ff1a0c1ec39ca1f43114e7a9f2f7b2bd"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"453ab02bde2196020cbd41b1ec442432f1bca716","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":136,"context_line":"                    instance_ref.host))"},{"line_number":137,"context_line":"        except OSError as e:"},{"line_number":138,"context_line":"            reason \u003d \u0027Failed to precheck live migration: %s\u0027 % e"},{"line_number":139,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        return migrate_data"}],"source_content_type":"text/x-python","patch_set":2,"id":"df140735_fd3e5189","line":138,"range":{"start_line":138,"start_character":21,"end_line":138,"end_character":60},"in_reply_to":"df140735_65e76442","updated":"2017-06-02 06:19:45.000000000","message":"Done","commit_id":"4b2af411ff1a0c1ec39ca1f43114e7a9f2f7b2bd"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"034b2f4d9895ce7640be540353c16cb34acc0d6d","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s\u0027) % e"},{"line_number":140,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        return migrate_data"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7f231b9d_708d5030","line":140,"updated":"2017-06-19 21:34:50.000000000","message":"Hmm, no.\n\nIf I\u0027m not mistaken, this would cause nova to disregard this current host as a potential target for live migration, if the source and this node are not on the same shared storage. This would be a regression for the shared-nothing live-migration scenario.\n\nThis should be addressed at the pathutils level, in the check_dirs_shared_storage method, by wrapping the with tempfile.NamedTemporaryFile(dir\u003ddest_dir) as tmp_file: statement in a try-except block and return False.","commit_id":"5d4f992903c29a3085b2b131e2fdb718f3f9ce0f"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"6887e4ea418009ae3e65eae8a7f5e050720e2fe0","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s\u0027) % e"},{"line_number":140,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        return migrate_data"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f201791_65db644e","line":140,"in_reply_to":"7f231b9d_708d5030","updated":"2017-06-28 14:17:27.000000000","message":"TLDR: this is fine IMO.\n\nThe OSError would be raised only if the local or remote paths are unavailable. In this case we do want this host to be disregarded as a potential live migration destination.\n\nIf the paths are in fact available, but do not point to the same shared location, this will just return False.","commit_id":"5d4f992903c29a3085b2b131e2fdb718f3f9ce0f"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"162188e2a645066dc72b00f1f86f4f5a109f8c38","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Unavailable instance location. Exception: %s\u0027) % e"},{"line_number":140,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        return migrate_data"}],"source_content_type":"text/x-python","patch_set":7,"id":"df3967d1_d3e03bfd","line":139,"range":{"start_line":139,"start_character":24,"end_line":139,"end_character":68},"updated":"2017-08-02 09:41:58.000000000","message":"I think this sentence should be rewritten.","commit_id":"d144a867bbcfdaf3f09c71c5763fc88672dffbfc"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"b6125f2dc671b4d5433f80d6b7cda748b5bbd4ad","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            migrate_data.is_shared_instance_path \u003d ("},{"line_number":136,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Unavailable instance location. Exception: %s\u0027) % e"},{"line_number":140,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f436f4f_2e030276","line":138,"updated":"2017-08-17 19:06:28.000000000","message":"This OSError exception raises at the following NamedTemopraryFile call:\n\n 184         with tempfile.NamedTemporaryFile(dir\u003ddest_dir) as tmp_file:\n 185             src_path \u003d os.path.join(src_dir,\n 186                                     os.path.basename(tmp_file.name))\n 187\n 188             shared_storage \u003d os.path.exists(src_path)\n\nSo how about adding try-exception block into the above?\nCurrent additional code is far from the raising point, and that makes maintenance/readability hard.","commit_id":"c0eab45c579b87fff47a01746f53686d801f82ef"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"bd30d90ee8abb4c385d3a59acc9b159d5d4b8706","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            migrate_data.is_shared_instance_path \u003d ("},{"line_number":136,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Unavailable instance location. Exception: %s\u0027) % e"},{"line_number":140,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f436f4f_edc5f20b","line":138,"in_reply_to":"9f436f4f_2e030276","updated":"2017-08-18 07:57:13.000000000","message":"This would imply raising a different exception there (we can\u0027t just use MigrationPreCheckError as that method is not tied to migrations).\n\nAnyway, true that this would be a safer approach.","commit_id":"c0eab45c579b87fff47a01746f53686d801f82ef"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"c050ef89dc2e444121b527f2932baf8e789a74c0","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            migrate_data.is_shared_instance_path \u003d ("},{"line_number":136,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Unavailable instance location. Exception: %s\u0027) % e"},{"line_number":140,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f436f4f_2349cb5f","line":138,"in_reply_to":"9f436f4f_edc5f20b","updated":"2017-08-18 08:25:25.000000000","message":"this would check which exception to be raised hard\n\ne.g we can create a new exception and raised in the place Ken\u0027ichi pointed and catch it here\nor we do some healthy check first then proceed the function\n\nbut I prefer to write some comment given the concern about further reading/maintain?","commit_id":"c0eab45c579b87fff47a01746f53686d801f82ef"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"817da8dc122edc2e60f89fd947e374ee3f6f25b4","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            migrate_data.is_shared_instance_path \u003d ("},{"line_number":136,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            # The remote instance dir might not exist and lead to OSError"},{"line_number":140,"context_line":"            # exception, see https://launchpad.net/bugs/1694636 for more info"},{"line_number":141,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s\u0027) % e"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f436f4f_3920d8b0","line":138,"updated":"2017-08-18 09:42:57.000000000","message":"I agree with earlier comments that catching a OSError here, such a long way away from the place where the error happened is challenging. Especially since OSError is a pretty broad exception.\n\nThere\u0027s some chance it is something else, in which case it would be useful to have a log entry.","commit_id":"4afa3c3cee41ec40ea4ccb19969a2b439678d6f4"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"30158e17c6cd821f093b22fa7ee19aeb50585963","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            migrate_data.is_shared_instance_path \u003d ("},{"line_number":136,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            # The remote instance dir might not exist and lead to OSError"},{"line_number":140,"context_line":"            # exception, see https://launchpad.net/bugs/1694636 for more info"},{"line_number":141,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s\u0027) % e"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f287b81_35db12ee","line":138,"in_reply_to":"7f287b81_c6f558cd","updated":"2017-08-24 09:04:20.000000000","message":"ok... let me try to fix it, thanks","commit_id":"4afa3c3cee41ec40ea4ccb19969a2b439678d6f4"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"ea800e52f5389d648eaf7781586fe691db6c3266","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            migrate_data.is_shared_instance_path \u003d ("},{"line_number":136,"context_line":"                self._pathutils.check_remote_instances_dir_shared("},{"line_number":137,"context_line":"                    instance_ref.host))"},{"line_number":138,"context_line":"        except OSError as e:"},{"line_number":139,"context_line":"            # The remote instance dir might not exist and lead to OSError"},{"line_number":140,"context_line":"            # exception, see https://launchpad.net/bugs/1694636 for more info"},{"line_number":141,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s\u0027) % e"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f287b81_c6f558cd","line":138,"in_reply_to":"9f436f4f_3920d8b0","updated":"2017-08-23 16:42:38.000000000","message":"\u003e OSError is a pretty broad exception.\n\nYeah, that is true and why I -1d on the previous PS.","commit_id":"4afa3c3cee41ec40ea4ccb19969a2b439678d6f4"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"c907dbc8222964c16f47a11eed2ac5ae3c61d1a7","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                  instance\u003dinstance_ref)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        dest_path \u003d self._pathutils.get_instances_dir(instance_ref.host)"},{"line_number":136,"context_line":"        if not os.path.exists(dest_path):"},{"line_number":137,"context_line":"            # The remote instance dir might not exist and lead to OSError"},{"line_number":138,"context_line":"            # exception, see https://launchpad.net/bugs/1694636 for more info"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s does not\u0027"},{"line_number":140,"context_line":"                       \u0027exist.\u0027) % dest_path"},{"line_number":141,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        migrate_data \u003d migrate_data_obj.HyperVLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f287b81_239e3554","line":140,"range":{"start_line":136,"start_character":8,"end_line":140,"end_character":44},"updated":"2017-08-24 12:16:01.000000000","message":"I\u0027d stick with the other approach, catching OSError in \u0027check_remote_instances_dir_shared\u0027.\n\nWe\u0027re just checking if the destination appears to exist. The source may also be unavailable (when accessed by the destination node) or we may not have write access. In both cases, we\u0027d still want to raise a MigrationPreCheckError.","commit_id":"7bedf9aaf7f5d93a9bcef725edfa75de71969fd3"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"553a1dea58e1f0db6d6d2bb0aecf958becfdbbf0","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            # The remote instance dir might not exist and lead to OSError"},{"line_number":138,"context_line":"            # exception, see https://launchpad.net/bugs/1694636 for more info"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s does not\u0027"},{"line_number":140,"context_line":"                       \u0027exist.\u0027) % dest_path"},{"line_number":141,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        migrate_data \u003d migrate_data_obj.HyperVLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f287b81_e3026d0b","line":140,"in_reply_to":"7f287b81_239e3554","updated":"2017-08-24 12:36:42.000000000","message":"Yeah, the goal from the previous advice was not to have the code change, but to narrow where the error was being caught so you could be certain that as the code evolved you wouldn\u0027t accidentally catch an unexpected exception.","commit_id":"7bedf9aaf7f5d93a9bcef725edfa75de71969fd3"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"adbada6ce738571d12d7fefd500b3aa1825637b5","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            # The remote instance dir might not exist and lead to OSError"},{"line_number":138,"context_line":"            # exception, see https://launchpad.net/bugs/1694636 for more info"},{"line_number":139,"context_line":"            reason \u003d _(\u0027Failed to precheck live migration: %s does not\u0027"},{"line_number":140,"context_line":"                       \u0027exist.\u0027) % dest_path"},{"line_number":141,"context_line":"            raise exception.MigrationPreCheckError(reason\u003dreason)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        migrate_data \u003d migrate_data_obj.HyperVLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f287b81_dfcccb6b","line":140,"in_reply_to":"7f287b81_e3026d0b","updated":"2017-08-25 07:00:50.000000000","message":"ok, let me see how to do the change, thanks","commit_id":"7bedf9aaf7f5d93a9bcef725edfa75de71969fd3"}],"nova/virt/hyperv/pathutils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee1760780cbbc9fbda40cbed87d1f3376674175f","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                                        os.path.basename(tmp_file.name))"},{"line_number":190,"context_line":"                shared_storage \u003d os.path.exists(src_path)"},{"line_number":191,"context_line":"        except OSError as e:"},{"line_number":192,"context_line":"            raise exception.HyperVFileNotExists(msg\u003dsix.text_type(e))"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        return shared_storage"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3f4b6375_2de743e7","line":192,"range":{"start_line":192,"start_character":28,"end_line":192,"end_character":47},"updated":"2017-10-20 20:36:28.000000000","message":"FileNotFound ?","commit_id":"71a733a88453f77042cf8070ec4cb767e27741cd"}]}
