)]}'
{"nova/api/openstack/compute/server_groups.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ed8effd19f05581a8cc16e703e269d2a6c651c7","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        if not im.cell_mapping:"},{"line_number":57,"context_line":"            # Not scheduled yet, so just throw it in the final list"},{"line_number":58,"context_line":"            # and move on"},{"line_number":59,"context_line":"            found_inst_uuids.append(im.instance_uuid)"},{"line_number":60,"context_line":"            continue"},{"line_number":61,"context_line":"        if im.cell_mapping.uuid not in cell_mappings:"},{"line_number":62,"context_line":"            cell_mappings[im.cell_mapping.uuid] \u003d im.cell_mapping"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_e2347e95","line":59,"updated":"2017-04-19 15:58:59.000000000","message":"Just to make sure, are we sure that instance_mappings records without having cell_mapping set are only instances to be scheduled ? My guts say yes, but confirmation welcome.","commit_id":"073b74c730df922145630cb44dfff69280cfe505"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"34c3575b251f14d43e57ff82c42f5eef8bc9d759","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        if not im.cell_mapping:"},{"line_number":57,"context_line":"            # Not scheduled yet, so just throw it in the final list"},{"line_number":58,"context_line":"            # and move on"},{"line_number":59,"context_line":"            found_inst_uuids.append(im.instance_uuid)"},{"line_number":60,"context_line":"            continue"},{"line_number":61,"context_line":"        if im.cell_mapping.uuid not in cell_mappings:"},{"line_number":62,"context_line":"            cell_mappings[im.cell_mapping.uuid] \u003d im.cell_mapping"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_5d3af7cb","line":59,"in_reply_to":"7ffa3b31_e2347e95","updated":"2017-04-19 16:16:48.000000000","message":"Yes, that\u0027s the only reason. However, if it makes you feel better:\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/instance_mapping.py#L47-L48","commit_id":"073b74c730df922145630cb44dfff69280cfe505"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ed8effd19f05581a8cc16e703e269d2a6c651c7","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        filters \u003d {\u0027uuid\u0027: uuids, \u0027deleted\u0027: False}"},{"line_number":72,"context_line":"        found_inst_uuids.extend(["},{"line_number":73,"context_line":"            inst.uuid for inst in objects.InstanceList.get_by_filters("},{"line_number":74,"context_line":"                context, filters\u003dfilters)])"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    return found_inst_uuids"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_62498e2f","line":74,"updated":"2017-04-19 15:58:59.000000000","message":"I can be blind, but I don\u0027t see where you set_target_cell() for getting the list of instances matching the filters ?","commit_id":"073b74c730df922145630cb44dfff69280cfe505"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"34c3575b251f14d43e57ff82c42f5eef8bc9d759","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        filters \u003d {\u0027uuid\u0027: uuids, \u0027deleted\u0027: False}"},{"line_number":72,"context_line":"        found_inst_uuids.extend(["},{"line_number":73,"context_line":"            inst.uuid for inst in objects.InstanceList.get_by_filters("},{"line_number":74,"context_line":"                context, filters\u003dfilters)])"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    return found_inst_uuids"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_bd6d7bcd","line":74,"in_reply_to":"7ffa3b31_62498e2f","updated":"2017-04-19 16:16:48.000000000","message":"Yeah, it\u0027s clearly there. In plain sight. On L71.5 .. DUH.","commit_id":"073b74c730df922145630cb44dfff69280cfe505"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8ed8effd19f05581a8cc16e703e269d2a6c651c7","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        members \u003d []"},{"line_number":95,"context_line":"        if group.members:"},{"line_number":96,"context_line":"            # Display the instances that are not deleted."},{"line_number":97,"context_line":"            members \u003d _get_not_deleted(context, group.members)"},{"line_number":98,"context_line":"        server_group[\u0027members\u0027] \u003d members"},{"line_number":99,"context_line":"        # Add project id information to the response data for"},{"line_number":100,"context_line":"        # API version v2.13"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_57b0968a","line":97,"range":{"start_line":97,"start_character":22,"end_line":97,"end_character":38},"updated":"2017-04-19 15:58:59.000000000","message":"nit: I\u0027d have it named \u0027get_instances_not_deleted_by_uuids\u0027","commit_id":"073b74c730df922145630cb44dfff69280cfe505"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca887067c6a8865d6f26beb28d6eb66ac019f413","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # Get a master list of cell mappings, and a list of instance"},{"line_number":55,"context_line":"    # uuids organized by cell"},{"line_number":56,"context_line":"    for im in mappings:"},{"line_number":57,"context_line":"        if not im.cell_mapping:"},{"line_number":58,"context_line":"            # Not scheduled yet, so just throw it in the final list"},{"line_number":59,"context_line":"            # and move on"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_f6838203","line":56,"updated":"2017-04-24 17:55:23.000000000","message":"If the group\u0027s policy was \"affinity\" we could optimize by only getting the single cell and then we wouldn\u0027t need to iterate over the cell mappings, but in the end it probably doesn\u0027t save us much once we\u0027ve already loaded all of the instance and cell mappings out of the database, because after that it\u0027s just python work. In the affinity case the cell_mappings dict should be len\u003d\u003d1 anyway.","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6b304da3a6c22fce21d0d9702bbfe54fbc63d33c","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # Get a master list of cell mappings, and a list of instance"},{"line_number":55,"context_line":"    # uuids organized by cell"},{"line_number":56,"context_line":"    for im in mappings:"},{"line_number":57,"context_line":"        if not im.cell_mapping:"},{"line_number":58,"context_line":"            # Not scheduled yet, so just throw it in the final list"},{"line_number":59,"context_line":"            # and move on"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_161f0e6d","line":56,"in_reply_to":"5ff73747_f6838203","updated":"2017-04-24 18:11:01.000000000","message":"Yeah, that\u0027s interesting, although if you\u0027ve used forced-migration to move things around, there\u0027s no guarantee that they\u0027re still in the same place. Probably need to do this just to be safe...","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca887067c6a8865d6f26beb28d6eb66ac019f413","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    # Query each cell for the instances that are inside, building"},{"line_number":67,"context_line":"    # a list of non-deleted instance uuids."},{"line_number":68,"context_line":"    for cell_uuid, cell_mapping in cell_mappings.items():"},{"line_number":69,"context_line":"        uuids \u003d inst_by_cell[cell_uuid]"},{"line_number":70,"context_line":"        LOG.debug(\u0027Querying cell %(cell)s for %(num)i instances\u0027,"},{"line_number":71,"context_line":"                  {\u0027cell\u0027: cell_mapping.identity, \u0027num\u0027: len(uuids)})"},{"line_number":72,"context_line":"        filters \u003d {\u0027uuid\u0027: uuids, \u0027deleted\u0027: False}"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_16d82e51","line":69,"range":{"start_line":69,"start_character":8,"end_line":69,"end_character":13},"updated":"2017-04-24 17:55:23.000000000","message":"This masks the formal argument. Not a big deal but I\u0027m pointing it out.","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6b304da3a6c22fce21d0d9702bbfe54fbc63d33c","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    # Query each cell for the instances that are inside, building"},{"line_number":67,"context_line":"    # a list of non-deleted instance uuids."},{"line_number":68,"context_line":"    for cell_uuid, cell_mapping in cell_mappings.items():"},{"line_number":69,"context_line":"        uuids \u003d inst_by_cell[cell_uuid]"},{"line_number":70,"context_line":"        LOG.debug(\u0027Querying cell %(cell)s for %(num)i instances\u0027,"},{"line_number":71,"context_line":"                  {\u0027cell\u0027: cell_mapping.identity, \u0027num\u0027: len(uuids)})"},{"line_number":72,"context_line":"        filters \u003d {\u0027uuid\u0027: uuids, \u0027deleted\u0027: False}"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_d6188682","line":69,"range":{"start_line":69,"start_character":8,"end_line":69,"end_character":13},"in_reply_to":"5ff73747_16d82e51","updated":"2017-04-24 18:11:01.000000000","message":"Will fix.","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"}],"nova/tests/unit/api/openstack/compute/test_server_groups.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca887067c6a8865d6f26beb28d6eb66ac019f413","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"class ServerGroupTestV21(test.TestCase):"},{"line_number":80,"context_line":"    USES_DB \u003d False"},{"line_number":81,"context_line":"    USES_DB_SELF \u003d True"},{"line_number":82,"context_line":"    validation_error \u003d exception.ValidationError"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_368faa24","line":80,"range":{"start_line":80,"start_character":4,"end_line":80,"end_character":11},"updated":"2017-04-24 17:55:23.000000000","message":"Why not just change the test to inherit from NoDBTestCase?","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca887067c6a8865d6f26beb28d6eb66ac019f413","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        return ig.uuid"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def _create_groups_and_instances(self, ctx):"},{"line_number":182,"context_line":"        cells \u003d {cm.uuid: cm for cm in objects.CellMappingList.get_all(ctx)}"},{"line_number":183,"context_line":"        cell1 \u003d cells[uuidsentinel.cell1]"},{"line_number":184,"context_line":"        cell2 \u003d cells[uuidsentinel.cell2]"},{"line_number":185,"context_line":"        instances \u003d [self._create_instance(ctx, cell\u003dcell1),"},{"line_number":186,"context_line":"                     self._create_instance(ctx, cell\u003dcell2),"},{"line_number":187,"context_line":"                     self._create_instance(ctx, cell\u003dNone)]"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_561eb6ba","line":184,"range":{"start_line":182,"start_character":8,"end_line":184,"end_character":41},"updated":"2017-04-24 17:55:23.000000000","message":"Could just store self.cell1 and self.cell2 in setUp to avoid the extra db call to look them up. But it\u0027s sqlite so meh.","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"aa8280eb1ec6a2230406db28a373573392e184e4","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        cell2 \u003d cells[uuidsentinel.cell2]"},{"line_number":185,"context_line":"        instances \u003d [self._create_instance(ctx, cell\u003dcell1),"},{"line_number":186,"context_line":"                     self._create_instance(ctx, cell\u003dcell2),"},{"line_number":187,"context_line":"                     self._create_instance(ctx, cell\u003dNone)]"},{"line_number":188,"context_line":"        members \u003d [instance.uuid for instance in instances]"},{"line_number":189,"context_line":"        ig_uuid \u003d self._create_instance_group(ctx, members)"},{"line_number":190,"context_line":"        return (ig_uuid, instances, members)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7ffa3b31_236905f8","line":187,"range":{"start_line":187,"start_character":48,"end_line":187,"end_character":57},"updated":"2017-04-21 14:13:56.000000000","message":"thanks for that, it helps verifying the case where instances are just not scheduled yet.","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca887067c6a8865d6f26beb28d6eb66ac019f413","unresolved":false,"context_lines":[{"line_number":336,"context_line":"                                                          instances[1].uuid)"},{"line_number":337,"context_line":"        with context.target_cell(ctx, im.cell_mapping):"},{"line_number":338,"context_line":"            instances[1].destroy()"},{"line_number":339,"context_line":"        im.destroy()"},{"line_number":340,"context_line":"        # check that the instance does not exist"},{"line_number":341,"context_line":"        self.assertRaises(exception.InstanceNotFound,"},{"line_number":342,"context_line":"                          objects.Instance.get_by_uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_76765209","line":339,"range":{"start_line":339,"start_character":8,"end_line":339,"end_character":20},"updated":"2017-04-24 17:55:23.000000000","message":"I\u0027m not sure why you destroy the InstanceMapping, we don\u0027t actually destroy these in runtime code anywhere. In fact, doesn\u0027t this mess up what you\u0027re trying to test because now objects.InstanceMappingList.get_by_instance_uuids in the code won\u0027t return this instance mapping, when what you really want is an instance mapping pointing at a deleted instance. -1 for this.","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6b304da3a6c22fce21d0d9702bbfe54fbc63d33c","unresolved":false,"context_lines":[{"line_number":336,"context_line":"                                                          instances[1].uuid)"},{"line_number":337,"context_line":"        with context.target_cell(ctx, im.cell_mapping):"},{"line_number":338,"context_line":"            instances[1].destroy()"},{"line_number":339,"context_line":"        im.destroy()"},{"line_number":340,"context_line":"        # check that the instance does not exist"},{"line_number":341,"context_line":"        self.assertRaises(exception.InstanceNotFound,"},{"line_number":342,"context_line":"                          objects.Instance.get_by_uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5ff73747_2a8e1a78","line":339,"range":{"start_line":339,"start_character":8,"end_line":339,"end_character":20},"in_reply_to":"5ff73747_76765209","updated":"2017-04-24 18:11:01.000000000","message":"Yeah, I don\u0027t think it\u0027s necessary I was just thinking it made sense in my head, but you\u0027re right.","commit_id":"6a929baee10cebcae5ffe53279e229804e086012"}]}