)]}'
{"nova/api/openstack/compute/extended_volumes.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1fdf51d4ab04e121091a56aa6e571478f3516cdb","unresolved":false,"context_lines":[{"line_number":46,"context_line":"            instance_bdms \u003d self._get_instance_bdms(bdms, server)"},{"line_number":47,"context_line":"            self._extend_server(context, server, req, instance_bdms)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def _get_instance_bdms_in_multiple_cells(self, ctxt, servers):"},{"line_number":50,"context_line":"        instance_uuids \u003d [server[\u0027id\u0027] for server in servers]"},{"line_number":51,"context_line":"        inst_maps \u003d objects.InstanceMappingList.get_by_instance_uuids("},{"line_number":52,"context_line":"                        ctxt, instance_uuids)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_8d9d9967","line":49,"updated":"2017-08-04 14:15:07.000000000","message":"nit: this could be a staticmethod","commit_id":"fc86565cdf5cf50003ce4e6ac63a38e46892568c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"63a845e2ae11963758d087220f8d1c0e4f5d543b","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        for inst_map in inst_maps:"},{"line_number":57,"context_line":"            if (inst_map.cell_mapping is not None and"},{"line_number":58,"context_line":"                    inst_map.cell_mapping.uuid not in cell_mappings):"},{"line_number":59,"context_line":"                cell_mappings.update("},{"line_number":60,"context_line":"                    {inst_map.cell_mapping.uuid: inst_map.cell_mapping})"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        bdms \u003d {}"}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_48655f45","line":59,"range":{"start_line":59,"start_character":0,"end_line":59,"end_character":37},"updated":"2017-08-04 14:53:22.000000000","message":"This seems kinda odd as you already checked that it\u0027s not present. Doesn\u0027t hurt anything, just seems weird.","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8cf4788c3eaac7d4707266ebf0a3d47d154e2787","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        bdms \u003d {}"},{"line_number":63,"context_line":"        for cell_mapping in cell_mappings.values():"},{"line_number":64,"context_line":"            with context.target_cell(ctxt, cell_mapping) as cctxt:"},{"line_number":65,"context_line":"                bdms.update("},{"line_number":66,"context_line":"                    objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":67,"context_line":"                        cctxt, instance_uuids))"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_931b7c71","line":65,"updated":"2017-08-04 14:59:21.000000000","message":"bdms is a dict, but you\u0027re updating it with a BlockDeviceMappingList. Surely that can\u0027t work right?","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"80619d665fd6f8a077c087afcabc2b7cc899e18d","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        bdms \u003d {}"},{"line_number":63,"context_line":"        for cell_mapping in cell_mappings.values():"},{"line_number":64,"context_line":"            with context.target_cell(ctxt, cell_mapping) as cctxt:"},{"line_number":65,"context_line":"                bdms.update("},{"line_number":66,"context_line":"                    objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":67,"context_line":"                        cctxt, instance_uuids))"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_73981874","line":65,"in_reply_to":"df3967d1_931b7c71","updated":"2017-08-04 15:08:09.000000000","message":"Okay I was looking at get_by_instance_uuids() not the confusingly similar bdms_by_instance_uuid().","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"82b99ee84194990bfc425ff01727bfc2b4abe98b","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        bdms \u003d {}"},{"line_number":63,"context_line":"        for cell_mapping in cell_mappings.values():"},{"line_number":64,"context_line":"            with context.target_cell(ctxt, cell_mapping) as cctxt:"},{"line_number":65,"context_line":"                bdms.update("},{"line_number":66,"context_line":"                    objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":67,"context_line":"                        cctxt, instance_uuids))"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_73c37858","line":65,"in_reply_to":"df3967d1_931b7c71","updated":"2017-08-04 15:06:34.000000000","message":"bdms_by_instance_uuid returns a dict of bdm lists keyed by instance uuid - it\u0027s confusing as hell.","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"63a845e2ae11963758d087220f8d1c0e4f5d543b","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            with context.target_cell(ctxt, cell_mapping) as cctxt:"},{"line_number":65,"context_line":"                bdms.update("},{"line_number":66,"context_line":"                    objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":67,"context_line":"                        cctxt, instance_uuids))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        return bdms"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_48c1dfe4","line":67,"updated":"2017-08-04 14:53:22.000000000","message":"Does this work if some of the uuids aren\u0027t found? Because you\u0027re querying each cell for the whole set of uuids, which will mean some are not found in each cell.","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8cf4788c3eaac7d4707266ebf0a3d47d154e2787","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            with context.target_cell(ctxt, cell_mapping) as cctxt:"},{"line_number":65,"context_line":"                bdms.update("},{"line_number":66,"context_line":"                    objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":67,"context_line":"                        cctxt, instance_uuids))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        return bdms"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_f371a8a9","line":67,"in_reply_to":"df3967d1_48c1dfe4","updated":"2017-08-04 14:59:21.000000000","message":"Looks like it does because it just does an \"in\" query for these UUIDs, and doesn\u0027t actually join the instances table or anything. Still, you\u0027re passing a bigger list to each cell than you need. Not necessarily a problem I guess, just .. overkill.","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4986a82a21625e66b057339123ceca6731122d64","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            with context.target_cell(ctxt, cell_mapping) as cctxt:"},{"line_number":65,"context_line":"                bdms.update("},{"line_number":66,"context_line":"                    objects.BlockDeviceMappingList.bdms_by_instance_uuid("},{"line_number":67,"context_line":"                        cctxt, instance_uuids))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        return bdms"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_93349cdf","line":67,"in_reply_to":"df3967d1_48c1dfe4","updated":"2017-08-04 14:59:15.000000000","message":"Yeah it returns a {}:\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/block_device.py#L332\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/base.py#L282\n\nBut I had to check that out too since it\u0027s confusing that it returns a dict keyed by instance_uuid and none of it\u0027s doc\u0027ed.","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"}],"nova/tests/unit/api/openstack/compute/test_extended_volumes.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1fdf51d4ab04e121091a56aa6e571478f3516cdb","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        fakes.stub_out_secgroup_api(self)"},{"line_number":100,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get)"},{"line_number":101,"context_line":"        self.stubs.Set(compute.api.API, \u0027get_all\u0027, fake_compute_get_all)"},{"line_number":102,"context_line":"        self.stub_out(\u0027nova.db.block_device_mapping_get_all_by_instance_uuids\u0027,"},{"line_number":103,"context_line":"                       fake_bdms_get_all_by_instance_uuids)"},{"line_number":104,"context_line":"        self._setUp()"},{"line_number":105,"context_line":"        self.app \u003d self._setup_app()"},{"line_number":106,"context_line":"        return_server \u003d fakes.fake_instance_get()"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_4d2e91fb","line":103,"range":{"start_line":102,"start_character":8,"end_line":103,"end_character":59},"updated":"2017-08-04 14:15:07.000000000","message":"OK so this is what actually stubs out the call to get the BDMs.","commit_id":"fc86565cdf5cf50003ce4e6ac63a38e46892568c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1fdf51d4ab04e121091a56aa6e571478f3516cdb","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        self.assertEqual(self.exp_volumes_show, actual)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    @mock.patch.object(objects.InstanceMappingList, \u0027get_by_instance_uuids\u0027)"},{"line_number":143,"context_line":"    def test_detail(self, mock_get_by_instance_uuids):"},{"line_number":144,"context_line":"        mock_get_by_instance_uuids.return_value \u003d ["},{"line_number":145,"context_line":"            objects.InstanceMapping("},{"line_number":146,"context_line":"                instance_uuid\u003dUUID1,"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_cd4161ab","line":143,"range":{"start_line":143,"start_character":26,"end_line":143,"end_character":52},"updated":"2017-08-04 14:15:07.000000000","message":"We should at least be asserting this was called. In other words, I can remove the fix and this test will still pass.","commit_id":"fc86565cdf5cf50003ce4e6ac63a38e46892568c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1fdf51d4ab04e121091a56aa6e571478f3516cdb","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            objects.InstanceMapping("},{"line_number":152,"context_line":"                instance_uuid\u003dUUID2,"},{"line_number":153,"context_line":"                cell_mapping\u003dobjects.CellMapping("},{"line_number":154,"context_line":"                    uuid\u003duuids.cell1,"},{"line_number":155,"context_line":"                    transport_url\u003d\u0027fake://nowhere/\u0027,"},{"line_number":156,"context_line":"                    database_connection\u003duuids.cell1))]"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_0dff6967","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":36},"updated":"2017-08-04 14:15:07.000000000","message":"This would be a more interesting test if you had at least two different cells providing instances/bdms here.","commit_id":"fc86565cdf5cf50003ce4e6ac63a38e46892568c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"01dff8fdb77eb14e3ec2cf4b3588fcf1ed61dbed","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            objects.InstanceMapping("},{"line_number":152,"context_line":"                instance_uuid\u003dUUID2,"},{"line_number":153,"context_line":"                cell_mapping\u003dobjects.CellMapping("},{"line_number":154,"context_line":"                    uuid\u003duuids.cell1,"},{"line_number":155,"context_line":"                    transport_url\u003d\u0027fake://nowhere/\u0027,"},{"line_number":156,"context_line":"                    database_connection\u003duuids.cell1))]"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_6d703509","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":36},"in_reply_to":"df3967d1_0dff6967","updated":"2017-08-04 14:17:53.000000000","message":"Oh I suppose you\u0027re restricted to cell1 because there are only two cells setup in the base class:\n\nhttps://github.com/openstack/nova/blob/master/nova/test.py#L323","commit_id":"fc86565cdf5cf50003ce4e6ac63a38e46892568c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"63a845e2ae11963758d087220f8d1c0e4f5d543b","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        res \u003d self._make_request(\u0027/detail\u0027)"},{"line_number":160,"context_line":"        mock_get_by_instance_uuids.assert_called_once_with("},{"line_number":161,"context_line":"            test.MatchType(nova_context.RequestContext), [UUID1, UUID2])"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        self.assertEqual(200, res.status_int)"},{"line_number":164,"context_line":"        for i, server in enumerate(self._get_servers(res.body)):"}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_086cf705","line":161,"updated":"2017-08-04 14:53:22.000000000","message":"This doesn\u0027t test that we targeted the cells we got, fwiw.","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4986a82a21625e66b057339123ceca6731122d64","unresolved":false,"context_lines":[{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        res \u003d self._make_request(\u0027/detail\u0027)"},{"line_number":160,"context_line":"        mock_get_by_instance_uuids.assert_called_once_with("},{"line_number":161,"context_line":"            test.MatchType(nova_context.RequestContext), [UUID1, UUID2])"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        self.assertEqual(200, res.status_int)"},{"line_number":164,"context_line":"        for i, server in enumerate(self._get_servers(res.body)):"}],"source_content_type":"text/x-python","patch_set":2,"id":"df3967d1_735db813","line":161,"in_reply_to":"df3967d1_086cf705","updated":"2017-08-04 14:59:15.000000000","message":"Yeah I noticed that earlier too, and it\u0027s related to how nova.db.block_device_mapping_get_all_by_instance_uuids is mocked out so we\u0027re not actually checking the context that\u0027s passed to objects.BlockDeviceMappingList.bdms_by_instance_uuid.","commit_id":"0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4"}]}