)]}'
{"nova/objects/instance_group.py":[{"author":{"_account_id":17532,"name":"Tina Kevin","email":"song.ruixia@zte.com.cn","username":"SongRuixia"},"change_message_id":"56e0a50f9476c2a2e7338c5a310a6b842128e4c6","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    query \u003d _instance_group_get_query(context,"},{"line_number":47,"context_line":"                                      id_field\u003dapi_models.InstanceGroup.uuid,"},{"line_number":48,"context_line":"                                      id\u003duuid)"},{"line_number":49,"context_line":"    db_group \u003d query.first()"},{"line_number":50,"context_line":"    if not db_group:"},{"line_number":51,"context_line":"        raise exception.InstanceGroupNotFound(group_uuid\u003duuid)"},{"line_number":52,"context_line":"    return db_group"}],"source_content_type":"text/x-python","patch_set":4,"id":"3aaa91ec_e932390f","line":49,"range":{"start_line":49,"start_character":26,"end_line":49,"end_character":28},"updated":"2016-06-24 04:22:01.000000000","message":"why not use this?\ndb_group \u003d _instance_group_get_query(context,                      id_field\u003dapi_models.InstanceGroup.uuid,id\u003duuid).first()","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"92281fb243ede18e54d9458ec6a37671e6ed1d40","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    query \u003d _instance_group_get_query(context,"},{"line_number":47,"context_line":"                                      id_field\u003dapi_models.InstanceGroup.uuid,"},{"line_number":48,"context_line":"                                      id\u003duuid)"},{"line_number":49,"context_line":"    db_group \u003d query.first()"},{"line_number":50,"context_line":"    if not db_group:"},{"line_number":51,"context_line":"        raise exception.InstanceGroupNotFound(group_uuid\u003duuid)"},{"line_number":52,"context_line":"    return db_group"}],"source_content_type":"text/x-python","patch_set":4,"id":"3aaa91ec_54cde083","line":49,"range":{"start_line":49,"start_character":26,"end_line":49,"end_character":28},"in_reply_to":"3aaa91ec_e932390f","updated":"2016-06-24 04:33:03.000000000","message":"Probably just an oversight. Will update in the next patch set","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":159,"context_line":"    if policies:"},{"line_number":160,"context_line":"        values[\u0027policies\u0027] \u003d policies"},{"line_number":161,"context_line":"    if members:"},{"line_number":162,"context_line":"        values[\u0027members\u0027] \u003d members"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"@db_api.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_1b92a19e","line":162,"updated":"2016-07-19 19:42:15.000000000","message":"Is this to avoid the appearance of having altered the input parameter? Maybe just copy it and avoid doing this?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4876fd439f9cf8d3473ffa1714ee0fa72d18c42d","unresolved":false,"context_lines":[{"line_number":159,"context_line":"    if policies:"},{"line_number":160,"context_line":"        values[\u0027policies\u0027] \u003d policies"},{"line_number":161,"context_line":"    if members:"},{"line_number":162,"context_line":"        values[\u0027members\u0027] \u003d members"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"@db_api.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_a3268ccb","line":162,"in_reply_to":"dada55a8_1b92a19e","updated":"2016-07-26 21:33:02.000000000","message":"The values are removed by pop earlier, I think to avoid them being used in the db_group.update call. The \u0027policies\u0027 and \u0027members\u0027 on the model are properties that return lists. The real fields are \u0027_policies\u0027 and \u0027_members\u0027 which are orm.relationships to the other tables.\n\nSo this is putting them back. We can make a copy instead.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8f06479bd288a24f645aac558c48f60595e2fc86","unresolved":false,"context_lines":[{"line_number":159,"context_line":"    if policies:"},{"line_number":160,"context_line":"        values[\u0027policies\u0027] \u003d policies"},{"line_number":161,"context_line":"    if members:"},{"line_number":162,"context_line":"        values[\u0027members\u0027] \u003d members"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"@db_api.api_context_manager.writer"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_e236b501","line":162,"in_reply_to":"dada55a8_a3268ccb","updated":"2016-07-26 23:14:46.000000000","message":"Right, but it\u0027s putting them back and then not using them (we return here). If we care about not mangling our input parameters, we should copy and mangle the copies, not try to reconstruct them before we return. If we raise on line 144, we will have mangled and not fixed them before leaving this function, for example.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    # We don\u0027t want \u0027_policies\u0027 and \u0027_members\u0027 attributes to be lazy loaded"},{"line_number":180,"context_line":"    # later. We know there is nothing here since we just created this"},{"line_number":181,"context_line":"    # instance group."},{"line_number":182,"context_line":"    if policies:"},{"line_number":183,"context_line":"        _instance_group_policies_add(context, group.id, policies)"},{"line_number":184,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_1bc0e17a","line":181,"updated":"2016-07-19 19:42:15.000000000","message":"I\u0027m not sure I understand this comment. Why do the following things have anything to do with lazy-loading?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4876fd439f9cf8d3473ffa1714ee0fa72d18c42d","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    # We don\u0027t want \u0027_policies\u0027 and \u0027_members\u0027 attributes to be lazy loaded"},{"line_number":180,"context_line":"    # later. We know there is nothing here since we just created this"},{"line_number":181,"context_line":"    # instance group."},{"line_number":182,"context_line":"    if policies:"},{"line_number":183,"context_line":"        _instance_group_policies_add(context, group.id, policies)"},{"line_number":184,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_636ad4ee","line":181,"in_reply_to":"dada55a8_1bc0e17a","updated":"2016-07-26 21:33:02.000000000","message":"The _policies and _members fields do a join on the respective other tables. I\u0027m guessing the intention was to avoid those queries when policies or members are accessed on the object if they\u0027re known to be empty, by setting them to []. But, I don\u0027t think it\u0027s working that way since the model isn\u0027t returned at the end of this function.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8f06479bd288a24f645aac558c48f60595e2fc86","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    # We don\u0027t want \u0027_policies\u0027 and \u0027_members\u0027 attributes to be lazy loaded"},{"line_number":180,"context_line":"    # later. We know there is nothing here since we just created this"},{"line_number":181,"context_line":"    # instance group."},{"line_number":182,"context_line":"    if policies:"},{"line_number":183,"context_line":"        _instance_group_policies_add(context, group.id, policies)"},{"line_number":184,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_420fc138","line":181,"in_reply_to":"dada55a8_636ad4ee","updated":"2016-07-26 23:14:46.000000000","message":"Right, nor is it used in the get call below, so I think this has to be old and no longer relevant.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    if policies:"},{"line_number":183,"context_line":"        _instance_group_policies_add(context, group.id, policies)"},{"line_number":184,"context_line":"    else:"},{"line_number":185,"context_line":"        group._policies \u003d []"},{"line_number":186,"context_line":"    if members:"},{"line_number":187,"context_line":"        _instance_group_members_add(context, group.id, members)"},{"line_number":188,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_fbba2d09","line":185,"updated":"2016-07-19 19:42:15.000000000","message":"This is set, but never used, right?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":188,"context_line":"    else:"},{"line_number":189,"context_line":"        group._members \u003d []"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    return _get_from_db_by_uuid(context, uuid)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"def _instance_group_id(context, group_uuid):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_3b0965de","line":191,"updated":"2016-07-19 19:42:15.000000000","message":"Why do we need to re-do the query here? We already have the group from above.. if we put the two lists on that model and return it then we can avoid doing this query, no?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4876fd439f9cf8d3473ffa1714ee0fa72d18c42d","unresolved":false,"context_lines":[{"line_number":188,"context_line":"    else:"},{"line_number":189,"context_line":"        group._members \u003d []"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    return _get_from_db_by_uuid(context, uuid)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"def _instance_group_id(context, group_uuid):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_69b11301","line":191,"in_reply_to":"dada55a8_3b0965de","updated":"2016-07-26 21:33:02.000000000","message":"The lists on the model are generated from properties, so I don\u0027t think we can put them on the model. I think if policies or members were added, they can be lazy-loaded later on access. Do you suppose this query avoids a potential lazy-load of _policies or _members? I can\u0027t think of why else it would be done.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8f06479bd288a24f645aac558c48f60595e2fc86","unresolved":false,"context_lines":[{"line_number":188,"context_line":"    else:"},{"line_number":189,"context_line":"        group._members \u003d []"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    return _get_from_db_by_uuid(context, uuid)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"def _instance_group_id(context, group_uuid):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_e504ff15","line":191,"in_reply_to":"dada55a8_69b11301","updated":"2016-07-26 23:14:46.000000000","message":"Yes, but I would think we could just slap them on the model before we return instead of re-doing the query...","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"@db_api.api_context_manager.writer"},{"line_number":204,"context_line":"def _destroy_in_db(context, group_uuid):"},{"line_number":205,"context_line":"    group_id \u003d _instance_group_id(context, group_uuid)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    query \u003d _instance_group_get_query(context,"},{"line_number":208,"context_line":"                                      id_field\u003dapi_models.InstanceGroup.uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_7bb8fde5","line":205,"updated":"2016-07-19 19:42:15.000000000","message":"This is really wasteful, right? Why don\u0027t we just look it up by uuid and use that for our .delete?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4876fd439f9cf8d3473ffa1714ee0fa72d18c42d","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"@db_api.api_context_manager.writer"},{"line_number":204,"context_line":"def _destroy_in_db(context, group_uuid):"},{"line_number":205,"context_line":"    group_id \u003d _instance_group_id(context, group_uuid)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    query \u003d _instance_group_get_query(context,"},{"line_number":208,"context_line":"                                      id_field\u003dapi_models.InstanceGroup.uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_d81a876e","line":205,"in_reply_to":"dada55a8_7bb8fde5","updated":"2016-07-26 21:33:02.000000000","message":"Yeah, I think this can be changed","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    query \u003d _instance_group_get_query(context,"},{"line_number":208,"context_line":"                                      id_field\u003dapi_models.InstanceGroup.uuid,"},{"line_number":209,"context_line":"                                      id\u003dgroup_uuid)"},{"line_number":210,"context_line":"    if query.count() \u003d\u003d 0:"},{"line_number":211,"context_line":"        raise exception.InstanceGroupNotFound(group_uuid\u003dgroup_uuid)"},{"line_number":212,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_db96494b","line":209,"updated":"2016-07-19 19:42:15.000000000","message":"Well, this is the wasteful thing. If we returned the group from L205, couldn\u0027t we avoid this extra query?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":222,"context_line":"@db_api.api_context_manager.writer"},{"line_number":223,"context_line":"def _add_members_in_db(context, group_uuid, members):"},{"line_number":224,"context_line":"    id \u003d _instance_group_id(context, group_uuid)"},{"line_number":225,"context_line":"    return _instance_group_members_add(context, id, members)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"# TODO(berrange): Remove NovaObjectDictCompat"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_bb91d554","line":225,"updated":"2016-07-19 19:42:15.000000000","message":"Same here.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4876fd439f9cf8d3473ffa1714ee0fa72d18c42d","unresolved":false,"context_lines":[{"line_number":222,"context_line":"@db_api.api_context_manager.writer"},{"line_number":223,"context_line":"def _add_members_in_db(context, group_uuid, members):"},{"line_number":224,"context_line":"    id \u003d _instance_group_id(context, group_uuid)"},{"line_number":225,"context_line":"    return _instance_group_members_add(context, id, members)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"# TODO(berrange): Remove NovaObjectDictCompat"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_f852eb11","line":225,"in_reply_to":"dada55a8_bb91d554","updated":"2016-07-26 21:33:02.000000000","message":"I think this one needs to stay as it is because _instance_group_members_add queries the instance_group_member table which has only a group_id field to link it to instance_groups. So we need the group.id","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8f06479bd288a24f645aac558c48f60595e2fc86","unresolved":false,"context_lines":[{"line_number":222,"context_line":"@db_api.api_context_manager.writer"},{"line_number":223,"context_line":"def _add_members_in_db(context, group_uuid, members):"},{"line_number":224,"context_line":"    id \u003d _instance_group_id(context, group_uuid)"},{"line_number":225,"context_line":"    return _instance_group_members_add(context, id, members)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"# TODO(berrange): Remove NovaObjectDictCompat"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_0575d396","line":225,"in_reply_to":"dada55a8_f852eb11","updated":"2016-07-26 23:14:46.000000000","message":"I think you can update by uuid by joining them and using uuid. But I guess one could argue against deviating too much from what we have in place now and optimize later. Just seems kinda ugly.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    @staticmethod"},{"line_number":304,"context_line":"    def _get_from_db_by_uuid(context, uuid):"},{"line_number":305,"context_line":"        return _get_from_db_by_uuid(context, uuid)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @staticmethod"},{"line_number":308,"context_line":"    def _get_from_db_by_id(context, id):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_5bb7f9d7","line":305,"updated":"2016-07-19 19:42:15.000000000","message":"We don\u0027t need this indirection anymore. You can bring all the functions above into methods here. We had to do this to work around an oslo.db bug, but that is no longer the case.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"805305be89572a91eb0d3741fb2d5b3f9e0082e8","unresolved":false,"context_lines":[{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    @staticmethod"},{"line_number":304,"context_line":"    def _get_from_db_by_uuid(context, uuid):"},{"line_number":305,"context_line":"        return _get_from_db_by_uuid(context, uuid)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @staticmethod"},{"line_number":308,"context_line":"    def _get_from_db_by_id(context, id):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_6f7921c8","line":305,"in_reply_to":"dada55a8_5bb7f9d7","updated":"2016-07-20 19:29:59.000000000","message":"Ah, thanks","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            pass"},{"line_number":352,"context_line":"        if db_group is None:"},{"line_number":353,"context_line":"            db_group \u003d db.instance_group_get(context, uuid)"},{"line_number":354,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"    @base.remotable_classmethod"},{"line_number":357,"context_line":"    def get_by_name(cls, context, name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_9be851b6","line":354,"updated":"2016-07-19 19:42:15.000000000","message":"Won\u0027t this crash if db_group from the old db (i.e. db.instance_group_get()) is None?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"269aa99b95e177ad245e56e05f4665af21b9c14c","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            pass"},{"line_number":352,"context_line":"        if db_group is None:"},{"line_number":353,"context_line":"            db_group \u003d db.instance_group_get(context, uuid)"},{"line_number":354,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"    @base.remotable_classmethod"},{"line_number":357,"context_line":"    def get_by_name(cls, context, name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_804d7f74","line":354,"in_reply_to":"dada55a8_4a39abd6","updated":"2016-07-25 18:02:30.000000000","message":"Yeah, I think you should.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1535675325f2dcb871617643766a31643a9b7998","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            pass"},{"line_number":352,"context_line":"        if db_group is None:"},{"line_number":353,"context_line":"            db_group \u003d db.instance_group_get(context, uuid)"},{"line_number":354,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"    @base.remotable_classmethod"},{"line_number":357,"context_line":"    def get_by_name(cls, context, name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_1861bf39","line":354,"in_reply_to":"dada55a8_804d7f74","updated":"2016-07-26 21:39:39.000000000","message":"Okay, I just checked again and found db.instance_group_get raises InstanceGroupNotFound already. That\u0027s why it\u0027s not done separately here.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"805305be89572a91eb0d3741fb2d5b3f9e0082e8","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            pass"},{"line_number":352,"context_line":"        if db_group is None:"},{"line_number":353,"context_line":"            db_group \u003d db.instance_group_get(context, uuid)"},{"line_number":354,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"    @base.remotable_classmethod"},{"line_number":357,"context_line":"    def get_by_name(cls, context, name):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_4a39abd6","line":354,"in_reply_to":"dada55a8_9be851b6","updated":"2016-07-20 19:29:59.000000000","message":"I guess so. I didn\u0027t think this is different than the current behavior before this patch, is it? Shall I add raise InstanceGroupNotFound before doing this if it\u0027s still None?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":385,"context_line":"        if db_group is None:"},{"line_number":386,"context_line":"            db_group \u003d db.instance_group_get_by_instance(context,"},{"line_number":387,"context_line":"                                                         instance_uuid)"},{"line_number":388,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    @classmethod"},{"line_number":391,"context_line":"    def get_by_hint(cls, context, hint):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_bb50b55f","line":388,"updated":"2016-07-19 19:42:15.000000000","message":"Same here.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1535675325f2dcb871617643766a31643a9b7998","unresolved":false,"context_lines":[{"line_number":385,"context_line":"        if db_group is None:"},{"line_number":386,"context_line":"            db_group \u003d db.instance_group_get_by_instance(context,"},{"line_number":387,"context_line":"                                                         instance_uuid)"},{"line_number":388,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    @classmethod"},{"line_number":391,"context_line":"    def get_by_hint(cls, context, hint):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_78527b68","line":388,"in_reply_to":"dada55a8_2171fd76","updated":"2016-07-26 21:39:39.000000000","message":"db.instance_group_get_by_instance raises InstanceGroupNotFound","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"805305be89572a91eb0d3741fb2d5b3f9e0082e8","unresolved":false,"context_lines":[{"line_number":385,"context_line":"        if db_group is None:"},{"line_number":386,"context_line":"            db_group \u003d db.instance_group_get_by_instance(context,"},{"line_number":387,"context_line":"                                                         instance_uuid)"},{"line_number":388,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    @classmethod"},{"line_number":391,"context_line":"    def get_by_hint(cls, context, hint):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_2171fd76","line":388,"in_reply_to":"dada55a8_bb50b55f","updated":"2016-07-20 19:29:59.000000000","message":"Same response here, should I add a raise of InstanceGroupNotFound if still None?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5205f531e27521b0fbf6d374f2a5389337271f4d","unresolved":false,"context_lines":[{"line_number":423,"context_line":"            db_group \u003d self._get_from_db_by_uuid(self._context, self.uuid)"},{"line_number":424,"context_line":"        else:"},{"line_number":425,"context_line":"            db.instance_group_update(self._context, self.uuid, updates)"},{"line_number":426,"context_line":"            db_group \u003d db.instance_group_get(self._context, self.uuid)"},{"line_number":427,"context_line":"        self._from_db_object(self._context, self, db_group)"},{"line_number":428,"context_line":"        compute_utils.notify_about_server_group_update(self._context,"},{"line_number":429,"context_line":"                                                       \"update\", payload)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_bb7395e6","line":426,"updated":"2016-07-19 19:42:15.000000000","message":"So just to be clear, we\u0027re continuing to update the groups in the old database here...\n\nWe could race with our query and a migration command I think. What if we just try to update in the api database, and if it fails with NotFound, we update in the old db?","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"805305be89572a91eb0d3741fb2d5b3f9e0082e8","unresolved":false,"context_lines":[{"line_number":423,"context_line":"            db_group \u003d self._get_from_db_by_uuid(self._context, self.uuid)"},{"line_number":424,"context_line":"        else:"},{"line_number":425,"context_line":"            db.instance_group_update(self._context, self.uuid, updates)"},{"line_number":426,"context_line":"            db_group \u003d db.instance_group_get(self._context, self.uuid)"},{"line_number":427,"context_line":"        self._from_db_object(self._context, self, db_group)"},{"line_number":428,"context_line":"        compute_utils.notify_about_server_group_update(self._context,"},{"line_number":429,"context_line":"                                                       \"update\", payload)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_41ccf169","line":426,"in_reply_to":"dada55a8_bb7395e6","updated":"2016-07-20 19:29:59.000000000","message":"Yes, from what I understand we want to update records where they happen to be and migrations are via the \u0027online data migrations\u0027 from nova-manage only.\n\nWhat you said makes sense, I\u0027ll change it","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bd199fb6b692ce0ac211edd43a2d6fa70a81be05","unresolved":false,"context_lines":[{"line_number":99,"context_line":"            join(\u0027_members\u0027).\\"},{"line_number":100,"context_line":"            filter(api_models.InstanceGroup.uuid \u003d\u003d group_uuid).\\"},{"line_number":101,"context_line":"            options(contains_eager(\u0027_members\u0027)).\\"},{"line_number":102,"context_line":"            first()"},{"line_number":103,"context_line":"    if not group:"},{"line_number":104,"context_line":"        raise exception.InstanceGroupNotFound(group_uuid\u003dgroup_uuid)"},{"line_number":105,"context_line":"    already_existing \u003d set()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_b3ec6d2a","line":102,"updated":"2016-07-29 00:56:10.000000000","message":"I\u0027m not sure if I did this right for getting the group and members in one query. The original code at L79 filters the members as well to only members that are part of the set of members passed in. I wasn\u0027t sure if/how to do that here.","commit_id":"604d19d2e1797baeab888f29c66d8cf1f3368007"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2f18064dda1eb00ec20ffd1e5fe499c6a23ba8","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                           \u0027group_id\u0027: group.id})"},{"line_number":118,"context_line":"        context.session.add(member_ref)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    return members"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"# TODO(berrange): Remove NovaObjectDictCompat"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_5c2a6b91","line":120,"updated":"2016-08-03 18:49:47.000000000","message":"Is there a reason this method returns members directly while the method above appends to a member_models list?","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ffb0f1edbf7b57796c42821caf6c46175cd4495d","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                           \u0027group_id\u0027: group.id})"},{"line_number":118,"context_line":"        context.session.add(member_ref)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    return members"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"# TODO(berrange): Remove NovaObjectDictCompat"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_a2fc1d3f","line":120,"in_reply_to":"bacf61ea_5c2a6b91","updated":"2016-08-03 20:50:23.000000000","message":"Yes, _instance_group_members_add_by_uuid is called only by add_members which needs to return just the member names. I considered having this return a list of models and then do a list comprehension with the names in _add_members_in_db but thought maybe that\u0027s needless. If you think it would be better to return models and create the list of names for add_members, I don\u0027t mind doing it.","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"b33d120763dad60eec39a627173ad04992d1829a","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                           \u0027group_id\u0027: group.id})"},{"line_number":118,"context_line":"        context.session.add(member_ref)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    return members"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"# TODO(berrange): Remove NovaObjectDictCompat"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_2cd63b4f","line":120,"in_reply_to":"bacf61ea_a2fc1d3f","updated":"2016-08-04 13:52:23.000000000","message":"I\u0027m a sucker for consistency when possible so I\u0027m for changing it. But I don\u0027t want this to get held up over that.","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2f18064dda1eb00ec20ffd1e5fe499c6a23ba8","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                continue"},{"line_number":171,"context_line":"            ignore \u003d {\u0027deleted\u0027: False,"},{"line_number":172,"context_line":"                      \u0027deleted_at\u0027: None}"},{"line_number":173,"context_line":"            if field in ignore and not hasattr(db_inst, field):"},{"line_number":174,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":175,"context_line":"            else:"},{"line_number":176,"context_line":"                instance_group[field] \u003d db_inst[field]"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_7ce76f40","line":173,"updated":"2016-08-03 18:49:47.000000000","message":"Nit: This could use a comment. It\u0027s clear why this is if you\u0027ve dealt with cell-\u003eapi migrations before and the lack of soft-delete in the api db, but may be confusing to casual viewers.","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ffb0f1edbf7b57796c42821caf6c46175cd4495d","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                continue"},{"line_number":171,"context_line":"            ignore \u003d {\u0027deleted\u0027: False,"},{"line_number":172,"context_line":"                      \u0027deleted_at\u0027: None}"},{"line_number":173,"context_line":"            if field in ignore and not hasattr(db_inst, field):"},{"line_number":174,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":175,"context_line":"            else:"},{"line_number":176,"context_line":"                instance_group[field] \u003d db_inst[field]"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_42aba11e","line":173,"in_reply_to":"bacf61ea_7ce76f40","updated":"2016-08-03 20:50:23.000000000","message":"Sure, I can add one","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2f18064dda1eb00ec20ffd1e5fe499c6a23ba8","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                                        id_field\u003dapi_models.InstanceGroup.id,"},{"line_number":197,"context_line":"                                        id\u003did).first()"},{"line_number":198,"context_line":"        if not grp:"},{"line_number":199,"context_line":"            raise exception.InstanceGroupNotFound(group_uuid\u003did)"},{"line_number":200,"context_line":"        return grp"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_dc001b69","line":199,"range":{"start_line":199,"start_character":61,"end_line":199,"end_character":63},"updated":"2016-08-03 18:49:47.000000000","message":"This has always bugged me about how our exception messages are done, because this isn\u0027t accurate. However it does match what the current cell db code does.","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ffb0f1edbf7b57796c42821caf6c46175cd4495d","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                                        id_field\u003dapi_models.InstanceGroup.id,"},{"line_number":197,"context_line":"                                        id\u003did).first()"},{"line_number":198,"context_line":"        if not grp:"},{"line_number":199,"context_line":"            raise exception.InstanceGroupNotFound(group_uuid\u003did)"},{"line_number":200,"context_line":"        return grp"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_023ec9d9","line":199,"range":{"start_line":199,"start_character":61,"end_line":199,"end_character":63},"in_reply_to":"bacf61ea_dc001b69","updated":"2016-08-03 20:50:23.000000000","message":"I considered that but the exception message seems intentionally vague enough that it\u0027s technically correct whether it\u0027s a uuid, an id, or a name.\n\n \"Instance group %(group_uuid)s could not be found.\"","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2f18064dda1eb00ec20ffd1e5fe499c6a23ba8","unresolved":false,"context_lines":[{"line_number":322,"context_line":"            pass"},{"line_number":323,"context_line":"        if db_group is None:"},{"line_number":324,"context_line":"            # TODO(russellb) We need to get the group by name here.  There\u0027s no"},{"line_number":325,"context_line":"            # db.api method for this yet.  Come back and optimize this by"},{"line_number":326,"context_line":"            # adding a new query by name.  This is unnecessarily expensive if a"},{"line_number":327,"context_line":"            # tenant has lots of groups."},{"line_number":328,"context_line":"            igs \u003d db.instance_group_get_all_by_project_id(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_d7a75409","line":325,"range":{"start_line":325,"start_character":43,"end_line":325,"end_character":70},"updated":"2016-08-03 18:49:47.000000000","message":"Hurray, you\u0027ve optimized this.","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ffb0f1edbf7b57796c42821caf6c46175cd4495d","unresolved":false,"context_lines":[{"line_number":322,"context_line":"            pass"},{"line_number":323,"context_line":"        if db_group is None:"},{"line_number":324,"context_line":"            # TODO(russellb) We need to get the group by name here.  There\u0027s no"},{"line_number":325,"context_line":"            # db.api method for this yet.  Come back and optimize this by"},{"line_number":326,"context_line":"            # adding a new query by name.  This is unnecessarily expensive if a"},{"line_number":327,"context_line":"            # tenant has lots of groups."},{"line_number":328,"context_line":"            igs \u003d db.instance_group_get_all_by_project_id(context,"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_82011909","line":325,"range":{"start_line":325,"start_character":43,"end_line":325,"end_character":70},"in_reply_to":"bacf61ea_d7a75409","updated":"2016-08-03 20:50:23.000000000","message":"Yeah! /fistpump","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"1a2f18064dda1eb00ec20ffd1e5fe499c6a23ba8","unresolved":false,"context_lines":[{"line_number":379,"context_line":"        payload[\u0027server_group_id\u0027] \u003d self.uuid"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        try:"},{"line_number":382,"context_line":"            self._save_in_db(self._context, self.uuid, updates)"},{"line_number":383,"context_line":"            db_group \u003d self._get_from_db_by_uuid(self._context, self.uuid)"},{"line_number":384,"context_line":"        except exception.InstanceGroupNotFound:"},{"line_number":385,"context_line":"            db.instance_group_update(self._context, self.uuid, updates)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_d7cc74bd","line":382,"updated":"2016-08-03 18:49:47.000000000","message":"This looks like there could be a small optimization by having _save_in_db return the group. It\u0027s hard to tell if the group it returned would match what _get_from_db_by_uuid returns though.","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ffb0f1edbf7b57796c42821caf6c46175cd4495d","unresolved":false,"context_lines":[{"line_number":379,"context_line":"        payload[\u0027server_group_id\u0027] \u003d self.uuid"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        try:"},{"line_number":382,"context_line":"            self._save_in_db(self._context, self.uuid, updates)"},{"line_number":383,"context_line":"            db_group \u003d self._get_from_db_by_uuid(self._context, self.uuid)"},{"line_number":384,"context_line":"        except exception.InstanceGroupNotFound:"},{"line_number":385,"context_line":"            db.instance_group_update(self._context, self.uuid, updates)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bacf61ea_fd599636","line":382,"in_reply_to":"bacf61ea_d7cc74bd","updated":"2016-08-03 20:50:23.000000000","message":"I wondered that too but kept with the existing pattern here. I\u0027ll see what I can find out about it.","commit_id":"a8645ad2970352f756bd548f123a8ac7252f5ee9"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"6b3cf15bb86a1bcb1400fc100a26a903605e886a","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    return member_models"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"def _instance_group_members_add_by_uuid(context, group_uuid, members):"},{"line_number":104,"context_line":"    all_members \u003d set(members)"},{"line_number":105,"context_line":"    # NOTE(melwitt): The condition on the join limits the number of members"},{"line_number":106,"context_line":"    # returned to only those we wish to check as already existing."}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_05b3219b","line":103,"updated":"2016-08-05 08:43:38.000000000","message":"Do you think it\u0027s possible to make some abstraction for these tree methods\n    like _merge_related_objects, maybe? It\u0027s too similar IMHO.","commit_id":"e00a81edfbe460ffc3e9f33f781e7eaa7987203f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"96de228d1f833662fab22ff7fe76bc0690771358","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    return member_models"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"def _instance_group_members_add_by_uuid(context, group_uuid, members):"},{"line_number":104,"context_line":"    all_members \u003d set(members)"},{"line_number":105,"context_line":"    # NOTE(melwitt): The condition on the join limits the number of members"},{"line_number":106,"context_line":"    # returned to only those we wish to check as already existing."}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_07cf8d56","line":103,"in_reply_to":"bacf61ea_05b3219b","updated":"2016-08-05 16:01:59.000000000","message":"Good point, I\u0027ll try to commonize these","commit_id":"e00a81edfbe460ffc3e9f33f781e7eaa7987203f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"161f876f260100f53b8083511f5b5ee2f749a4ae","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    @db_api.api_context_manager.writer"},{"line_number":305,"context_line":"    def _add_members_in_db(context, group_uuid, members):"},{"line_number":306,"context_line":"        return _instance_group_members_add_by_uuid("},{"line_number":307,"context_line":"                context, group_uuid, members)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def obj_load_attr(self, attrname):"},{"line_number":310,"context_line":"        # NOTE(sbauza): Only hosts could be lazy-loaded right now"}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_3155cb4f","line":307,"updated":"2016-08-05 06:56:44.000000000","message":"Bah, this indentation isn\u0027t like the others!","commit_id":"e00a81edfbe460ffc3e9f33f781e7eaa7987203f"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"6b3cf15bb86a1bcb1400fc100a26a903605e886a","unresolved":false,"context_lines":[{"line_number":341,"context_line":"            igs \u003d db.instance_group_get_all_by_project_id(context,"},{"line_number":342,"context_line":"                                                          context.project_id)"},{"line_number":343,"context_line":"            for ig in igs:"},{"line_number":344,"context_line":"                if ig[\u0027name\u0027] \u003d\u003d name:"},{"line_number":345,"context_line":"                    db_group \u003d ig"},{"line_number":346,"context_line":"                    break"},{"line_number":347,"context_line":"            if db_group is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_a0520311","line":344,"updated":"2016-08-05 08:43:38.000000000","message":"Uh, really weird thing.","commit_id":"e00a81edfbe460ffc3e9f33f781e7eaa7987203f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"96de228d1f833662fab22ff7fe76bc0690771358","unresolved":false,"context_lines":[{"line_number":341,"context_line":"            igs \u003d db.instance_group_get_all_by_project_id(context,"},{"line_number":342,"context_line":"                                                          context.project_id)"},{"line_number":343,"context_line":"            for ig in igs:"},{"line_number":344,"context_line":"                if ig[\u0027name\u0027] \u003d\u003d name:"},{"line_number":345,"context_line":"                    db_group \u003d ig"},{"line_number":346,"context_line":"                    break"},{"line_number":347,"context_line":"            if db_group is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_67c3e938","line":344,"in_reply_to":"bacf61ea_09c7a996","updated":"2016-08-05 16:01:59.000000000","message":"db.instance_group_get_all_by_project_id returns sqlalchemy model objects which do support attribute access, but whenever we fake them in unit tests, we make them dicts. I think that\u0027s what drives the dict-like access in a lot of places.","commit_id":"e00a81edfbe460ffc3e9f33f781e7eaa7987203f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"189d2f536d8af7dce63a73b13dd8f18ae33cdc0b","unresolved":false,"context_lines":[{"line_number":341,"context_line":"            igs \u003d db.instance_group_get_all_by_project_id(context,"},{"line_number":342,"context_line":"                                                          context.project_id)"},{"line_number":343,"context_line":"            for ig in igs:"},{"line_number":344,"context_line":"                if ig[\u0027name\u0027] \u003d\u003d name:"},{"line_number":345,"context_line":"                    db_group \u003d ig"},{"line_number":346,"context_line":"                    break"},{"line_number":347,"context_line":"            if db_group is None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bacf61ea_09c7a996","line":344,"in_reply_to":"bacf61ea_a0520311","updated":"2016-08-05 14:38:31.000000000","message":"This is because objects.InstanceGroupList.get_by_project_id that was used before returned a list of InstanceGroup objects so .name could be used to get the name field. However that method would now pull from the api db and we need to query the cell db here. So db.instance_group_get_all_by_project_id is used which returns a dict like object so that\u0027s why the access pattern changed.","commit_id":"e00a81edfbe460ffc3e9f33f781e7eaa7987203f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d1a112425c849fd5e8047087b9c21007352ea237","unresolved":false,"context_lines":[{"line_number":322,"context_line":"                    db_group \u003d ig"},{"line_number":323,"context_line":"                    break"},{"line_number":324,"context_line":"            if db_group is None:"},{"line_number":325,"context_line":"                raise exception.InstanceGroupNotFound(group_uuid\u003dname)"},{"line_number":326,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    @base.remotable_classmethod"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ac06dbe_4d6dd603","line":325,"updated":"2016-08-19 17:10:08.000000000","message":"It\u0027s kinda messy to reimplement this all here. Can you just break out this from InstanceGroupList and call it? I assume that you did this since the view is now unified from the list. We can just make a non-remotable method on the list and call it here since we\u0027ll already be on the conductor side if we\u0027re doing this.","commit_id":"5ec93655531112631f6652cd5a6daaf10b42f334"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dfccc1f56c56ae6e149acb8eb1447e2a0465546b","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        return cls._from_db_object(context, cls(), db_group)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    @classmethod"},{"line_number":341,"context_line":"    def get_by_hint(cls, context, hint):"},{"line_number":342,"context_line":"        if uuidutils.is_uuid_like(hint):"},{"line_number":343,"context_line":"            return cls.get_by_uuid(context, hint)"},{"line_number":344,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ac06dbe_08058cc1","line":341,"updated":"2016-08-19 17:20:18.000000000","message":"Note that this could return a group from the api db when the user meant a group in the main db, if they used name as a hint. That will be bad because you could be requesting anti-affinity in a group and not get what you wanted.\n\nHowever, Iooked and we don\u0027t actually have a unique constraint on name, so this is probably hittable in a single db now anyway :(","commit_id":"5ec93655531112631f6652cd5a6daaf10b42f334"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d1a112425c849fd5e8047087b9c21007352ea237","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        policies \u003d updates.pop(\u0027policies\u0027, None)"},{"line_number":398,"context_line":"        members \u003d updates.pop(\u0027members\u0027, None)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        db_group \u003d self._create_in_db(self._context, updates,"},{"line_number":401,"context_line":"                                      policies\u003dpolicies,"},{"line_number":402,"context_line":"                                      members\u003dmembers)"},{"line_number":403,"context_line":"        self._from_db_object(self._context, self, db_group)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ac06dbe_eda5a2b5","line":400,"updated":"2016-08-19 17:10:08.000000000","message":"So I wonder if we should be extra diligent here and check that this group doesn\u0027t exist in the main db before we create in the api db. It would have to be a uuid collision to matter, but.. the penalty for not enforcing uniqueness would be things co-located that shouldn\u0027t be, etc.","commit_id":"5ec93655531112631f6652cd5a6daaf10b42f334"}],"nova/tests/unit/objects/test_instance_group.py":[{"author":{"_account_id":17532,"name":"Tina Kevin","email":"song.ruixia@zte.com.cn","username":"SongRuixia"},"change_message_id":"f88afffcb054d3b3c67593966c2dab90e4547a15","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    @mock.patch(\u0027nova.objects.InstanceGroup._get_from_db_by_uuid\u0027)"},{"line_number":49,"context_line":"    def test_get_by_uuid_main(self, mock_api_get, mock_db_get):"},{"line_number":50,"context_line":"        error \u003d exception.InstanceGroupNotFound(group_uuid\u003d_DB_UUID)"},{"line_number":51,"context_line":"        mock_api_get.side_effect \u003d error"},{"line_number":52,"context_line":"        obj \u003d objects.InstanceGroup.get_by_uuid(self.context,"},{"line_number":53,"context_line":"                                                       _DB_UUID)"},{"line_number":54,"context_line":"        mock_db_get.assert_called_once_with(self.context, _DB_UUID)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3aaa91ec_95bf42a9","line":51,"range":{"start_line":51,"start_character":35,"end_line":51,"end_character":40},"updated":"2016-06-24 06:29:43.000000000","message":"@mock.patch(\u0027nova.objects.InstanceGroup._get_from_db_by_uuid\u0027,\n       side_effect\u003dexception.InstanceGroupNotFound(group_uuid\u003d_DB_UUID)\ndef test_get_by_uuid_main(self, mock_api_get, mock_db_get):\n\nThis is more clear.","commit_id":"7fcf988a29f58b24f15f30f27cabfbc6ed4d6758"}]}