)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cf045f4b014c1110c7ecb51e38f423b251922b94","unresolved":false,"context_lines":[{"line_number":21,"context_line":"Honoring the instance list command will come later."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ib984c30543acb3ca9cb95fb53d44d9ded0f5a5c8"},{"line_number":24,"context_line":"Partially-implements: bp cells-scheduling-interaction"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"1af94dfe_c900fb04","line":24,"range":{"start_line":24,"start_character":25,"end_line":24,"end_character":53},"updated":"2016-03-18 17:31:42.000000000","message":"add-buildrequest-obj","commit_id":"83e167f0129c725f0f3d75643337985a79270465"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"711adf79b4b3aa03ff8b06d3e256527f75e9572d","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Until instance creation is moved to after scheduling this is mostly a"},{"line_number":19,"context_line":"noop."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Honoring the instance list command will come later."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ib984c30543acb3ca9cb95fb53d44d9ded0f5a5c8"},{"line_number":24,"context_line":"Partially-implements: bp add-buildrequest-obj"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":62,"id":"9ad45d7e_a4bd9b04","line":21,"updated":"2016-08-12 13:49:02.000000000","message":"For instance list command we should load a bunch of instances and build_requests and repeat just the same logic, right?","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d70b00dae0c516d9d711cb3ee65fb88f46eb98ad","unresolved":false,"context_lines":[{"line_number":18,"context_line":"Until instance creation is moved to after scheduling this is mostly a"},{"line_number":19,"context_line":"noop."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Honoring the instance list command will come later."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ib984c30543acb3ca9cb95fb53d44d9ded0f5a5c8"},{"line_number":24,"context_line":"Partially-implements: bp add-buildrequest-obj"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":62,"id":"9ad45d7e_27023b70","line":21,"in_reply_to":"9ad45d7e_a4bd9b04","updated":"2016-08-12 16:58:56.000000000","message":"Mostly. It\u0027s a bit different because we have to account for filtering and sorting the results. But once we have a filtered/sorted list of instances/build_requests it\u0027s mostly the same thing.","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"}],"nova/compute/api.py":[{"author":{"_account_id":17139,"name":"Chuck Carmack","email":"carmack@us.ibm.com","username":"ccarmack"},"change_message_id":"146ce58f21f4a27ffa81da9825734afc85cbb777","unresolved":false,"context_lines":[{"line_number":930,"context_line":"                        base_options[\u0027access_ip_v4\u0027],"},{"line_number":931,"context_line":"                        base_options[\u0027access_ip_v6\u0027], base_options[\u0027progress\u0027],"},{"line_number":932,"context_line":"                        base_options[\u0027vm_state\u0027], base_options[\u0027task_states\u0027],"},{"line_number":933,"context_line":"                        base_options[\u0027display_name\u0027], security_groups,"},{"line_number":934,"context_line":"                        block_device_mapping, shutdown_terminate)"},{"line_number":935,"context_line":"                build_request.create()"},{"line_number":936,"context_line":"                # Create the mapping.  The null cell_id indicates that the"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_3ed71325","line":933,"updated":"2016-01-07 16:40:52.000000000","message":"Add display_description (please see my other comment)","commit_id":"015edf9b6e43f46ef6809e7b7fec3facad55f826"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5a3258729400a445954b40d4c6b4c039a2ed4b47","unresolved":false,"context_lines":[{"line_number":930,"context_line":"                        base_options[\u0027access_ip_v4\u0027],"},{"line_number":931,"context_line":"                        base_options[\u0027access_ip_v6\u0027], base_options[\u0027progress\u0027],"},{"line_number":932,"context_line":"                        base_options[\u0027vm_state\u0027], base_options[\u0027task_states\u0027],"},{"line_number":933,"context_line":"                        base_options[\u0027display_name\u0027], security_groups,"},{"line_number":934,"context_line":"                        block_device_mapping, shutdown_terminate)"},{"line_number":935,"context_line":"                build_request.create()"},{"line_number":936,"context_line":"                # Create the mapping.  The null cell_id indicates that the"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_0e96444c","line":933,"in_reply_to":"fa69d971_3ed71325","updated":"2016-01-08 16:03:08.000000000","message":"Will do.  I think there are a few more things I\u0027ll need to add as well.","commit_id":"015edf9b6e43f46ef6809e7b7fec3facad55f826"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"17c25dd78ff346dbb262c32a13632541a4245b39","unresolved":false,"context_lines":[{"line_number":900,"context_line":"            \u0027root_device_name\u0027: root_device_name,"},{"line_number":901,"context_line":"            \u0027progress\u0027: 0,"},{"line_number":902,"context_line":"            \u0027deleted\u0027: 0,"},{"line_number":903,"context_line":"            \u0027disable_terminate\u0027: False,"},{"line_number":904,"context_line":"            \u0027pci_requests\u0027: pci_request_info,"},{"line_number":905,"context_line":"            \u0027numa_topology\u0027: numa_topology,"},{"line_number":906,"context_line":"            \u0027system_metadata\u0027: system_metadata}"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_b7568727","line":903,"updated":"2016-05-17 17:38:10.000000000","message":"Assuming these are needed to honor instance show","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"65faa5f03362aafd0836e52ff75da8d9623865de","unresolved":false,"context_lines":[{"line_number":900,"context_line":"            \u0027root_device_name\u0027: root_device_name,"},{"line_number":901,"context_line":"            \u0027progress\u0027: 0,"},{"line_number":902,"context_line":"            \u0027deleted\u0027: 0,"},{"line_number":903,"context_line":"            \u0027disable_terminate\u0027: False,"},{"line_number":904,"context_line":"            \u0027pci_requests\u0027: pci_request_info,"},{"line_number":905,"context_line":"            \u0027numa_topology\u0027: numa_topology,"},{"line_number":906,"context_line":"            \u0027system_metadata\u0027: system_metadata}"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_cc30d5a0","line":903,"in_reply_to":"dab17558_b7568727","updated":"2016-05-17 19:15:48.000000000","message":"I honestly don\u0027t remember at this point. And removing these lines doesn\u0027t cause any test failures...\n\nI\u0027ll write a test that fails without these or leave them out.","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"cce91de1eb663836652a35e44f80dc8f82d242f8","unresolved":false,"context_lines":[{"line_number":900,"context_line":"            \u0027root_device_name\u0027: root_device_name,"},{"line_number":901,"context_line":"            \u0027progress\u0027: 0,"},{"line_number":902,"context_line":"            \u0027deleted\u0027: 0,"},{"line_number":903,"context_line":"            \u0027disable_terminate\u0027: False,"},{"line_number":904,"context_line":"            \u0027pci_requests\u0027: pci_request_info,"},{"line_number":905,"context_line":"            \u0027numa_topology\u0027: numa_topology,"},{"line_number":906,"context_line":"            \u0027system_metadata\u0027: system_metadata}"}],"source_content_type":"text/x-python","patch_set":29,"id":"bab6814e_d965eee1","line":903,"in_reply_to":"dab17558_cc30d5a0","updated":"2016-05-18 21:29:37.000000000","message":"I added a functional test which revealed that deleted is necessary but disable_terminate is not. And a few other fields are necessary. But I now set them on the instance in the BuildRequest object when it loads the instance from the db.","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"fc33d6abb0c9985c8bf4eccdb3f4ff1a61eb4aa9","unresolved":false,"context_lines":[{"line_number":1986,"context_line":""},{"line_number":1987,"context_line":"        self.compute_rpcapi.trigger_crash_dump(context, instance)"},{"line_number":1988,"context_line":""},{"line_number":1989,"context_line":"    def _get_instance_map_or_none(self, context, instance_uuid):"},{"line_number":1990,"context_line":"        try:"},{"line_number":1991,"context_line":"            inst_map \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":1992,"context_line":"                    context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":43,"id":"7aa08908_3a56a106","line":1989,"updated":"2016-06-16 10:27:27.000000000","message":"It can be InstanceMapping class method.","commit_id":"e7f9b4606414add0ac6c80f5b7bbafb560fe9292"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"93589270b68c049b9bf5706a563d12333eb69c5c","unresolved":false,"context_lines":[{"line_number":1986,"context_line":""},{"line_number":1987,"context_line":"        self.compute_rpcapi.trigger_crash_dump(context, instance)"},{"line_number":1988,"context_line":""},{"line_number":1989,"context_line":"    def _get_instance_map_or_none(self, context, instance_uuid):"},{"line_number":1990,"context_line":"        try:"},{"line_number":1991,"context_line":"            inst_map \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":1992,"context_line":"                    context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":43,"id":"5a9d85d2_194021a5","line":1989,"in_reply_to":"7aa08908_3a56a106","updated":"2016-06-20 20:01:16.000000000","message":"Yeah, I didn\u0027t consider that. This is a temporary method, as the comment indicates an InstanceMapping should always be found but that may not be true while deployers upgrade. At some point this should be removed. It would be more difficult to remove this from the InstanceMapping object due to the versioning requirements.","commit_id":"e7f9b4606414add0ac6c80f5b7bbafb560fe9292"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"711adf79b4b3aa03ff8b06d3e256527f75e9572d","unresolved":false,"context_lines":[{"line_number":2130,"context_line":"            return objects.Instance.get_by_uuid(context, instance_uuid,"},{"line_number":2131,"context_line":"                                                expected_attrs\u003dexpected_attrs)"},{"line_number":2132,"context_line":"        inst_map \u003d self._get_instance_map_or_none(context, instance_uuid)"},{"line_number":2133,"context_line":"        if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2134,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":2135,"context_line":"                instance \u003d objects.Instance.get_by_uuid("},{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_a977349e","line":2133,"updated":"2016-08-12 13:49:02.000000000","message":"If cell DB is unavailable (incorrect connection params, the server is gone away, whatever) what should we response to the user? I thought it\u0027s something like cell is unavailable, right?\nI\u0027m curious about on which level it will be done?","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d70b00dae0c516d9d711cb3ee65fb88f46eb98ad","unresolved":false,"context_lines":[{"line_number":2130,"context_line":"            return objects.Instance.get_by_uuid(context, instance_uuid,"},{"line_number":2131,"context_line":"                                                expected_attrs\u003dexpected_attrs)"},{"line_number":2132,"context_line":"        inst_map \u003d self._get_instance_map_or_none(context, instance_uuid)"},{"line_number":2133,"context_line":"        if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2134,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":2135,"context_line":"                instance \u003d objects.Instance.get_by_uuid("},{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_0749f78e","line":2133,"in_reply_to":"9ad45d7e_a977349e","updated":"2016-08-12 16:58:56.000000000","message":"It would return whatever the response is currently for that situation. The failure would be in sqlalchemy or oslo.db and would just get raised through all of this code.\n\nIf we were going to handle a database error I would probably put it in the wsgi handling code to just convert all db exceptions to some generic 500 error.","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"b783c1190fdf27a4af9ed5851e01605df74f6d68","unresolved":false,"context_lines":[{"line_number":2131,"context_line":"                                                expected_attrs\u003dexpected_attrs)"},{"line_number":2132,"context_line":"        inst_map \u003d self._get_instance_map_or_none(context, instance_uuid)"},{"line_number":2133,"context_line":"        if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2134,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":2135,"context_line":"                instance \u003d objects.Instance.get_by_uuid("},{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2137,"context_line":"        elif inst_map and (inst_map.cell_mapping is None):"},{"line_number":2138,"context_line":"            # This means the instance has not been scheduled and put in"},{"line_number":2139,"context_line":"            # a cell yet. For now it also may mean that the deployer"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_fdbbf4e2","line":2136,"range":{"start_line":2134,"start_character":0,"end_line":2136,"end_character":74},"updated":"2016-08-11 13:46:47.000000000","message":"Am i right if this can be extracted to separate function as code mentioned below by Melanie Wit?","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"b12df8ff3818e889caf1beaf22d50d81ee2bd86e","unresolved":false,"context_lines":[{"line_number":2131,"context_line":"                                                expected_attrs\u003dexpected_attrs)"},{"line_number":2132,"context_line":"        inst_map \u003d self._get_instance_map_or_none(context, instance_uuid)"},{"line_number":2133,"context_line":"        if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2134,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":2135,"context_line":"                instance \u003d objects.Instance.get_by_uuid("},{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2137,"context_line":"        elif inst_map and (inst_map.cell_mapping is None):"},{"line_number":2138,"context_line":"            # This means the instance has not been scheduled and put in"},{"line_number":2139,"context_line":"            # a cell yet. For now it also may mean that the deployer"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_44316d59","line":2136,"range":{"start_line":2134,"start_character":0,"end_line":2136,"end_character":74},"in_reply_to":"9ad45d7e_fdbbf4e2","updated":"2016-08-11 17:54:01.000000000","message":"Yep :)","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"711adf79b4b3aa03ff8b06d3e256527f75e9572d","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_d97cf1ed","line":2148,"updated":"2016-08-12 13:49:02.000000000","message":"I think the case you are solving here is when the instance is in the creating state. If we really need to return the instance, maybe, we should wait for some time or make a cycle of getting instance mapping.\nOr, maybe, simply return that instance doesn\u0027t exist?","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"d70b00dae0c516d9d711cb3ee65fb88f46eb98ad","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_c3aeb2e6","line":2148,"in_reply_to":"9ad45d7e_d97cf1ed","updated":"2016-08-12 16:58:56.000000000","message":"We can\u0027t return that the instance doesn\u0027t exist unless it really doesn\u0027t exist.\n\nYes, this is for the creation case. But I don\u0027t think we need to wait before querying for the instance mapping. The order of operations in the conductor will be that the instance mapping will point to a cell before the BuildRequest is deleted. So if we\u0027re in this block due to the BuildRequestNotFound then we know that the conductor has set the instance mapping to a cell.\n\nThe None case is that currently the conductor deletes instance mappings when an instance is put in a cell, but that is going to change soon. So eventually this will just be a _get_instance_map call.","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0fcb8556b5031a18a761e4d1fa2d339e76f9facc","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"},{"line_number":2152,"context_line":"                                                  inst_map.cell_mapping):"},{"line_number":2153,"context_line":"                        instance \u003d objects.Instance.get_by_uuid("},{"line_number":2154,"context_line":"                            context, instance_uuid,"},{"line_number":2155,"context_line":"                            expected_attrs\u003dexpected_attrs)"},{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_f5803fc1","line":2155,"range":{"start_line":2148,"start_character":16,"end_line":2155,"end_character":58},"updated":"2016-08-11 07:39:08.000000000","message":"I can\u0027t help but think we could use a helper function for this.","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2b5a462c5832cad7c6159121c2b4c659bda17aba","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"},{"line_number":2152,"context_line":"                                                  inst_map.cell_mapping):"},{"line_number":2153,"context_line":"                        instance \u003d objects.Instance.get_by_uuid("},{"line_number":2154,"context_line":"                            context, instance_uuid,"},{"line_number":2155,"context_line":"                            expected_attrs\u003dexpected_attrs)"},{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"}],"source_content_type":"text/x-python","patch_set":62,"id":"3ac371cc_9864cce5","line":2155,"range":{"start_line":2148,"start_character":16,"end_line":2155,"end_character":58},"in_reply_to":"3ac371cc_5d16c655","updated":"2016-08-16 21:25:40.000000000","message":"A-ha, okay.","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"0dfa63b2bdbb530ce18986ffaffbb459c6014720","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"},{"line_number":2152,"context_line":"                                                  inst_map.cell_mapping):"},{"line_number":2153,"context_line":"                        instance \u003d objects.Instance.get_by_uuid("},{"line_number":2154,"context_line":"                            context, instance_uuid,"},{"line_number":2155,"context_line":"                            expected_attrs\u003dexpected_attrs)"},{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"}],"source_content_type":"text/x-python","patch_set":62,"id":"3ac371cc_5d16c655","line":2155,"range":{"start_line":2148,"start_character":16,"end_line":2155,"end_character":58},"in_reply_to":"9ad45d7e_6b773402","updated":"2016-08-16 21:22:54.000000000","message":"After attempting this briefly I realize it\u0027s trickier than it looks because there are else clauses that rely on the result of inst_map.\n\nSo I could return a tuple of (inst_map, instance) from a helper method which would only be useful here. Otherwise this looks a lot like _lookup_instance which doesn\u0027t work here because I need the instance_mapping as well.","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5d04ceca91272f84fe7949dbc4cfd9691133b52a","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"},{"line_number":2152,"context_line":"                                                  inst_map.cell_mapping):"},{"line_number":2153,"context_line":"                        instance \u003d objects.Instance.get_by_uuid("},{"line_number":2154,"context_line":"                            context, instance_uuid,"},{"line_number":2155,"context_line":"                            expected_attrs\u003dexpected_attrs)"},{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_6b773402","line":2155,"range":{"start_line":2148,"start_character":16,"end_line":2155,"end_character":58},"in_reply_to":"9ad45d7e_f5803fc1","updated":"2016-08-11 12:48:36.000000000","message":"Yeah, I\u0027ve done this in a few places now.","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":2118,"context_line":"            inst_map \u003d None"},{"line_number":2119,"context_line":"        return inst_map"},{"line_number":2120,"context_line":""},{"line_number":2121,"context_line":"    def _get_instance(self, context, instance_uuid, expected_attrs):"},{"line_number":2122,"context_line":"        # Before service version 15 the BuildRequest is not cleaned up during"},{"line_number":2123,"context_line":"        # a delete request so there is no reason to look it up here as we can\u0027t"},{"line_number":2124,"context_line":"        # trust that it\u0027s not referencing a deleted instance. Also even if"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_f3be17fe","line":2121,"updated":"2016-08-19 21:28:40.000000000","message":"This might have already been brought up, but I wonder how much of this is duplicated with the _lookup_instance method for the local-delete scenario.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":2118,"context_line":"            inst_map \u003d None"},{"line_number":2119,"context_line":"        return inst_map"},{"line_number":2120,"context_line":""},{"line_number":2121,"context_line":"    def _get_instance(self, context, instance_uuid, expected_attrs):"},{"line_number":2122,"context_line":"        # Before service version 15 the BuildRequest is not cleaned up during"},{"line_number":2123,"context_line":"        # a delete request so there is no reason to look it up here as we can\u0027t"},{"line_number":2124,"context_line":"        # trust that it\u0027s not referencing a deleted instance. Also even if"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_c62cfbad","line":2121,"in_reply_to":"1ac06dbe_f3be17fe","updated":"2016-08-19 22:01:39.000000000","message":"There are different assumptions between the two methods. _lookup_instance does not care about checking for an instance in a BuildRequest, it assumes that\u0027s not relevant. This method is just get me an instance wherever you can find it at all costs and makes no assumptions about whether or not there\u0027s a BuildRequest involved. So there\u0027s some overlap but the purposes are different.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":2124,"context_line":"        # trust that it\u0027s not referencing a deleted instance. Also even if"},{"line_number":2125,"context_line":"        # there is an instance mapping we don\u0027t need to honor it for older"},{"line_number":2126,"context_line":"        # service versions."},{"line_number":2127,"context_line":"        service_version \u003d objects.Service.get_minimum_version("},{"line_number":2128,"context_line":"            context, \u0027nova-api\u0027)"},{"line_number":2129,"context_line":"        if service_version \u003c 15:"},{"line_number":2130,"context_line":"            return objects.Instance.get_by_uuid(context, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_73a50703","line":2127,"updated":"2016-08-19 21:28:40.000000000","message":"This is already cached so we\u0027re good there:\n\nhttps://github.com/openstack/nova/blob/master/nova/cmd/api.py#L46\n\nWe might want to remove the TODO in there though because that was written for the REST API stuff for 2.37 but would still be needed for this I\u0027d think.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb2d14827534324cfd2bdb12b2dc7cf9969627a1","unresolved":false,"context_lines":[{"line_number":2124,"context_line":"        # trust that it\u0027s not referencing a deleted instance. Also even if"},{"line_number":2125,"context_line":"        # there is an instance mapping we don\u0027t need to honor it for older"},{"line_number":2126,"context_line":"        # service versions."},{"line_number":2127,"context_line":"        service_version \u003d objects.Service.get_minimum_version("},{"line_number":2128,"context_line":"            context, \u0027nova-api\u0027)"},{"line_number":2129,"context_line":"        if service_version \u003c 15:"},{"line_number":2130,"context_line":"            return objects.Instance.get_by_uuid(context, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_f56277d7","line":2127,"in_reply_to":"1ac06dbe_294de072","updated":"2016-08-19 23:30:18.000000000","message":"The caching is only necessary for the performance benefit of not needing to do that service query from the DB every time we get an instance, which doubles the traffic to the DB on every instance query, which kind of sucks.\n\nAgree with leaving the caching in, that was my point with removing the TODO so we don\u0027t forget about this.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":2124,"context_line":"        # trust that it\u0027s not referencing a deleted instance. Also even if"},{"line_number":2125,"context_line":"        # there is an instance mapping we don\u0027t need to honor it for older"},{"line_number":2126,"context_line":"        # service versions."},{"line_number":2127,"context_line":"        service_version \u003d objects.Service.get_minimum_version("},{"line_number":2128,"context_line":"            context, \u0027nova-api\u0027)"},{"line_number":2129,"context_line":"        if service_version \u003c 15:"},{"line_number":2130,"context_line":"            return objects.Instance.get_by_uuid(context, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_294de072","line":2127,"in_reply_to":"1ac06dbe_73a50703","updated":"2016-08-19 22:01:39.000000000","message":"Is that caching necessary? It would speed up this lookup but won\u0027t change the result right?\n\nSeems like it would be good to just leave the caching in at this point as we\u0027re likely to keep making decisions based on service version for a while.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"81a20e0758cb6c001be78a532d3ff7e29e60429f","unresolved":false,"context_lines":[{"line_number":2124,"context_line":"        # trust that it\u0027s not referencing a deleted instance. Also even if"},{"line_number":2125,"context_line":"        # there is an instance mapping we don\u0027t need to honor it for older"},{"line_number":2126,"context_line":"        # service versions."},{"line_number":2127,"context_line":"        service_version \u003d objects.Service.get_minimum_version("},{"line_number":2128,"context_line":"            context, \u0027nova-api\u0027)"},{"line_number":2129,"context_line":"        if service_version \u003c 15:"},{"line_number":2130,"context_line":"            return objects.Instance.get_by_uuid(context, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_ddabe2d4","line":2127,"in_reply_to":"1ac06dbe_f56277d7","updated":"2016-08-22 14:10:21.000000000","message":"https://review.openstack.org/#/c/358660","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":2134,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":2135,"context_line":"                instance \u003d objects.Instance.get_by_uuid("},{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2137,"context_line":"        elif inst_map and (inst_map.cell_mapping is None):"},{"line_number":2138,"context_line":"            # This means the instance has not been scheduled and put in"},{"line_number":2139,"context_line":"            # a cell yet. For now it also may mean that the deployer"},{"line_number":2140,"context_line":"            # has not created their cell(s) yet."}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_532bab30","line":2137,"updated":"2016-08-19 21:28:40.000000000","message":"style nit: I would have probably done something different like this:\n\nif inst_map:\n  if inst_map.cell_mapping is not None:\n    ...\n  else:\n    ...\nelse:\n  ...","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2137,"context_line":"        elif inst_map and (inst_map.cell_mapping is None):"},{"line_number":2138,"context_line":"            # This means the instance has not been scheduled and put in"},{"line_number":2139,"context_line":"            # a cell yet. For now it also may mean that the deployer"},{"line_number":2140,"context_line":"            # has not created their cell(s) yet."},{"line_number":2141,"context_line":"            try:"},{"line_number":2142,"context_line":"                build_req \u003d objects.BuildRequest.get_by_instance_uuid("},{"line_number":2143,"context_line":"                        context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_93e4030e","line":2140,"range":{"start_line":2139,"start_character":26,"end_line":2140,"end_character":48},"updated":"2016-08-19 21:28:40.000000000","message":"Do we need/want a TODO to make this a hard fail after we\u0027ve started requiring cells v2 and the data migration to be done?","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2137,"context_line":"        elif inst_map and (inst_map.cell_mapping is None):"},{"line_number":2138,"context_line":"            # This means the instance has not been scheduled and put in"},{"line_number":2139,"context_line":"            # a cell yet. For now it also may mean that the deployer"},{"line_number":2140,"context_line":"            # has not created their cell(s) yet."},{"line_number":2141,"context_line":"            try:"},{"line_number":2142,"context_line":"                build_req \u003d objects.BuildRequest.get_by_instance_uuid("},{"line_number":2143,"context_line":"                        context, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_299b20cb","line":2140,"range":{"start_line":2139,"start_character":26,"end_line":2140,"end_character":48},"in_reply_to":"1ac06dbe_93e4030e","updated":"2016-08-19 22:01:39.000000000","message":"This one won\u0027t be a hard fail later because it also just indicates an unscheduled instance.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"},{"line_number":2152,"context_line":"                                                  inst_map.cell_mapping):"},{"line_number":2153,"context_line":"                        instance \u003d objects.Instance.get_by_uuid("},{"line_number":2154,"context_line":"                            context, instance_uuid,"},{"line_number":2155,"context_line":"                            expected_attrs\u003dexpected_attrs)"},{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_b395bf41","line":2155,"range":{"start_line":2148,"start_character":16,"end_line":2155,"end_character":58},"updated":"2016-08-19 21:28:40.000000000","message":"Seems we could throw this into a helper method since it\u0027s duplicated with the above, but I think that was brought up earlier. Might be better to be explicit in here so we don\u0027t get lost.\n\n--\n\nAlso, do we have a unit test for this?","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"},{"line_number":2147,"context_line":"                # while fetching. Try again."},{"line_number":2148,"context_line":"                inst_map \u003d self._get_instance_map_or_none(context,"},{"line_number":2149,"context_line":"                                                          instance_uuid)"},{"line_number":2150,"context_line":"                if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2151,"context_line":"                    with nova_context.target_cell(context,"},{"line_number":2152,"context_line":"                                                  inst_map.cell_mapping):"},{"line_number":2153,"context_line":"                        instance \u003d objects.Instance.get_by_uuid("},{"line_number":2154,"context_line":"                            context, instance_uuid,"},{"line_number":2155,"context_line":"                            expected_attrs\u003dexpected_attrs)"},{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_69f3580b","line":2155,"range":{"start_line":2148,"start_character":16,"end_line":2155,"end_character":58},"in_reply_to":"1ac06dbe_b395bf41","updated":"2016-08-19 22:01:39.000000000","message":"Yeah, this was brought up earlier. I didn\u0027t throw it into a helper because we need both the instance and the inst_map back from this block and then the \"else\" statement for the \"if\" is different in each case. I think it would be an awkward helper method and complicate the if/else logic here.\n\nAnd you\u0027re right, there is no unit test for this case.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"},{"line_number":2159,"context_line":"                    # happened yet. This will be a failure case later."},{"line_number":2160,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2161,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2162,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_138db331","line":2159,"range":{"start_line":2159,"start_character":36,"end_line":2159,"end_character":70},"updated":"2016-08-19 21:28:40.000000000","message":"Maybe add a TODO so we don\u0027t forget.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":2156,"context_line":"                else:"},{"line_number":2157,"context_line":"                    # If BuildRequest is not found but inst_map.cell_mapping"},{"line_number":2158,"context_line":"                    # does not point at a cell then cell migration has not"},{"line_number":2159,"context_line":"                    # happened yet. This will be a failure case later."},{"line_number":2160,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2161,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2162,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_a94b902a","line":2159,"range":{"start_line":2159,"start_character":36,"end_line":2159,"end_character":70},"in_reply_to":"1ac06dbe_138db331","updated":"2016-08-19 22:01:39.000000000","message":"Yeah, can add if I respin.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":2165,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance. This \u0027"},{"line_number":2166,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2167,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2168,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027))"},{"line_number":2169,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2170,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2171,"context_line":""}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_9379a332","line":2168,"updated":"2016-08-19 21:28:40.000000000","message":"If you\u0027re going to request that someone open a bug, do you want to also log the traceback?","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb2d14827534324cfd2bdb12b2dc7cf9969627a1","unresolved":false,"context_lines":[{"line_number":2165,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance. This \u0027"},{"line_number":2166,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2167,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2168,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027))"},{"line_number":2169,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2170,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2171,"context_line":""}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_b5963f92","line":2168,"in_reply_to":"1ac06dbe_2937a0b1","updated":"2016-08-19 23:30:18.000000000","message":"OK, well at the very least I just noticed that we\u0027re not logging the instance uuid in this message, so we should have the instance_uuid\u003dinstance_uuid kwarg on this log message for context, otherwise we have no idea which instance doesn\u0027t have the mapping, which makes it a useless warning. :)","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":2165,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance. This \u0027"},{"line_number":2166,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2167,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2168,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027))"},{"line_number":2169,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2170,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2171,"context_line":""}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_2937a0b1","line":2168,"in_reply_to":"1ac06dbe_9379a332","updated":"2016-08-19 22:01:39.000000000","message":"I think the relevant information is what\u0027s in the database rather than the code path that got them here. So I\u0027m not sure the traceback will be useful but some information about the state of their database will be. I could add some text about that but I\u0027m not sure how to concisely describe what would be useful.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"81a20e0758cb6c001be78a532d3ff7e29e60429f","unresolved":false,"context_lines":[{"line_number":2165,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance. This \u0027"},{"line_number":2166,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2167,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2168,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027))"},{"line_number":2169,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2170,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2171,"context_line":""}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_bd7d4670","line":2168,"in_reply_to":"1ac06dbe_b5963f92","updated":"2016-08-22 14:10:21.000000000","message":"Good point.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e9bdbb4c8a694aef1fa2bca2de2618fe076b5583","unresolved":false,"context_lines":[{"line_number":2167,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance %s. This \u0027"},{"line_number":2168,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2169,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2170,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027) % instance_uuid)"},{"line_number":2171,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2172,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2173,"context_line":""}],"source_content_type":"text/x-python","patch_set":66,"id":"1ac06dbe_67016a6a","line":2170,"range":{"start_line":2170,"start_character":63,"end_line":2170,"end_character":64},"updated":"2016-08-22 19:35:57.000000000","message":"Commas avoid string interpolation unless the message is actually logged","commit_id":"af25863b04703c7bc564d9f36f61c6bed08ee3ac"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4529e4a5a7658a056aa4e07dbe17c68452482e07","unresolved":false,"context_lines":[{"line_number":2167,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance %s. This \u0027"},{"line_number":2168,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2169,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2170,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027) % instance_uuid)"},{"line_number":2171,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2172,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2173,"context_line":""}],"source_content_type":"text/x-python","patch_set":66,"id":"1ac06dbe_883913f5","line":2170,"range":{"start_line":2170,"start_character":63,"end_line":2170,"end_character":64},"in_reply_to":"1ac06dbe_67016a6a","updated":"2016-08-22 20:29:31.000000000","message":"Gotcha.","commit_id":"af25863b04703c7bc564d9f36f61c6bed08ee3ac"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8b0b150430f45653325083682c23872a5d30cb6d","unresolved":false,"context_lines":[{"line_number":2131,"context_line":"                                                expected_attrs\u003dexpected_attrs)"},{"line_number":2132,"context_line":"        inst_map \u003d self._get_instance_map_or_none(context, instance_uuid)"},{"line_number":2133,"context_line":"        if inst_map and (inst_map.cell_mapping is not None):"},{"line_number":2134,"context_line":"            with nova_context.target_cell(context, inst_map.cell_mapping):"},{"line_number":2135,"context_line":"                instance \u003d objects.Instance.get_by_uuid("},{"line_number":2136,"context_line":"                    context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2137,"context_line":"        elif inst_map and (inst_map.cell_mapping is None):"}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_42084ada","line":2134,"updated":"2016-08-23 20:10:39.000000000","message":"nit: a comment saying that we found a cell and tho reading it to get the instance could be useful, in particular since that\u0027s probably the first time we\u0027re using the contextmanager for switching MQs.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1ea3687453ce1130e043f1970f338d5baffd0d56","unresolved":false,"context_lines":[{"line_number":2140,"context_line":"            # has not created their cell(s) yet."},{"line_number":2141,"context_line":"            try:"},{"line_number":2142,"context_line":"                build_req \u003d objects.BuildRequest.get_by_instance_uuid("},{"line_number":2143,"context_line":"                        context, instance_uuid)"},{"line_number":2144,"context_line":"                instance \u003d build_req.instance"},{"line_number":2145,"context_line":"            except exception.BuildRequestNotFound:"},{"line_number":2146,"context_line":"                # Instance was mapped and the BuildRequest was deleted"}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_ed511dda","line":2143,"updated":"2016-08-24 20:29:37.000000000","message":"I\u0027m leaving the comment here for the sake of the discussion we had over IRC, but I want to make sure that the possibility to have a wrong BuildRequest due to an updated instance is not possible. For the moment, that patch is not having problem with that, but before we merge the instance creation move to the conductor, we need to make sure that the instance update is not possible unless the instance is scheduled (and then no BuildRequest is there)","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8b0b150430f45653325083682c23872a5d30cb6d","unresolved":false,"context_lines":[{"line_number":2160,"context_line":"                    # TODO(alaski): Make this a failure case after we put in"},{"line_number":2161,"context_line":"                    # a block that requires migrating to cellsv2."},{"line_number":2162,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2163,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2164,"context_line":"        else:"},{"line_number":2165,"context_line":"            # This should not happen if we made it past the service_version"},{"line_number":2166,"context_line":"            # check above. But it does not need to be an exception yet."}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_c2defadb","line":2163,"updated":"2016-08-23 20:10:39.000000000","message":"Could we maybe put that defensive code within the _get_instance_map_or_none() so that it would be consistent with the else statement below (which would become even not needed if we do the check within the call)","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2c70c4bc153da9af04220c375a70587506f7c437","unresolved":false,"context_lines":[{"line_number":2160,"context_line":"                    # TODO(alaski): Make this a failure case after we put in"},{"line_number":2161,"context_line":"                    # a block that requires migrating to cellsv2."},{"line_number":2162,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2163,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2164,"context_line":"        else:"},{"line_number":2165,"context_line":"            # This should not happen if we made it past the service_version"},{"line_number":2166,"context_line":"            # check above. But it does not need to be an exception yet."}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_259fe551","line":2163,"in_reply_to":"1ac06dbe_6c857858","updated":"2016-08-24 14:25:55.000000000","message":"Okay, like I said over IRC, it was more likely a prescription for avoiding future potential mistakes (in particular due to the duplication of the code statement whether the mapping is present) but I don\u0027t want to overcomplicate things.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"98c602e64273c3475fc1825ffb88474d3a80e20a","unresolved":false,"context_lines":[{"line_number":2160,"context_line":"                    # TODO(alaski): Make this a failure case after we put in"},{"line_number":2161,"context_line":"                    # a block that requires migrating to cellsv2."},{"line_number":2162,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2163,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2164,"context_line":"        else:"},{"line_number":2165,"context_line":"            # This should not happen if we made it past the service_version"},{"line_number":2166,"context_line":"            # check above. But it does not need to be an exception yet."}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_6c857858","line":2163,"in_reply_to":"1ac06dbe_c2defadb","updated":"2016-08-23 20:55:40.000000000","message":"_get_instance_map_or_none returns an instance mapping, not an instance. So I\u0027m not sure I see this being put in that method. And if I change it to return an instance by embedding this clause then I\u0027m really just moving much of this _get_instance method into that method. I can do that if the length of this method is problematic, which it might be since there\u0027s a strong desire by reviewers to consolidate things. But this is as straightforward as I could make it for now. At the moment I favor simplicity over conciseness. Though there are a lot of branches here it\u0027s easy enough to follow a single path through if you know the conditions.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8b0b150430f45653325083682c23872a5d30cb6d","unresolved":false,"context_lines":[{"line_number":2161,"context_line":"                    # a block that requires migrating to cellsv2."},{"line_number":2162,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2163,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2164,"context_line":"        else:"},{"line_number":2165,"context_line":"            # This should not happen if we made it past the service_version"},{"line_number":2166,"context_line":"            # check above. But it does not need to be an exception yet."},{"line_number":2167,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance %s. This \u0027"},{"line_number":2168,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2169,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2170,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027), instance_uuid)"},{"line_number":2171,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2172,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2173,"context_line":""},{"line_number":2174,"context_line":"        return instance"},{"line_number":2175,"context_line":""}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_82c9a2ae","line":2172,"range":{"start_line":2164,"start_character":0,"end_line":2172,"end_character":70},"updated":"2016-08-23 20:10:39.000000000","message":"From a practical point of view, I think we should be failing earlier by just verifying whether the instance mapping is there or not and then leave that warning so that later, we could cut that defensive approach without needing to touch the really needed conditional around the nullable cell mapping.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2c70c4bc153da9af04220c375a70587506f7c437","unresolved":false,"context_lines":[{"line_number":2161,"context_line":"                    # a block that requires migrating to cellsv2."},{"line_number":2162,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2163,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2164,"context_line":"        else:"},{"line_number":2165,"context_line":"            # This should not happen if we made it past the service_version"},{"line_number":2166,"context_line":"            # check above. But it does not need to be an exception yet."},{"line_number":2167,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance %s. This \u0027"},{"line_number":2168,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2169,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2170,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027), instance_uuid)"},{"line_number":2171,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2172,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2173,"context_line":""},{"line_number":2174,"context_line":"        return instance"},{"line_number":2175,"context_line":""}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_e59cad5f","line":2172,"range":{"start_line":2164,"start_character":0,"end_line":2172,"end_character":70},"in_reply_to":"1ac06dbe_4c2cfc98","updated":"2016-08-24 14:25:55.000000000","message":"Fair enough.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"98c602e64273c3475fc1825ffb88474d3a80e20a","unresolved":false,"context_lines":[{"line_number":2161,"context_line":"                    # a block that requires migrating to cellsv2."},{"line_number":2162,"context_line":"                    instance \u003d objects.Instance.get_by_uuid("},{"line_number":2163,"context_line":"                        context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2164,"context_line":"        else:"},{"line_number":2165,"context_line":"            # This should not happen if we made it past the service_version"},{"line_number":2166,"context_line":"            # check above. But it does not need to be an exception yet."},{"line_number":2167,"context_line":"            LOG.warning(_LW(\u0027No instance_mapping found for instance %s. This \u0027"},{"line_number":2168,"context_line":"                            \u0027should not be happening and will lead to errors \u0027"},{"line_number":2169,"context_line":"                            \u0027in the future. Please open a bug at \u0027"},{"line_number":2170,"context_line":"                            \u0027https://bugs.launchpad.net/nova\u0027), instance_uuid)"},{"line_number":2171,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2172,"context_line":"                context, instance_uuid, expected_attrs\u003dexpected_attrs)"},{"line_number":2173,"context_line":""},{"line_number":2174,"context_line":"        return instance"},{"line_number":2175,"context_line":""}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_4c2cfc98","line":2172,"range":{"start_line":2164,"start_character":0,"end_line":2172,"end_character":70},"in_reply_to":"1ac06dbe_82c9a2ae","updated":"2016-08-23 20:55:40.000000000","message":"I\u0027m wary of that approach because I want to be defensive against mistakes I\u0027ve made. While I don\u0027t expect this code to ever be run if it is it\u0027s likely my fault and users/deployers shouldn\u0027t suffer for it.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"}],"nova/objects/build_request.py":[{"author":{"_account_id":17139,"name":"Chuck Carmack","email":"carmack@us.ibm.com","username":"ccarmack"},"change_message_id":"94f464c086417438241abb494c88e1531b34f1fe","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        instance \u003d objects.Instance(self._context)"},{"line_number":162,"context_line":"        build_req_fields \u003d [\u0027user_id\u0027, \u0027metadata\u0027, \u0027access_ip_v4\u0027,"},{"line_number":163,"context_line":"                \u0027access_ip_v6\u0027, \u0027progress\u0027, \u0027vm_state\u0027, \u0027task_state\u0027,"},{"line_number":164,"context_line":"                \u0027display_name\u0027, \u0027security_groups\u0027, \u0027security_groups\u0027,"},{"line_number":165,"context_line":"                \u0027shutdown_terminate\u0027]"},{"line_number":166,"context_line":"        for build_req_field in build_req_fields:"},{"line_number":167,"context_line":"            setattr(instance, build_req_field, getattr(self, build_req_field))"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_bbfeb536","line":164,"updated":"2016-01-07 16:30:29.000000000","message":"I think the display_description needs to be added. I\u0027m working on a spec for M that shows/lists the description (https://review.openstack.org/#/c/228908/).\n\nEven if the spec is not done, this would match the behavour when it was loaded from the instance db.","commit_id":"015edf9b6e43f46ef6809e7b7fec3facad55f826"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"58a62b6f250e0fc827258dd246364058c06130f6","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        instance.deleted \u003d 0"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        instance.metadata \u003d self.instance_metadata"},{"line_number":173,"context_line":"        build_req_fields \u003d [\u0027project_id\u0027, \u0027user_id\u0027, \u0027image_ref\u0027,"},{"line_number":174,"context_line":"                \u0027access_ip_v4\u0027, \u0027access_ip_v6\u0027, \u0027progress\u0027, \u0027vm_state\u0027,"},{"line_number":175,"context_line":"                \u0027task_state\u0027, \u0027display_name\u0027, \u0027security_groups\u0027, \u0027created_at\u0027,"},{"line_number":176,"context_line":"                \u0027updated_at\u0027, \u0027info_cache\u0027, \u0027config_drive\u0027, \u0027key_name\u0027,"},{"line_number":177,"context_line":"                \u0027locked_by\u0027]"},{"line_number":178,"context_line":"        for build_req_field in build_req_fields:"},{"line_number":179,"context_line":"            setattr(instance, build_req_field, getattr(self, build_req_field))"},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_a4f7fff3","line":177,"range":{"start_line":173,"start_character":27,"end_line":177,"end_character":28},"updated":"2016-03-02 16:23:32.000000000","message":"Seems odd to hard-code these when you can get them from self.fields.keys() and then just filter out things you don\u0027t want, but then you have to hard-code the exclusion list, so maybe it\u0027s a net zero either way.\n\nOr....are these fields that are used in the API view builder for the server details? If that\u0027s the case, we might want a comment to explain that. I\u0027m thinking about what happens when we have a new microversion in the API that exposes new fields in the details response.","commit_id":"a792a1dd35f12951bf33f09ea960984f8462102d"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fc2a27f6d6c5724d8b99a276a380998ff9e329c5","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        instance.deleted \u003d 0"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        instance.metadata \u003d self.instance_metadata"},{"line_number":173,"context_line":"        build_req_fields \u003d [\u0027project_id\u0027, \u0027user_id\u0027, \u0027image_ref\u0027,"},{"line_number":174,"context_line":"                \u0027access_ip_v4\u0027, \u0027access_ip_v6\u0027, \u0027progress\u0027, \u0027vm_state\u0027,"},{"line_number":175,"context_line":"                \u0027task_state\u0027, \u0027display_name\u0027, \u0027security_groups\u0027, \u0027created_at\u0027,"},{"line_number":176,"context_line":"                \u0027updated_at\u0027, \u0027info_cache\u0027, \u0027config_drive\u0027, \u0027key_name\u0027,"},{"line_number":177,"context_line":"                \u0027locked_by\u0027]"},{"line_number":178,"context_line":"        for build_req_field in build_req_fields:"},{"line_number":179,"context_line":"            setattr(instance, build_req_field, getattr(self, build_req_field))"},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_1d851a06","line":177,"range":{"start_line":173,"start_character":27,"end_line":177,"end_character":28},"in_reply_to":"7af24918_a4f7fff3","updated":"2016-03-03 16:10:02.000000000","message":"This is a combination of fields from the API view builder for server details, plus fields added in extensions, minus the fields below that come from request_spec. Hardcoding is going to be the easiest way for now.","commit_id":"a792a1dd35f12951bf33f09ea960984f8462102d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9725707b1b92bf6fe47565757419bde24c4ec315","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        # not lazy-loadable, and don\u0027t change."},{"line_number":74,"context_line":"        self.instance.deleted \u003d 0"},{"line_number":75,"context_line":"        self.instance.terminated_at \u003d None"},{"line_number":76,"context_line":"        self.instance.host \u003d None"},{"line_number":77,"context_line":"        self.instance.launched_at \u003d None"},{"line_number":78,"context_line":"        # created_at/updated_at are not on the serialized instance because it"},{"line_number":79,"context_line":"        # was never persisted."}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_6e2f48a1","line":76,"updated":"2016-05-22 22:37:40.000000000","message":"You also should set self.instance.launched_on \u003d None","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4c31f5ae6d2d4a5d8a83eb27374f3d54b334d8c7","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        # not lazy-loadable, and don\u0027t change."},{"line_number":74,"context_line":"        self.instance.deleted \u003d 0"},{"line_number":75,"context_line":"        self.instance.terminated_at \u003d None"},{"line_number":76,"context_line":"        self.instance.host \u003d None"},{"line_number":77,"context_line":"        self.instance.launched_at \u003d None"},{"line_number":78,"context_line":"        # created_at/updated_at are not on the serialized instance because it"},{"line_number":79,"context_line":"        # was never persisted."}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_1b8500ce","line":76,"in_reply_to":"bab6814e_6e2f48a1","updated":"2016-05-23 12:47:21.000000000","message":"Can do.","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"},{"author":{"_account_id":14762,"name":"Zhihai Song","email":"zhhsng@gmail.com","username":"ZhihaiSong"},"change_message_id":"7bab05bb9b391eb7318ed0ba8952b11e27b1b7f3","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                continue"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                setattr(req, key, db_req[key])"},{"line_number":131,"context_line":"            setattr(req, key, db_req[key])"},{"line_number":132,"context_line":"        # Load instance last because other fields on req may be referenced"},{"line_number":133,"context_line":"        req._load_instance(db_req[\u0027instance\u0027])"},{"line_number":134,"context_line":"        req.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":52,"id":"dada55a8_2bb12ae2","line":131,"range":{"start_line":131,"start_character":12,"end_line":131,"end_character":42},"updated":"2016-07-19 12:59:59.000000000","message":"The if...elif...else covers all logical paths. So, this line can be removed.","commit_id":"de4bc1259e9fb6f7384e59e75571f0af1cfcfddb"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"5523c8a61a3321f49a54428fc7fb44ede22fc365","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                continue"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                setattr(req, key, db_req[key])"},{"line_number":131,"context_line":"            setattr(req, key, db_req[key])"},{"line_number":132,"context_line":"        # Load instance last because other fields on req may be referenced"},{"line_number":133,"context_line":"        req._load_instance(db_req[\u0027instance\u0027])"},{"line_number":134,"context_line":"        req.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":52,"id":"dada55a8_eb7bfbf2","line":131,"range":{"start_line":131,"start_character":12,"end_line":131,"end_character":42},"in_reply_to":"dada55a8_2bb12ae2","updated":"2016-07-21 20:56:32.000000000","message":"Good point.","commit_id":"de4bc1259e9fb6f7384e59e75571f0af1cfcfddb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8ba2b1c0526dd6002a07774baf8f7e830a40eaf6","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                continue"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                setattr(req, key, db_req[key])"},{"line_number":131,"context_line":"            setattr(req, key, db_req[key])"},{"line_number":132,"context_line":"        # Load instance last because other fields on req may be referenced"},{"line_number":133,"context_line":"        req._load_instance(db_req[\u0027instance\u0027])"},{"line_number":134,"context_line":"        req.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":61,"id":"9ad45d7e_db5dc17a","line":131,"updated":"2016-08-09 15:48:58.000000000","message":"Hmm, say what now? We\u0027ll end up doing the setattr twice in a row here, right? Also, I\u0027m not sure why the continue statement needs to be added as a result of the change you\u0027re trying to make.","commit_id":"ec2628e5798b7e5e322c5cb3df111cc2be9713ba"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"860d045bdaf97eda32bb0c7bd24d68a3bd1f182d","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                continue"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                setattr(req, key, db_req[key])"},{"line_number":131,"context_line":"            setattr(req, key, db_req[key])"},{"line_number":132,"context_line":"        # Load instance last because other fields on req may be referenced"},{"line_number":133,"context_line":"        req._load_instance(db_req[\u0027instance\u0027])"},{"line_number":134,"context_line":"        req.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":61,"id":"9ad45d7e_5e141a43","line":131,"in_reply_to":"9ad45d7e_db5dc17a","updated":"2016-08-09 17:11:59.000000000","message":"Yeah, I\u0027m not sure how I ended up with this.\n\nWhat I think I meant to do was have the continue and not the else clause. But perhaps it\u0027s better to leave the else and remove the continue.","commit_id":"ec2628e5798b7e5e322c5cb3df111cc2be9713ba"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"73b2ab1cb2567bbde49a1f503f78f5adf19462d8","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                              \u0027BuildRequest\u0027))"},{"line_number":80,"context_line":"            raise exception.BuildRequestNotFound(uuid\u003dself.instance_uuid)"},{"line_number":81,"context_line":"        # NOTE(alaski): Set some fields on instance that are needed by the api,"},{"line_number":82,"context_line":"        # not lazy-loadable, and don\u0027t change."},{"line_number":83,"context_line":"        self.instance.deleted \u003d 0"},{"line_number":84,"context_line":"        self.instance.disable_terminate \u003d False"},{"line_number":85,"context_line":"        self.instance.terminated_at \u003d None"}],"source_content_type":"text/x-python","patch_set":63,"id":"3ac371cc_2bcdfa5d","line":82,"updated":"2016-08-18 22:18:31.000000000","message":"I take it this means it\u0027s not possible for a BuildRequest to exist any time after an instance has had these fields set. That is, it can never happen to query a BuildRequest from the DB and have its fields wiped out to these values.\n\nAsking because I was wondering if they should be set only if not set.","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"c3bd017a3cbedcfe107b2757d54c8acd8d8a877f","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                              \u0027BuildRequest\u0027))"},{"line_number":80,"context_line":"            raise exception.BuildRequestNotFound(uuid\u003dself.instance_uuid)"},{"line_number":81,"context_line":"        # NOTE(alaski): Set some fields on instance that are needed by the api,"},{"line_number":82,"context_line":"        # not lazy-loadable, and don\u0027t change."},{"line_number":83,"context_line":"        self.instance.deleted \u003d 0"},{"line_number":84,"context_line":"        self.instance.disable_terminate \u003d False"},{"line_number":85,"context_line":"        self.instance.terminated_at \u003d None"}],"source_content_type":"text/x-python","patch_set":63,"id":"1ac06dbe_25ac05ed","line":82,"in_reply_to":"3ac371cc_2bcdfa5d","updated":"2016-08-19 12:53:27.000000000","message":"All of these fields, except locked_by and deleted, are things that will be set on the instance once it\u0027s in a cell and at that point the BuildRequest will be deleted. Deleted we don\u0027t need to worry about because anything that would cause that to be set will just delete the BuildRequest instead.\n\nLocked_by could potentially be updated at some point in the future so I can add a check for that one.","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        # The fields above are not set until the instance is in a cell at"},{"line_number":92,"context_line":"        # which point this BuildRequest will be gone. locked_by could"},{"line_number":93,"context_line":"        # potentially be set by an update so it should not be overwritten."},{"line_number":94,"context_line":"        if not self.instance.obj_attr_is_set(\u0027locked_by\u0027):"},{"line_number":95,"context_line":"            self.instance.locked_by \u003d None"},{"line_number":96,"context_line":"        # created_at/updated_at are not on the serialized instance because it"},{"line_number":97,"context_line":"        # was never persisted."}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_f3c45741","line":94,"range":{"start_line":94,"start_character":15,"end_line":94,"end_character":57},"updated":"2016-08-19 21:28:40.000000000","message":"nit: I think you can use:\n\nif not \u0027locked_by\u0027 in instance: or whatever it is (maybe not in)","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb2d14827534324cfd2bdb12b2dc7cf9969627a1","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        # The fields above are not set until the instance is in a cell at"},{"line_number":92,"context_line":"        # which point this BuildRequest will be gone. locked_by could"},{"line_number":93,"context_line":"        # potentially be set by an update so it should not be overwritten."},{"line_number":94,"context_line":"        if not self.instance.obj_attr_is_set(\u0027locked_by\u0027):"},{"line_number":95,"context_line":"            self.instance.locked_by \u003d None"},{"line_number":96,"context_line":"        # created_at/updated_at are not on the serialized instance because it"},{"line_number":97,"context_line":"        # was never persisted."}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_1563d3b2","line":94,"range":{"start_line":94,"start_character":15,"end_line":94,"end_character":57},"in_reply_to":"1ac06dbe_a972707a","updated":"2016-08-19 23:30:18.000000000","message":"Yeah I know, definitely not -1 for this, it\u0027s just a thing we fixed in mitaka because it looks nicer.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        # The fields above are not set until the instance is in a cell at"},{"line_number":92,"context_line":"        # which point this BuildRequest will be gone. locked_by could"},{"line_number":93,"context_line":"        # potentially be set by an update so it should not be overwritten."},{"line_number":94,"context_line":"        if not self.instance.obj_attr_is_set(\u0027locked_by\u0027):"},{"line_number":95,"context_line":"            self.instance.locked_by \u003d None"},{"line_number":96,"context_line":"        # created_at/updated_at are not on the serialized instance because it"},{"line_number":97,"context_line":"        # was never persisted."}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_a972707a","line":94,"range":{"start_line":94,"start_character":15,"end_line":94,"end_character":57},"in_reply_to":"1ac06dbe_f3c45741","updated":"2016-08-19 22:01:39.000000000","message":"Good to know. I\u0027ve always used this because I know it won\u0027t trigger a lazy-load and can never remember if something else will.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8b0b150430f45653325083682c23872a5d30cb6d","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        # was never persisted."},{"line_number":98,"context_line":"        self.instance.created_at \u003d self.created_at"},{"line_number":99,"context_line":"        self.instance.updated_at \u003d self.updated_at"},{"line_number":100,"context_line":"        self.instance.tags \u003d objects.TagList([])"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def _load_block_device_mappings(self, db_bdms):"},{"line_number":103,"context_line":"        # \u0027db_bdms\u0027 is a serialized BlockDeviceMappingList object. If it\u0027s None"}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_c261da06","line":100,"updated":"2016-08-23 20:10:39.000000000","message":"I wonder if we shouldn\u0027t rather add defaults to the fields so that we could call out obj_set_defaults() on the keys we want instead of doing that explicitely (and would be less errorprone I guess).\n\nThat said, I wonder whether we can just add a default attribute to a field without needing to bump a version. Given the API contract (ie. enforcing a new default that was not there previously), I guess it\u0027s okay (compared to removing a default which would obviously require a version bump - and a backwards compatible path)","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"98c602e64273c3475fc1825ffb88474d3a80e20a","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        # was never persisted."},{"line_number":98,"context_line":"        self.instance.created_at \u003d self.created_at"},{"line_number":99,"context_line":"        self.instance.updated_at \u003d self.updated_at"},{"line_number":100,"context_line":"        self.instance.tags \u003d objects.TagList([])"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def _load_block_device_mappings(self, db_bdms):"},{"line_number":103,"context_line":"        # \u0027db_bdms\u0027 is a serialized BlockDeviceMappingList object. If it\u0027s None"}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_9914744d","line":100,"in_reply_to":"1ac06dbe_c261da06","updated":"2016-08-23 20:55:40.000000000","message":"meh. Most of these would be fine as a default, but even if they were defaulted in the object I would set them here because the BuildRequest instance has specific needs.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"}],"nova/objects/instance.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"a2640ab768359bbb1f205ee929ee04a72def7b03","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                                              columns_to_join\u003dcolumns_to_join,"},{"line_number":381,"context_line":"                                              use_slave\u003duse_slave)"},{"line_number":382,"context_line":"            return cls._from_db_object(context, cls(), db_inst,"},{"line_number":383,"context_line":"                                       expected_attrs)"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    @base.remotable_classmethod"},{"line_number":386,"context_line":"    def get_by_id(cls, context, inst_id, expected_attrs\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_de12d356","line":383,"updated":"2016-01-05 22:08:07.000000000","message":"I\u0027m still debating on whether this should be hidden in the Instance object methods or done one layer up.","commit_id":"015edf9b6e43f46ef6809e7b7fec3facad55f826"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"53a05722c6ae1264120c5c089a35c52060b6722c","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                                              columns_to_join\u003dcolumns_to_join,"},{"line_number":381,"context_line":"                                              use_slave\u003duse_slave)"},{"line_number":382,"context_line":"            return cls._from_db_object(context, cls(), db_inst,"},{"line_number":383,"context_line":"                                       expected_attrs)"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    @base.remotable_classmethod"},{"line_number":386,"context_line":"    def get_by_id(cls, context, inst_id, expected_attrs\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6ed579_165da214","line":383,"in_reply_to":"da6ed579_2dcb4355","updated":"2016-01-14 22:03:49.000000000","message":"It should just be api show/list calls.  I\u0027ll move it up a layer as this does feel like the wrong way to go about it.","commit_id":"015edf9b6e43f46ef6809e7b7fec3facad55f826"},{"author":{"_account_id":18247,"name":"Mark Doffman","email":"mdoffman@linux.vnet.ibm.com","username":"doffm"},"change_message_id":"10c9a195f01b5f848050251d6c1b3401a029101e","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                                              columns_to_join\u003dcolumns_to_join,"},{"line_number":381,"context_line":"                                              use_slave\u003duse_slave)"},{"line_number":382,"context_line":"            return cls._from_db_object(context, cls(), db_inst,"},{"line_number":383,"context_line":"                                       expected_attrs)"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    @base.remotable_classmethod"},{"line_number":386,"context_line":"    def get_by_id(cls, context, inst_id, expected_attrs\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6ed579_2dcb4355","line":383,"in_reply_to":"fa69d971_de12d356","updated":"2016-01-12 18:07:15.000000000","message":"+1 for one layer up. I somehow imagine that objects are closely related to their db table, so putting build_req functionality in to instance object doesn\u0027t seem great.\n\nWhere do you think getting the instance from the build request will be called? If its only on the show / list api calls probably can be one layer up. Only makes sense if there are a good number of places.","commit_id":"015edf9b6e43f46ef6809e7b7fec3facad55f826"}],"nova/tests/functional/db/test_build_request.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0fcb8556b5031a18a761e4d1fa2d339e76f9facc","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                self.assertEqual(1, len(db_value))"},{"line_number":64,"context_line":"                # Can\u0027t compare list objects directly, just compare the single"},{"line_number":65,"context_line":"                # item they contain."},{"line_number":66,"context_line":"                objects.base.obj_equal_prims(expected[0], db_value[0])"},{"line_number":67,"context_line":"                continue"},{"line_number":68,"context_line":"            self.assertEqual(expected, db_value)"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":62,"id":"9ad45d7e_d56ffbf9","line":66,"updated":"2016-08-11 07:39:08.000000000","message":"Nice improvements in here","commit_id":"f936cd12a3a270aef684d8da8149c4b542185cf6"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1ea3687453ce1130e043f1970f338d5baffd0d56","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            # but not when they\u0027re part of a serialized object and not a"},{"line_number":68,"context_line":"            # dedicated datetime column."},{"line_number":69,"context_line":"            self.assertEqual(db_val.replace(tzinfo\u003dNone),"},{"line_number":70,"context_line":"                             obj_val.replace(tzinfo\u003dNone))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        for key in self.build_req_obj.fields.keys():"},{"line_number":73,"context_line":"            expected \u003d getattr(req, key)"}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_05e749ad","side":"PARENT","line":70,"updated":"2016-08-24 20:29:37.000000000","message":"You remove that code because now you correctly set the instance-related date fields, right?","commit_id":"b292bf344187faf9a515e25312bdf1de562767a3"}],"nova/tests/functional/wsgi/test_servers.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"73b2ab1cb2567bbde49a1f503f78f5adf19462d8","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        create_resp \u003d self.api.api_post(\u0027servers\u0027, body)"},{"line_number":58,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":59,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":60,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":61,"context_line":"        # Just validate some basics"},{"line_number":62,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":63,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":63,"id":"3ac371cc_c1b405d6","line":60,"updated":"2016-08-18 22:18:31.000000000","message":"Does this ensure we have all of the necessary instance fields populated for a \u0027nova show\u0027? Just wondering if it would crash/fail without them. (I just tried it locally but it\u0027s unfortunately failing as-is).","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"18807ee1521710be3826f76eb65a9f44a8b546a4","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        create_resp \u003d self.api.api_post(\u0027servers\u0027, body)"},{"line_number":58,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":59,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":60,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":61,"context_line":"        # Just validate some basics"},{"line_number":62,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":63,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":63,"id":"1ac06dbe_a73746e2","line":60,"in_reply_to":"1ac06dbe_a3d8db1e","updated":"2016-08-19 18:14:47.000000000","message":"Perfect!","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"aef1eedde2b80eabf1a3289404248d7c52f9f6a5","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        create_resp \u003d self.api.api_post(\u0027servers\u0027, body)"},{"line_number":58,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":59,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":60,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":61,"context_line":"        # Just validate some basics"},{"line_number":62,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":63,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":63,"id":"1ac06dbe_a3d8db1e","line":60,"in_reply_to":"3ac371cc_c1b405d6","updated":"2016-08-19 17:59:22.000000000","message":"The way the api fixture is setup for the test class means that this code runs the latest microversion with all \"extensions\" running. So any missing field should cause a failure. The test failure on this revision was due to the \u0027body\u0027 dict above not including the now required \u0027network\u0027 key.","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":60,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":61,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":62,"context_line":"        # Just validate some basics"},{"line_number":63,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":64,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"},{"line_number":65,"context_line":"        self.assertEqual(\u00271\u0027, server[\u0027flavor\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_73d187cd","line":62,"updated":"2016-08-19 21:28:40.000000000","message":"How about more interesting fields like locked_by, launched_at or tags? This is the case that the instance is synthesized from the BuildRequest right - and build_request.instance has to set some special things, so maybe we should compare those? Like make sure created_at is equal between the server and the build request.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":60,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":61,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":62,"context_line":"        # Just validate some basics"},{"line_number":63,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":64,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"},{"line_number":65,"context_line":"        self.assertEqual(\u00271\u0027, server[\u0027flavor\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_e9dbc819","line":62,"in_reply_to":"1ac06dbe_73d187cd","updated":"2016-08-19 22:01:39.000000000","message":"Sure. I added this because Tempest was blowing up just trying to get the instance, so my main concern was that something sane was returned rather than an error. But I can add additional validation.\n\nCreated_at is going to end up being different between the BuildRequest and eventual Instance because it\u0027s set when the record is inserted into the db. That could be overwritten when the Instance is recorded but that\u0027s not something that can be checked here because this test class specifically tests things before scheduling occurs.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"81a20e0758cb6c001be78a532d3ff7e29e60429f","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":60,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":61,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":62,"context_line":"        # Just validate some basics"},{"line_number":63,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":64,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"},{"line_number":65,"context_line":"        self.assertEqual(\u00271\u0027, server[\u0027flavor\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_3daf9682","line":62,"in_reply_to":"1ac06dbe_b5ce3f81","updated":"2016-08-22 14:10:21.000000000","message":"Currently instance creation happens before scheduling, but I have a patch up which moves that until after scheduling. When that happens this test run will not trigger the code that writes an instance record.\n\nEnsuring that the created_at time remains the same between the BuildRequest and Instance record will need to happen in a different test, likely attached to the patch that moves instance creation to after scheduling.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb2d14827534324cfd2bdb12b2dc7cf9969627a1","unresolved":false,"context_lines":[{"line_number":59,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":60,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":61,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":62,"context_line":"        # Just validate some basics"},{"line_number":63,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":64,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"},{"line_number":65,"context_line":"        self.assertEqual(\u00271\u0027, server[\u0027flavor\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_b5ce3f81","line":62,"in_reply_to":"1ac06dbe_e9dbc819","updated":"2016-08-19 23:30:18.000000000","message":"Right, so this test runs before scheduling occurs, so I guess what you\u0027re saying is that checking created_at could be a race between the time that we had to fallback to build the instance from the BuildRequest and when the Instance was actually created in the database?\n\nWe could stub out the DB API method that creates the instance, so the instance doesn\u0027t actually get created in the database and must be created from the BuildRequest, but that\u0027s pretty nasty in a functional test.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fee053ee8f351e2b733dfb2e6febe0b2027ddf87","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":79,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":80,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":81,"context_line":"        # Just validate some basics"},{"line_number":82,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":83,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"},{"line_number":84,"context_line":"        self.assertEqual(\u00271\u0027, server[\u0027flavor\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_937403d0","line":81,"updated":"2016-08-19 21:28:40.000000000","message":"Ditto, what\u0027s different about this and the above? In this case, we get the instance from the db, not the build request, so couldn\u0027t we check to make sure that the created_at field is different between the server and build request to make sure the instance wasn\u0027t synthesized from the BuildRequest?","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"aaf0ca4b2fb9d6cd8a7922edbd9ab35bc9eb2e3e","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":79,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":80,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":81,"context_line":"        # Just validate some basics"},{"line_number":82,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":83,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"},{"line_number":84,"context_line":"        self.assertEqual(\u00271\u0027, server[\u0027flavor\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_aa105902","line":81,"in_reply_to":"1ac06dbe_89517465","updated":"2016-08-22 15:00:24.000000000","message":"I should add that these tests are mostly just safeguards to check that I haven\u0027t broken anything major. I\u0027m less concerned with testing full functionality here, as unit tests cover much of that, just checking that nothing major is overlooked.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"75130b392c5581dc14f4672a6b00c3c57ad608fb","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        get_resp \u003d self.api.api_get(\u0027servers/%s\u0027 %"},{"line_number":79,"context_line":"                                    create_resp.body[\u0027server\u0027][\u0027id\u0027])"},{"line_number":80,"context_line":"        server \u003d get_resp.body[\u0027server\u0027]"},{"line_number":81,"context_line":"        # Just validate some basics"},{"line_number":82,"context_line":"        self.assertEqual(\u0027foo\u0027, server[\u0027name\u0027])"},{"line_number":83,"context_line":"        self.assertEqual(image_ref, server[\u0027image\u0027][\u0027id\u0027])"},{"line_number":84,"context_line":"        self.assertEqual(\u00271\u0027, server[\u0027flavor\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":65,"id":"1ac06dbe_89517465","line":81,"in_reply_to":"1ac06dbe_937403d0","updated":"2016-08-19 22:01:39.000000000","message":"We have no way of getting the BuildRequest in this case without doing some very invasive things in the service setup afaik.\n\nIn fact this test is going to need to go away at some point because we won\u0027t have old versions to test against. Once instance creation moves to the conductor this test will no longer be possible.","commit_id":"c47767a9644bc0d656d224f10b9d8f3584c047ad"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1ea3687453ce1130e043f1970f338d5baffd0d56","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        self.assertEqual(\u0027scheduling\u0027, server[\u0027OS-EXT-STS:task_state\u0027])"},{"line_number":100,"context_line":"        self.assertEqual(\u0027building\u0027, server[\u0027OS-EXT-STS:vm_state\u0027])"},{"line_number":101,"context_line":"        self.assertEqual(\u0027BUILD\u0027, server[\u0027status\u0027])"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def test_delete_instance_from_buildrequest(self):"},{"line_number":104,"context_line":"        self.useFixture(nova_fixtures.AllServicesCurrent())"},{"line_number":105,"context_line":"        image_ref \u003d fake_image.get_valid_image_id()"}],"source_content_type":"text/x-python","patch_set":67,"id":"1ac06dbe_cd3ee128","line":102,"updated":"2016-08-24 20:29:37.000000000","message":"Quick question, shouldn\u0027t we have a test for instances being already there and not using the BuildRequest ? I know it\u0027s covered by UTs, hence me not really seeing that as a big concern.","commit_id":"a0594c643abf6106a76812d1555eb2646e7c91c5"}],"nova/tests/unit/api/openstack/fakes.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8ba2b1c0526dd6002a07774baf8f7e830a40eaf6","unresolved":false,"context_lines":[{"line_number":676,"context_line":"def fake_instance_mapping_get_by_instance_uuid(cls, context, instance_uuid):"},{"line_number":677,"context_line":"    # NOTE(alaski): For now it is not expected that instances are mapped to a"},{"line_number":678,"context_line":"    # cell. As that changes over time this should change with it."},{"line_number":679,"context_line":"    raise exc.InstanceMappingNotFound(uuid\u003dinstance_uuid)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":""},{"line_number":682,"context_line":"def stub_out_instance_mapping(test):"}],"source_content_type":"text/x-python","patch_set":61,"id":"9ad45d7e_5647e84a","line":679,"updated":"2016-08-09 15:48:58.000000000","message":"How is this going to change over time? You\u0027re stubbing the call to always return NotFound, so we\u0027ll always assume not found right? Or did you mean it \"should be changed over time\" ?","commit_id":"ec2628e5798b7e5e322c5cb3df111cc2be9713ba"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"860d045bdaf97eda32bb0c7bd24d68a3bd1f182d","unresolved":false,"context_lines":[{"line_number":676,"context_line":"def fake_instance_mapping_get_by_instance_uuid(cls, context, instance_uuid):"},{"line_number":677,"context_line":"    # NOTE(alaski): For now it is not expected that instances are mapped to a"},{"line_number":678,"context_line":"    # cell. As that changes over time this should change with it."},{"line_number":679,"context_line":"    raise exc.InstanceMappingNotFound(uuid\u003dinstance_uuid)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":""},{"line_number":682,"context_line":"def stub_out_instance_mapping(test):"}],"source_content_type":"text/x-python","patch_set":61,"id":"9ad45d7e_3ec0467c","line":679,"in_reply_to":"9ad45d7e_5647e84a","updated":"2016-08-09 17:11:59.000000000","message":"What I mean is this fake should attempt to match what the prevailing status quo is at any given time. Right now most or all deployments will not have instancemappings in place so this fake sets up that condition. As more things are put in place to enourage/force deployers to \"migrate\" to cellsv2 this fake should change to return an instance_mapping.\n\nHonestly all I\u0027ve done is defer the pain of changing test assumptions until that later point.","commit_id":"ec2628e5798b7e5e322c5cb3df111cc2be9713ba"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8ba2b1c0526dd6002a07774baf8f7e830a40eaf6","unresolved":false,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":"def stub_out_instance_mapping(test):"},{"line_number":683,"context_line":"    test.stub_out(\u0027nova.objects.InstanceMapping.get_by_instance_uuid\u0027,"},{"line_number":684,"context_line":"            fake_instance_mapping_get_by_instance_uuid)"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"def fake_get_available_languages():"}],"source_content_type":"text/x-python","patch_set":61,"id":"9ad45d7e_36b07443","line":684,"updated":"2016-08-09 15:48:58.000000000","message":"While convenient, I really hate these \"stub the world\" mechanisms. They\u0027re super confusing for the instance, flavor, and network stuff that we have right now. Just sayin\u0027.","commit_id":"ec2628e5798b7e5e322c5cb3df111cc2be9713ba"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"860d045bdaf97eda32bb0c7bd24d68a3bd1f182d","unresolved":false,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":"def stub_out_instance_mapping(test):"},{"line_number":683,"context_line":"    test.stub_out(\u0027nova.objects.InstanceMapping.get_by_instance_uuid\u0027,"},{"line_number":684,"context_line":"            fake_instance_mapping_get_by_instance_uuid)"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"def fake_get_available_languages():"}],"source_content_type":"text/x-python","patch_set":61,"id":"9ad45d7e_be61d6aa","line":684,"in_reply_to":"9ad45d7e_36b07443","updated":"2016-08-09 17:11:59.000000000","message":"Yeah, it\u0027s really laziness that drove this. Without this in place I need to go and fix up a lot more tests. I can do that though.","commit_id":"ec2628e5798b7e5e322c5cb3df111cc2be9713ba"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"7196fdfd6111fd224b524729ad6dc2c2496a973f","unresolved":false,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":"def stub_out_instance_mapping(test):"},{"line_number":683,"context_line":"    test.stub_out(\u0027nova.objects.InstanceMapping.get_by_instance_uuid\u0027,"},{"line_number":684,"context_line":"            fake_instance_mapping_get_by_instance_uuid)"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"def fake_get_available_languages():"}],"source_content_type":"text/x-python","patch_set":61,"id":"9ad45d7e_4f40e6a5","line":684,"in_reply_to":"9ad45d7e_be61d6aa","updated":"2016-08-09 18:22:14.000000000","message":"Since I\u0027ve added a version check around the new behavior this is no longer necessary. \\o/","commit_id":"ec2628e5798b7e5e322c5cb3df111cc2be9713ba"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9725707b1b92bf6fe47565757419bde24c4ec315","unresolved":false,"context_lines":[{"line_number":8160,"context_line":"        # Test get instance."},{"line_number":8161,"context_line":"        c \u003d context.get_admin_context()"},{"line_number":8162,"context_line":"        exp_instance \u003d self._create_fake_instance_obj()"},{"line_number":8163,"context_line":"        with mock.patch.object(objects.InstanceMapping,"},{"line_number":8164,"context_line":"                \u0027get_by_instance_uuid\u0027,"},{"line_number":8165,"context_line":"                return_value\u003dobjects.InstanceMapping("},{"line_number":8166,"context_line":"                    cell_mapping\u003dobjects.CellMapping())):"}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_4e2a4491","line":8163,"updated":"2016-05-22 22:37:40.000000000","message":"nit: here and below, nicer to use decorator mock.patch instead of context manager mock.patch.object...","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4c31f5ae6d2d4a5d8a83eb27374f3d54b334d8c7","unresolved":false,"context_lines":[{"line_number":8160,"context_line":"        # Test get instance."},{"line_number":8161,"context_line":"        c \u003d context.get_admin_context()"},{"line_number":8162,"context_line":"        exp_instance \u003d self._create_fake_instance_obj()"},{"line_number":8163,"context_line":"        with mock.patch.object(objects.InstanceMapping,"},{"line_number":8164,"context_line":"                \u0027get_by_instance_uuid\u0027,"},{"line_number":8165,"context_line":"                return_value\u003dobjects.InstanceMapping("},{"line_number":8166,"context_line":"                    cell_mapping\u003dobjects.CellMapping())):"}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_7bdeccdd","line":8163,"in_reply_to":"bab6814e_4e2a4491","updated":"2016-05-23 12:47:21.000000000","message":"Okay, will fix.","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"73b2ab1cb2567bbde49a1f503f78f5adf19462d8","unresolved":false,"context_lines":[{"line_number":8199,"context_line":"        with mock.patch.object(objects.InstanceMapping,"},{"line_number":8200,"context_line":"                \u0027get_by_instance_uuid\u0027,"},{"line_number":8201,"context_line":"                return_value\u003dobjects.InstanceMapping("},{"line_number":8202,"context_line":"                    cell_mapping\u003dobjects.CellMapping())):"},{"line_number":8203,"context_line":"            instance \u003d self.compute_api.get(self.context, exp_instance.uuid)"},{"line_number":8204,"context_line":"        self.assertEqual(exp_instance.id, instance.id)"},{"line_number":8205,"context_line":""}],"source_content_type":"text/x-python","patch_set":63,"id":"3ac371cc_c4fd330a","line":8202,"updated":"2016-08-18 22:18:31.000000000","message":"I noticed in the test_servers.py functional tests, the absence of the AllServicesCurrent fixture is supposed to mean \"old service\" and the InstanceMapping lookup is skipped. So I\u0027m not understanding how the instance mapping gets looked up here (how mocking this is useful) since we don\u0027t have AllServicesCurrent. I thought it would take the path of service version \u003c 15?","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"18807ee1521710be3826f76eb65a9f44a8b546a4","unresolved":false,"context_lines":[{"line_number":8199,"context_line":"        with mock.patch.object(objects.InstanceMapping,"},{"line_number":8200,"context_line":"                \u0027get_by_instance_uuid\u0027,"},{"line_number":8201,"context_line":"                return_value\u003dobjects.InstanceMapping("},{"line_number":8202,"context_line":"                    cell_mapping\u003dobjects.CellMapping())):"},{"line_number":8203,"context_line":"            instance \u003d self.compute_api.get(self.context, exp_instance.uuid)"},{"line_number":8204,"context_line":"        self.assertEqual(exp_instance.id, instance.id)"},{"line_number":8205,"context_line":""}],"source_content_type":"text/x-python","patch_set":63,"id":"1ac06dbe_e7b65e3c","line":8202,"in_reply_to":"1ac06dbe_833f9f24","updated":"2016-08-19 18:14:47.000000000","message":"Okay. It was confusing to me because I was trying to figure out what code it was exercising/testing by having a cell mapping that\u0027s not None. And then I realized instance mapping shouldn\u0027t be looked up at all unless AllServicesCurrent, IIUC.","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"101c9607e6c3972983cc15e658c91d86d01b6713","unresolved":false,"context_lines":[{"line_number":8199,"context_line":"        with mock.patch.object(objects.InstanceMapping,"},{"line_number":8200,"context_line":"                \u0027get_by_instance_uuid\u0027,"},{"line_number":8201,"context_line":"                return_value\u003dobjects.InstanceMapping("},{"line_number":8202,"context_line":"                    cell_mapping\u003dobjects.CellMapping())):"},{"line_number":8203,"context_line":"            instance \u003d self.compute_api.get(self.context, exp_instance.uuid)"},{"line_number":8204,"context_line":"        self.assertEqual(exp_instance.id, instance.id)"},{"line_number":8205,"context_line":""}],"source_content_type":"text/x-python","patch_set":63,"id":"1ac06dbe_6746ee24","line":8202,"in_reply_to":"1ac06dbe_e7b65e3c","updated":"2016-08-19 18:24:17.000000000","message":"Yep. Just tested and these can all be removed.","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"aef1eedde2b80eabf1a3289404248d7c52f9f6a5","unresolved":false,"context_lines":[{"line_number":8199,"context_line":"        with mock.patch.object(objects.InstanceMapping,"},{"line_number":8200,"context_line":"                \u0027get_by_instance_uuid\u0027,"},{"line_number":8201,"context_line":"                return_value\u003dobjects.InstanceMapping("},{"line_number":8202,"context_line":"                    cell_mapping\u003dobjects.CellMapping())):"},{"line_number":8203,"context_line":"            instance \u003d self.compute_api.get(self.context, exp_instance.uuid)"},{"line_number":8204,"context_line":"        self.assertEqual(exp_instance.id, instance.id)"},{"line_number":8205,"context_line":""}],"source_content_type":"text/x-python","patch_set":63,"id":"1ac06dbe_833f9f24","line":8202,"in_reply_to":"3ac371cc_c4fd330a","updated":"2016-08-19 17:59:22.000000000","message":"This may be cruft from an earlier revision where the service check was in a different place. Let me try removing and retesting, but I should be able to remove this.","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"17c25dd78ff346dbb262c32a13632541a4245b39","unresolved":false,"context_lines":[{"line_number":3731,"context_line":""},{"line_number":3732,"context_line":"        # Just check that an InstanceMappingNotFound causes the instance to"},{"line_number":3733,"context_line":"        # get looked up normally."},{"line_number":3734,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":3735,"context_line":"        self.compute_api.get(self.context, instance.uuid, want_objects\u003dTrue)"},{"line_number":3736,"context_line":"        self.assertFalse(mock_get_build_req.called)"},{"line_number":3737,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_d2d99923","line":3734,"updated":"2016-05-17 17:38:10.000000000","message":"This is a bit strange IMHO, creating an instance to test that lookup of an instance happened. It would make more sense to me to pass a uuid sentinel to compute_api.get and then assert that mock_get_inst was called with that uuid","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"17c25dd78ff346dbb262c32a13632541a4245b39","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":3735,"context_line":"        self.compute_api.get(self.context, instance.uuid, want_objects\u003dTrue)"},{"line_number":3736,"context_line":"        self.assertFalse(mock_get_build_req.called)"},{"line_number":3737,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3738,"context_line":""},{"line_number":3739,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027)"},{"line_number":3740,"context_line":"    @mock.patch.object(objects.BuildRequest, \u0027get_by_instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_128a011b","line":3737,"updated":"2016-05-17 17:38:10.000000000","message":"Don\u0027t we also want to assert that mock_get_inst was called with the uuid?\n\nAlso, I\u0027m not sure if this is overkill, but we could also assert that what\u0027s returned from compute_api.get is the instance returned if you set the mock_get_inst.return_value. Maybe that\u0027s out of scope for this test","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fd85eb2f56f482807e76b3e722c1cf864e323293","unresolved":false,"context_lines":[{"line_number":3734,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":3735,"context_line":"        self.compute_api.get(self.context, instance.uuid, want_objects\u003dTrue)"},{"line_number":3736,"context_line":"        self.assertFalse(mock_get_build_req.called)"},{"line_number":3737,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3738,"context_line":""},{"line_number":3739,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027)"},{"line_number":3740,"context_line":"    @mock.patch.object(objects.BuildRequest, \u0027get_by_instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_ec05d979","line":3737,"in_reply_to":"dab17558_128a011b","updated":"2016-05-17 19:26:31.000000000","message":"Yeah. Using a sentinel would be a good cleanup here. Checking the result of compute_api.get puts me back at creating an instance object for the test. That gets a lot of functional testing by Tempest so I\u0027ll leave it out of here.","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"17c25dd78ff346dbb262c32a13632541a4245b39","unresolved":false,"context_lines":[{"line_number":3745,"context_line":"                cell_mapping\u003dNone)"},{"line_number":3746,"context_line":"        mock_get_build_req.return_value \u003d build_req_obj"},{"line_number":3747,"context_line":""},{"line_number":3748,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":3749,"context_line":"        inst_from_build_req \u003d self.compute_api.get(self.context, instance.uuid,"},{"line_number":3750,"context_line":"                want_objects\u003dTrue)"},{"line_number":3751,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_726a6d1b","line":3748,"updated":"2016-05-17 17:38:10.000000000","message":"Again don\u0027t see utility/meaning in creating an instance object here. Just need a uuid","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"17c25dd78ff346dbb262c32a13632541a4245b39","unresolved":false,"context_lines":[{"line_number":3751,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3752,"context_line":"        mock_get_build_req.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3753,"context_line":"        self.assertEqual(build_req_obj.instance,"},{"line_number":3754,"context_line":"                inst_from_build_req)"},{"line_number":3755,"context_line":""},{"line_number":3756,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027)"},{"line_number":3757,"context_line":"    @mock.patch.object(objects.BuildRequest, \u0027get_by_instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_b27a15b8","line":3754,"updated":"2016-05-17 17:38:10.000000000","message":"This can go on the previous line, I think. Also, what is this testing? inst_from_build_req will be None AFAICT and this is asserting None \u003d\u003d None. Or is that what you intended?\n\nIf not, maybe keep the creation of an instance object and set build_req_obj.instance \u003d instance before compute_api.get?","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"73e62baded3c801dd3b35c03a5cc0576f4c04174","unresolved":false,"context_lines":[{"line_number":3751,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3752,"context_line":"        mock_get_build_req.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3753,"context_line":"        self.assertEqual(build_req_obj.instance,"},{"line_number":3754,"context_line":"                inst_from_build_req)"},{"line_number":3755,"context_line":""},{"line_number":3756,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027)"},{"line_number":3757,"context_line":"    @mock.patch.object(objects.BuildRequest, \u0027get_by_instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_31f730d6","line":3754,"in_reply_to":"dab17558_2ce6c1af","updated":"2016-05-18 01:47:04.000000000","message":"Oops, sorry! I missed that fake_build_request.fake_req_obj creates its own Instance object too","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fd85eb2f56f482807e76b3e722c1cf864e323293","unresolved":false,"context_lines":[{"line_number":3751,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3752,"context_line":"        mock_get_build_req.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3753,"context_line":"        self.assertEqual(build_req_obj.instance,"},{"line_number":3754,"context_line":"                inst_from_build_req)"},{"line_number":3755,"context_line":""},{"line_number":3756,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027)"},{"line_number":3757,"context_line":"    @mock.patch.object(objects.BuildRequest, \u0027get_by_instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_2ce6c1af","line":3754,"in_reply_to":"dab17558_b27a15b8","updated":"2016-05-17 19:26:31.000000000","message":"inst_from_build_req should be equal to the instance that\u0027s created in fake_build_request.fake_req_obj(self.context) on line 3743. Creating the instance on line 3748 is totally confusing here and should be removed. instance \u003d build_req_obj.instance would hopefully clarify things.","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"17c25dd78ff346dbb262c32a13632541a4245b39","unresolved":false,"context_lines":[{"line_number":3770,"context_line":""},{"line_number":3771,"context_line":"        self.compute_api.get(self.context, instance.uuid, want_objects\u003dTrue)"},{"line_number":3772,"context_line":"        self.assertFalse(mock_get_build_req.called)"},{"line_number":3773,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context, instance.uuid)"},{"line_number":3774,"context_line":""},{"line_number":3775,"context_line":""},{"line_number":3776,"context_line":"class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":29,"id":"dab17558_4daea881","line":3773,"updated":"2016-05-17 17:38:10.000000000","message":"Also assert that mock_get_inst was called with the uuid? Or assert that the instance returned from compute_api.get is the fake instance created earlier, either way","commit_id":"2b9e0d37236b086cdf54b3a42579590a3431b03d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9725707b1b92bf6fe47565757419bde24c4ec315","unresolved":false,"context_lines":[{"line_number":3783,"context_line":"                                                  \u0027metadata\u0027,"},{"line_number":3784,"context_line":"                                                  \u0027system_metadata\u0027,"},{"line_number":3785,"context_line":"                                                  \u0027security_groups\u0027,"},{"line_number":3786,"context_line":"                                                  \u0027info_cache\u0027])"},{"line_number":3787,"context_line":""},{"line_number":3788,"context_line":""},{"line_number":3789,"context_line":"class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_8e1eec6d","line":3786,"updated":"2016-05-22 22:37:40.000000000","message":"++ good unit tests for the code proposed in this patch.","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"ffe163053adbb4d4bbfb62ef83e4e766ea60a7f1","unresolved":false,"context_lines":[{"line_number":4030,"context_line":"            mock_get_inst_map):"},{"line_number":4031,"context_line":"        # This just checks that the instance is looked up normally and not"},{"line_number":4032,"context_line":"        # synthesized from a BuildRequest object. Verification of pulling the"},{"line_number":4033,"context_line":"        # instance from the proper cell will be added when that capability is."},{"line_number":4034,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":4035,"context_line":"        build_req_obj \u003d fake_build_request.fake_req_obj(self.context)"},{"line_number":4036,"context_line":"        mock_get_inst_map.return_value \u003d objects.InstanceMapping("}],"source_content_type":"text/x-python","patch_set":58,"id":"bacf61ea_111ea30b","line":4033,"updated":"2016-08-05 17:56:08.000000000","message":"missing word at end of comment?","commit_id":"7f3610cbbff512750566e8020a420a66c6b43135"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"73b2ab1cb2567bbde49a1f503f78f5adf19462d8","unresolved":false,"context_lines":[{"line_number":4083,"context_line":"        # Just check that an InstanceMappingNotFound causes the instance to"},{"line_number":4084,"context_line":"        # get looked up normally."},{"line_number":4085,"context_line":"        self.compute_api.get(self.context, uuids.inst_uuid)"},{"line_number":4086,"context_line":"        self.assertFalse(mock_get_build_req.called)"},{"line_number":4087,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context,"},{"line_number":4088,"context_line":"                                                  uuids.inst_uuid)"},{"line_number":4089,"context_line":"        mock_get_inst.assert_called_once_with(self.context, uuids.inst_uuid,"}],"source_content_type":"text/x-python","patch_set":63,"id":"3ac371cc_5001f938","line":4086,"updated":"2016-08-18 22:18:31.000000000","message":"Info tidbit: there is now assert_not_called() in the mock library [1]. I can\u0027t figure out which version it was introduced though.\n\n[1] https://github.com/testing-cabal/mock/blob/74eae17/NEWS#L42","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"aef1eedde2b80eabf1a3289404248d7c52f9f6a5","unresolved":false,"context_lines":[{"line_number":4083,"context_line":"        # Just check that an InstanceMappingNotFound causes the instance to"},{"line_number":4084,"context_line":"        # get looked up normally."},{"line_number":4085,"context_line":"        self.compute_api.get(self.context, uuids.inst_uuid)"},{"line_number":4086,"context_line":"        self.assertFalse(mock_get_build_req.called)"},{"line_number":4087,"context_line":"        mock_get_inst_map.assert_called_once_with(self.context,"},{"line_number":4088,"context_line":"                                                  uuids.inst_uuid)"},{"line_number":4089,"context_line":"        mock_get_inst.assert_called_once_with(self.context, uuids.inst_uuid,"}],"source_content_type":"text/x-python","patch_set":63,"id":"1ac06dbe_a3461bbd","line":4086,"in_reply_to":"3ac371cc_5001f938","updated":"2016-08-19 17:59:22.000000000","message":"Cool, I\u0027ll try that.","commit_id":"028b252ea9a1db28e533909d901939bca75b8ac6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e9bdbb4c8a694aef1fa2bca2de2618fe076b5583","unresolved":false,"context_lines":[{"line_number":4113,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027)"},{"line_number":4114,"context_line":"    @mock.patch.object(objects.BuildRequest, \u0027get_by_instance_uuid\u0027)"},{"line_number":4115,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":4116,"context_line":"    def test_get_instance_not_in_cell_buildreq_deleted_inst_in_cell("},{"line_number":4117,"context_line":"            self, mock_get_inst, mock_get_build_req, mock_get_inst_map,"},{"line_number":4118,"context_line":"            mock_target_cell):"},{"line_number":4119,"context_line":"        self.useFixture(fixtures.AllServicesCurrent())"}],"source_content_type":"text/x-python","patch_set":66,"id":"1ac06dbe_02ace48a","line":4116,"updated":"2016-08-22 19:35:57.000000000","message":"This technically says what\u0027s going on but a docstring or comment would have been nice :P","commit_id":"af25863b04703c7bc564d9f36f61c6bed08ee3ac"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4529e4a5a7658a056aa4e07dbe17c68452482e07","unresolved":false,"context_lines":[{"line_number":4113,"context_line":"    @mock.patch.object(objects.InstanceMapping, \u0027get_by_instance_uuid\u0027)"},{"line_number":4114,"context_line":"    @mock.patch.object(objects.BuildRequest, \u0027get_by_instance_uuid\u0027)"},{"line_number":4115,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":4116,"context_line":"    def test_get_instance_not_in_cell_buildreq_deleted_inst_in_cell("},{"line_number":4117,"context_line":"            self, mock_get_inst, mock_get_build_req, mock_get_inst_map,"},{"line_number":4118,"context_line":"            mock_target_cell):"},{"line_number":4119,"context_line":"        self.useFixture(fixtures.AllServicesCurrent())"}],"source_content_type":"text/x-python","patch_set":66,"id":"1ac06dbe_481a7b7e","line":4116,"in_reply_to":"1ac06dbe_02ace48a","updated":"2016-08-22 20:29:31.000000000","message":"Can do.","commit_id":"af25863b04703c7bc564d9f36f61c6bed08ee3ac"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e9bdbb4c8a694aef1fa2bca2de2618fe076b5583","unresolved":false,"context_lines":[{"line_number":4131,"context_line":""},{"line_number":4132,"context_line":"        inst_map_calls \u003d [mock.call(self.context, instance.uuid),"},{"line_number":4133,"context_line":"                          mock.call(self.context, instance.uuid)]"},{"line_number":4134,"context_line":"        mock_get_inst_map.assert_has_calls(inst_map_calls)"},{"line_number":4135,"context_line":"        mock_get_build_req.assert_called_once_with(self.context, instance.uuid)"},{"line_number":4136,"context_line":"        mock_target_cell.assert_called_once_with(self.context,"},{"line_number":4137,"context_line":"                                                 inst_map.cell_mapping)"}],"source_content_type":"text/x-python","patch_set":66,"id":"1ac06dbe_0780768a","line":4134,"updated":"2016-08-22 19:35:57.000000000","message":"Technically you need to also self.assertEqual(2, mock_get_inst_map.call_count) to assert that you expected 2 calls to have happened. \u0027has_calls\u0027 isn\u0027t bounded","commit_id":"af25863b04703c7bc564d9f36f61c6bed08ee3ac"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4529e4a5a7658a056aa4e07dbe17c68452482e07","unresolved":false,"context_lines":[{"line_number":4131,"context_line":""},{"line_number":4132,"context_line":"        inst_map_calls \u003d [mock.call(self.context, instance.uuid),"},{"line_number":4133,"context_line":"                          mock.call(self.context, instance.uuid)]"},{"line_number":4134,"context_line":"        mock_get_inst_map.assert_has_calls(inst_map_calls)"},{"line_number":4135,"context_line":"        mock_get_build_req.assert_called_once_with(self.context, instance.uuid)"},{"line_number":4136,"context_line":"        mock_target_cell.assert_called_once_with(self.context,"},{"line_number":4137,"context_line":"                                                 inst_map.cell_mapping)"}],"source_content_type":"text/x-python","patch_set":66,"id":"1ac06dbe_281f3f6d","line":4134,"in_reply_to":"1ac06dbe_0780768a","updated":"2016-08-22 20:29:31.000000000","message":"Well that\u0027s annoying.","commit_id":"af25863b04703c7bc564d9f36f61c6bed08ee3ac"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e9bdbb4c8a694aef1fa2bca2de2618fe076b5583","unresolved":false,"context_lines":[{"line_number":4167,"context_line":""},{"line_number":4168,"context_line":"        inst_map_calls \u003d [mock.call(self.context, instance.uuid),"},{"line_number":4169,"context_line":"                          mock.call(self.context, instance.uuid)]"},{"line_number":4170,"context_line":"        mock_get_inst_map.assert_has_calls(inst_map_calls)"},{"line_number":4171,"context_line":"        mock_get_build_req.assert_called_once_with(self.context, instance.uuid)"},{"line_number":4172,"context_line":"        mock_target_cell.assert_not_called()"},{"line_number":4173,"context_line":"        mock_get_inst.assert_called_once_with(self.context, instance.uuid,"}],"source_content_type":"text/x-python","patch_set":66,"id":"1ac06dbe_a2da30e6","line":4170,"updated":"2016-08-22 19:35:57.000000000","message":"Same comment","commit_id":"af25863b04703c7bc564d9f36f61c6bed08ee3ac"}],"nova/tests/unit/fake_build_request.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9725707b1b92bf6fe47565757419bde24c4ec315","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    instance \u003d fake_instance.fake_instance_obj(ctxt, objects.Instance,"},{"line_number":28,"context_line":"            uuid\u003dinstance_uuid)"},{"line_number":29,"context_line":"    # This will always be set this way for an instance at build time"},{"line_number":30,"context_line":"    instance.host \u003d None"},{"line_number":31,"context_line":"    db_build_request \u003d {"},{"line_number":32,"context_line":"            \u0027id\u0027: 1,"},{"line_number":33,"context_line":"            \u0027project_id\u0027: \u0027fake-project\u0027,"}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_ae1bf07f","line":30,"updated":"2016-05-22 22:37:40.000000000","message":"What about hypervisor_hostname, which is used in the scheduling/placement process?","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4c31f5ae6d2d4a5d8a83eb27374f3d54b334d8c7","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    instance \u003d fake_instance.fake_instance_obj(ctxt, objects.Instance,"},{"line_number":28,"context_line":"            uuid\u003dinstance_uuid)"},{"line_number":29,"context_line":"    # This will always be set this way for an instance at build time"},{"line_number":30,"context_line":"    instance.host \u003d None"},{"line_number":31,"context_line":"    db_build_request \u003d {"},{"line_number":32,"context_line":"            \u0027id\u0027: 1,"},{"line_number":33,"context_line":"            \u0027project_id\u0027: \u0027fake-project\u0027,"}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_dbc1b8e5","line":30,"in_reply_to":"bab6814e_ae1bf07f","updated":"2016-05-23 12:47:21.000000000","message":"Hmm, I\u0027m not sure I saw that being set anywhere on instance before scheduling. But I\u0027ll double check, and can set it to None either way.","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3d7690101438f70435ad07c4b6133e4ca645523b","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    instance \u003d fake_instance.fake_instance_obj(ctxt, objects.Instance,"},{"line_number":28,"context_line":"            uuid\u003dinstance_uuid)"},{"line_number":29,"context_line":"    # This will always be set this way for an instance at build time"},{"line_number":30,"context_line":"    instance.host \u003d None"},{"line_number":31,"context_line":"    db_build_request \u003d {"},{"line_number":32,"context_line":"            \u0027id\u0027: 1,"},{"line_number":33,"context_line":"            \u0027project_id\u0027: \u0027fake-project\u0027,"}],"source_content_type":"text/x-python","patch_set":32,"id":"bab6814e_195f760f","line":30,"in_reply_to":"bab6814e_dbc1b8e5","updated":"2016-05-24 17:48:57.000000000","message":"hypervisor_hostname is set on a compute_node not an instance.","commit_id":"446f93d7c7a4fe0809b0cb4e0091d54a1b959974"}],"nova/tests/unit/objects/test_build_request.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"fc2a27f6d6c5724d8b99a276a380998ff9e329c5","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    def test_to_instance_object(self):"},{"line_number":86,"context_line":"        # These are exposed in the API. Ideally this list would be maintained"},{"line_number":87,"context_line":"        # in the API so as new fields are displayed we fail here and know to"},{"line_number":88,"context_line":"        # add the field to the instance object returned by to_instance_obj."},{"line_number":89,"context_line":"        displayed_fields \u003d [\u0027deleted\u0027, \u0027host\u0027, \u0027created_at\u0027, \u0027updated_at\u0027,"},{"line_number":90,"context_line":"                \u0027info_cache\u0027, \u0027power_state\u0027, \u0027config_drive\u0027, \u0027key_name\u0027,"},{"line_number":91,"context_line":"                \u0027locked_by\u0027, \u0027user_id\u0027, \u0027metadata\u0027, \u0027access_ip_v4\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_1d86bada","line":88,"updated":"2016-03-03 16:10:02.000000000","message":"This is my thinking on how to catch fields added through a new microversion. I\u0027d like a whitelist of fields that can be pulled from the instance object which would cause this test to fail if updated and not accounted for by BuildRequest.","commit_id":"a792a1dd35f12951bf33f09ea960984f8462102d"}]}