)]}'
{"nova/api/openstack/compute/views/servers.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"50a40b44ffa6de22fae814acab98e5200e289ab7","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        }"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a6eadb0_0ca9867b","line":267,"updated":"2016-12-14 22:18:40.000000000","message":"Note that this comment is actually incorrect. All instances will actually already have their fault information already populate because InstanceList.fill_faults() is called from the controller:\n\nhttps://github.com/openstack/nova/blob/921db88c52b0644c575c878ac3d9993d503e22a8/nova/api/openstack/compute/servers.py#L358\n\nNote that the view\u0027s _get_fault() isn\u0027t called when index() is called on the controller, only detail(), thus every time _get_fault() is called here, the instance\u0027s fault information will already be populated from the call to InstanceList.fill_faults()\n\nIn practice, what this means is that this logic here about targeting one or more cells depending on the instance mapping will need to go into the InstanceList.fill_faults() method here:\n\nhttps://github.com/openstack/nova/blob/921db88c52b0644c575c878ac3d9993d503e22a8/nova/objects/instance.py#L1339","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6772d89595037ec8dd9f38d6a3980f0c5688b0ea","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        }"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_287d986e","line":267,"in_reply_to":"1a6eadb0_0ca9867b","updated":"2017-01-03 15:47:27.000000000","message":"Jay, https://github.com/openstack/nova/blob/921db88c52b0644c575c878ac3d9993d503e22a8/nova/api/openstack/compute/servers.py#L358 is only called when listing servers with details. We can still hit this code for just showing details on a specific server here:\n\nhttps://github.com/openstack/nova/blob/921db88c52b0644c575c878ac3d9993d503e22a8/nova/api/openstack/compute/servers.py#L511\n\nIn that GET on a single server there is no InstanceList involved so fill_faults wouldn\u0027t be called.","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e354f51eb61d4f6671f96fa78c501010da0779cb","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        }"},{"line_number":265,"context_line":""},{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a6eadb0_ba189cd4","line":267,"in_reply_to":"1a6eadb0_0ca9867b","updated":"2016-12-15 14:55:55.000000000","message":"This is a specific fix to code that definitely follows this path. Remove this patch and run functional tests and you will see what breaks. It\u0027s definitely lazy-loaded here in the detail case.\n\nYes, the other area needs to be augmented as well (as well as about a hundred other places). However, this is minimal change to be able to get the last patch in this series to land, which is what I\u0027ve been going for.","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"131ca5573df8cb255ca3bfc1dd1b682d8698dd4e","unresolved":false,"context_lines":[{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_5e2452ae","line":269,"updated":"2017-01-03 16:26:24.000000000","message":"Can\u0027t we avoid this by checking something like:\n\n  if \u0027fault\u0027 in instance?\n    fault \u003d instance.fault\n  else:\n    # lookup the instance mapping stuff...","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ee03422940bc8bdeb51d979131bec784f9a1130","unresolved":false,"context_lines":[{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_83bade08","line":269,"in_reply_to":"ba5201f7_1d272d13","updated":"2017-01-03 19:28:27.000000000","message":"I believe that when listing servers with details the instance will already have the fault field set per:\n\nhttps://github.com/openstack/nova/blob/921db88c52b0644c575c878ac3d9993d503e22a8/nova/api/openstack/compute/servers.py#L358","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"21dc0885cbea7f9976ba3577c991cc4f17dc6e4f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_de476249","line":269,"in_reply_to":"ba5201f7_5e2452ae","updated":"2017-01-03 16:26:42.000000000","message":"-1 for this.","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5646acac008762d5902041d2ca440acbe6b2772f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_1d272d13","line":269,"in_reply_to":"ba5201f7_5e2452ae","updated":"2017-01-03 18:31:06.000000000","message":"I\u0027m not sure we\u0027ll ever get here with a fault expanded, despite the vagueness of the comment above. But sure, I\u0027ll do that.","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5fee6ec3a502e9d915a1fcc7ec82a5cc68b6ea1f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_c3b946cd","line":269,"in_reply_to":"ba5201f7_83bade08","updated":"2017-01-03 19:36:44.000000000","message":"Matt, that\u0027s what I thought too (see my comments above), but apparently **something** about the GET /servers/detail path of the OSAPIFixture.admin_api.get_servers() call is not calling fill_faults()...","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ef572ef026e56c000c9341aab7cb110b12b245e2","unresolved":false,"context_lines":[{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_4322b602","line":269,"in_reply_to":"ba5201f7_c3b946cd","updated":"2017-01-03 19:40:44.000000000","message":"Jay, I guess it depends on if get_servers() in the fixture client is called with detail\u003dTrue or not:\n\nhttps://github.com/openstack/nova/blob/a7095c440ec514826b8d49d2aaf547d6254789d3/nova/tests/functional/api/client.py#L254","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0750baeef7ecb1177957351e61799cae18f13734","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"},{"line_number":273,"context_line":"                # to load it the old way, which may be because we\u0027re still"},{"line_number":274,"context_line":"                # pre-scheduling but have fault set as a result."},{"line_number":275,"context_line":"                fault \u003d instance.fault"},{"line_number":276,"context_line":"            else:"},{"line_number":277,"context_line":"                with nova_context.target_cell(instance._context,"},{"line_number":278,"context_line":"                                              mapping.cell_mapping):"},{"line_number":279,"context_line":"                    fault \u003d instance.fault"},{"line_number":280,"context_line":"        except exception.InstanceMappingNotFound:"},{"line_number":281,"context_line":"            # NOTE(danms): No instance mapping at all, which likely"},{"line_number":282,"context_line":"            # means a legacy environment or instance."}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_63cee1b5","line":279,"range":{"start_line":271,"start_character":12,"end_line":279,"end_character":42},"updated":"2017-01-03 16:08:52.000000000","message":"We apparently don\u0027t have any unit testing of this? I put exceptions in both blocks and test_serversV21 passed. Maybe something else is testing this that I didn\u0027t run though.","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73686bc05939eceec8406da9d21e4527b0c33461","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":270,"context_line":"                request.environ[\u0027nova.context\u0027], instance.uuid)"},{"line_number":271,"context_line":"            if mapping.cell_mapping is None:"},{"line_number":272,"context_line":"                # NOTE(danms): If no cell mapping is in place, we just have"},{"line_number":273,"context_line":"                # to load it the old way, which may be because we\u0027re still"},{"line_number":274,"context_line":"                # pre-scheduling but have fault set as a result."},{"line_number":275,"context_line":"                fault \u003d instance.fault"},{"line_number":276,"context_line":"            else:"},{"line_number":277,"context_line":"                with nova_context.target_cell(instance._context,"},{"line_number":278,"context_line":"                                              mapping.cell_mapping):"},{"line_number":279,"context_line":"                    fault \u003d instance.fault"},{"line_number":280,"context_line":"        except exception.InstanceMappingNotFound:"},{"line_number":281,"context_line":"            # NOTE(danms): No instance mapping at all, which likely"},{"line_number":282,"context_line":"            # means a legacy environment or instance."}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_1eed6a25","line":279,"range":{"start_line":271,"start_character":12,"end_line":279,"end_character":42},"in_reply_to":"ba5201f7_63cee1b5","updated":"2017-01-03 16:28:17.000000000","message":"The functional tests hit these conditions so ignore this comment/question.","commit_id":"5e6364171dc0bfe24fb3a4fce1a35c31a7aba2ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"64745abf3308d0a7efa348fb5cb04a2d5c1cce18","unresolved":false,"context_lines":[{"line_number":266,"context_line":"    def _get_fault(self, request, instance):"},{"line_number":267,"context_line":"        # This can result in a lazy load of the fault information"},{"line_number":268,"context_line":"        try:"},{"line_number":269,"context_line":"            if \u0027fault\u0027 in instance:"},{"line_number":270,"context_line":"                fault \u003d instance.fault"},{"line_number":271,"context_line":"            else:"},{"line_number":272,"context_line":"                mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":273,"context_line":"                    request.environ[\u0027nova.context\u0027], instance.uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5201f7_54598a59","line":270,"range":{"start_line":269,"start_character":12,"end_line":270,"end_character":38},"updated":"2017-01-03 21:23:06.000000000","message":"nit: this doesn\u0027t need to be in the try/except","commit_id":"a528c6f66c965acbc0ef71bb9e3c79d9af823503"}],"nova/tests/unit/api/openstack/compute/test_serversV21.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"43365f32a125486c59b82fe1ebd229013d9e57eb","unresolved":false,"context_lines":[{"line_number":4056,"context_line":""},{"line_number":4057,"context_line":"        mock_im.side_effect \u003d exception.InstanceMappingNotFound(uuid\u003d\u0027foo\u0027)"},{"line_number":4058,"context_line":""},{"line_number":4059,"context_line":"        self.request.context \u003d context.RequestContext(\u0027fake\u0027, \u0027fake\u0027)"},{"line_number":4060,"context_line":"        self.view_builder.show(self.request, self.instance)"},{"line_number":4061,"context_line":"        mock_im.assert_called_once_with(mock.ANY, self.uuid)"},{"line_number":4062,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba5201f7_ef38eb3f","line":4059,"range":{"start_line":4059,"start_character":8,"end_line":4059,"end_character":69},"updated":"2017-01-03 21:45:05.000000000","message":"nit: this is probably just copy/pasta but it\u0027s already done for you in the setUp.","commit_id":"a9a2bca070b992bcf02628c1d0d394989f0a0d03"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"43365f32a125486c59b82fe1ebd229013d9e57eb","unresolved":false,"context_lines":[{"line_number":4068,"context_line":"                                             message\u003d\"No valid host was found\")"},{"line_number":4069,"context_line":"        self.instance[\u0027fault\u0027] \u003d fault"},{"line_number":4070,"context_line":""},{"line_number":4071,"context_line":"        self.request.context \u003d context.RequestContext(\u0027fake\u0027, \u0027fake\u0027)"},{"line_number":4072,"context_line":"        self.view_builder.show(self.request, self.instance)"},{"line_number":4073,"context_line":"        self.assertFalse(mock_im.called)"},{"line_number":4074,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba5201f7_0f40cfa4","line":4071,"updated":"2017-01-03 21:45:05.000000000","message":"ditto","commit_id":"a9a2bca070b992bcf02628c1d0d394989f0a0d03"}]}