)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9a8ebc31586425e71c1096133689a9d5a4e72b2a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yikun Jiang \u003cyikunkero@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-04-21 15:20:34 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add policy to InstacenGroup object and api models."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We add the \"rules\" field to policy object, and the old"},{"line_number":10,"context_line":"InstanceGroup object needs to be adapted to support new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9f6a8fd7_5955d2f8","line":7,"range":{"start_line":7,"start_character":19,"end_line":7,"end_character":22},"updated":"2018-04-25 16:47:24.000000000","message":"nce","commit_id":"126916fb7bf9eba7487a450b587436b8f33c6967"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"06ffe151de4b7478168ddc7974a6e0bda3027100","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yikun Jiang \u003cyikunkero@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-04-21 15:20:34 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add policy to InstacenGroup object and api models."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We add the \"rules\" field to policy object, and the old"},{"line_number":10,"context_line":"InstanceGroup object needs to be adapted to support new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_49ade32f","line":7,"range":{"start_line":7,"start_character":19,"end_line":7,"end_character":22},"in_reply_to":"9f6a8fd7_5955d2f8","updated":"2018-05-17 13:32:55.000000000","message":"Done","commit_id":"126916fb7bf9eba7487a450b587436b8f33c6967"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9a8ebc31586425e71c1096133689a9d5a4e72b2a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"The change items in this patch as below:"},{"line_number":14,"context_line":"1. Change \"_policies\" to \"policy\" in api_models.InstanceGroup"},{"line_number":15,"context_line":"2. Add a new InstanceGroupPolicy object."},{"line_number":16,"context_line":"3. Add policy(InstanceGroupPolicy) to InstacenGroup, and adapt"},{"line_number":17,"context_line":"obj method to new object which contains the new policy field."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Related to blueprint complex-anti-affinity-policies"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9f6a8fd7_7958d62f","line":16,"range":{"start_line":16,"start_character":43,"end_line":16,"end_character":46},"updated":"2018-04-25 16:47:24.000000000","message":"nce","commit_id":"126916fb7bf9eba7487a450b587436b8f33c6967"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"06ffe151de4b7478168ddc7974a6e0bda3027100","unresolved":false,"context_lines":[{"line_number":13,"context_line":"The change items in this patch as below:"},{"line_number":14,"context_line":"1. Change \"_policies\" to \"policy\" in api_models.InstanceGroup"},{"line_number":15,"context_line":"2. Add a new InstanceGroupPolicy object."},{"line_number":16,"context_line":"3. Add policy(InstanceGroupPolicy) to InstacenGroup, and adapt"},{"line_number":17,"context_line":"obj method to new object which contains the new policy field."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Related to blueprint complex-anti-affinity-policies"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_e9a94f24","line":16,"range":{"start_line":16,"start_character":43,"end_line":16,"end_character":46},"in_reply_to":"9f6a8fd7_7958d62f","updated":"2018-05-17 13:32:55.000000000","message":"Done","commit_id":"126916fb7bf9eba7487a450b587436b8f33c6967"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yikun Jiang \u003cyikunkero@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-05 09:43:33 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add policy to InstanceGroup object and api models."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We add the \"rules\" field to policy object, and the old"},{"line_number":10,"context_line":"InstanceGroup object needs to be adapted to support new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"5f7c97a3_c3485eb5","line":7,"range":{"start_line":7,"start_character":35,"end_line":7,"end_character":49},"updated":"2018-07-05 15:30:23.000000000","message":"You can remove this since it was split out into a separate change.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yikun Jiang \u003cyikunkero@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-05 09:43:33 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add policy to InstanceGroup object and api models."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We add the \"rules\" field to policy object, and the old"},{"line_number":10,"context_line":"InstanceGroup object needs to be adapted to support new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"5f7c97a3_0c88b568","line":7,"range":{"start_line":7,"start_character":35,"end_line":7,"end_character":49},"in_reply_to":"5f7c97a3_c3485eb5","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":11,"context_line":"policy object."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The change items in this patch as below:"},{"line_number":14,"context_line":"1. Change \"_policies\" to \"policy\" in api_models.InstanceGroup."},{"line_number":15,"context_line":"2. Add policy(InstanceGroupPolicy) to InstanceGroup, deprecate"},{"line_number":16,"context_line":"the policies field."},{"line_number":17,"context_line":"3. Adapt obj method to new object which contains the new policy"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"5f7c97a3_a3c9a225","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":62},"updated":"2018-07-05 15:30:23.000000000","message":"This was split out.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":11,"context_line":"policy object."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The change items in this patch as below:"},{"line_number":14,"context_line":"1. Change \"_policies\" to \"policy\" in api_models.InstanceGroup."},{"line_number":15,"context_line":"2. Add policy(InstanceGroupPolicy) to InstanceGroup, deprecate"},{"line_number":16,"context_line":"the policies field."},{"line_number":17,"context_line":"3. Adapt obj method to new object which contains the new policy"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"5f7c97a3_ec4d393c","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":62},"in_reply_to":"5f7c97a3_a3c9a225","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":14,"context_line":"1. Change \"_policies\" to \"policy\" in api_models.InstanceGroup."},{"line_number":15,"context_line":"2. Add policy(InstanceGroupPolicy) to InstanceGroup, deprecate"},{"line_number":16,"context_line":"the policies field."},{"line_number":17,"context_line":"3. Adapt obj method to new object which contains the new policy"},{"line_number":18,"context_line":"field."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"5f7c97a3_c3ddbede","line":18,"range":{"start_line":17,"start_character":3,"end_line":18,"end_character":6},"updated":"2018-07-05 15:30:23.000000000","message":"I\u0027m a bit unclear on what this means. Do you mean something like, InstanceGroup.create() will now use InstanceGroupPolicy.create() rather than creating policy records directly in the database using the DB model?","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":14,"context_line":"1. Change \"_policies\" to \"policy\" in api_models.InstanceGroup."},{"line_number":15,"context_line":"2. Add policy(InstanceGroupPolicy) to InstanceGroup, deprecate"},{"line_number":16,"context_line":"the policies field."},{"line_number":17,"context_line":"3. Adapt obj method to new object which contains the new policy"},{"line_number":18,"context_line":"field."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":28,"id":"5f7c97a3_cc395da3","line":18,"range":{"start_line":17,"start_character":3,"end_line":18,"end_character":6},"in_reply_to":"5f7c97a3_c3ddbede","updated":"2018-07-06 12:18:27.000000000","message":"I mean we add the support on dealing new style policy in _create_in_db method and _from_db_object.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0da9560b388a5aa5e01371fc7269049953803fd3","unresolved":false,"context_lines":[{"line_number":10,"context_line":"1. Add policy(InstanceGroupPolicy) to InstanceGroup, deprecate"},{"line_number":11,"context_line":"the policies field."},{"line_number":12,"context_line":"2. Also, make _from_db_object and _create_in_db method to"},{"line_number":13,"context_line":"support to deal new \"policy\" field."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_79cfa827","line":13,"range":{"start_line":13,"start_character":8,"end_line":13,"end_character":15},"updated":"2018-07-06 13:49:40.000000000","message":"nit: this could be removed","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"bca8897675ca35ea78b3c1b653b40f94d38f5c05","unresolved":false,"context_lines":[{"line_number":10,"context_line":"1. Add policy(InstanceGroupPolicy) to InstanceGroup, deprecate"},{"line_number":11,"context_line":"the policies field."},{"line_number":12,"context_line":"2. Also, make _from_db_object and _create_in_db method to"},{"line_number":13,"context_line":"support to deal new \"policy\" field."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_db381e3c","line":13,"range":{"start_line":13,"start_character":8,"end_line":13,"end_character":15},"in_reply_to":"5f7c97a3_79cfa827","updated":"2018-07-09 07:29:47.000000000","message":"Done","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"82fbf7d2b2c03aeeb66fcb4cd9e5fce2851b7b3e","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the policies field."},{"line_number":12,"context_line":"2. Also, make _from_db_object and _create_in_db method to"},{"line_number":13,"context_line":"support new \"policy\" field."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ib33719a4b9599d86848c618a6e142c71ece79ca5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"5f7c97a3_5a871e5c","line":14,"updated":"2018-07-09 14:04:18.000000000","message":"I would also mention that internal usage of InstanceGroup.policies is converted to use the new model in the REST API and RequestSpec compat code.","commit_id":"c0e2d8e0eb3d66c33fb46982d316b345d2d83bb1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a382f3dd53e93a1fac438c9af4b31294771426b0","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the policies field."},{"line_number":12,"context_line":"2. Also, make _from_db_object and _create_in_db method to"},{"line_number":13,"context_line":"support new \"policy\" field."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ib33719a4b9599d86848c618a6e142c71ece79ca5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"5f7c97a3_5d12b856","line":14,"in_reply_to":"5f7c97a3_5a871e5c","updated":"2018-07-09 14:39:14.000000000","message":"Done","commit_id":"c0e2d8e0eb3d66c33fb46982d316b345d2d83bb1"}],"nova/api/openstack/compute/server_groups.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c4db48f547cf6eb1dfd7ec2ad952fe95f870021f","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            policies \u003d vals.get(\u0027policies\u0027)"},{"line_number":170,"context_line":"            sgp \u003d objects.InstanceGroupPolicy(context, policy\u003dpolicies[0],"},{"line_number":171,"context_line":"                                              rules\u003dNone)"},{"line_number":172,"context_line":"            sg.policy \u003d sgp"},{"line_number":173,"context_line":"            sg.create()"},{"line_number":174,"context_line":"        except ValueError as e:"},{"line_number":175,"context_line":"            raise exc.HTTPBadRequest(explanation\u003de)"}],"source_content_type":"text/x-python","patch_set":30,"id":"5f7c97a3_8426bd15","line":172,"updated":"2018-07-09 13:01:39.000000000","message":"Since you\u0027re changing this in this change, maybe you should also handle this code in this same change:\n\nhttps://review.openstack.org/#/c/571166/16/nova/objects/request_spec.py@213","commit_id":"65c40395a68dc4a1dc6087250775ed46c5a568a7"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"ab1fc972e2e8f84c0e6d6e4d7518dd275e7d5c44","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            policies \u003d vals.get(\u0027policies\u0027)"},{"line_number":170,"context_line":"            sgp \u003d objects.InstanceGroupPolicy(context, policy\u003dpolicies[0],"},{"line_number":171,"context_line":"                                              rules\u003dNone)"},{"line_number":172,"context_line":"            sg.policy \u003d sgp"},{"line_number":173,"context_line":"            sg.create()"},{"line_number":174,"context_line":"        except ValueError as e:"},{"line_number":175,"context_line":"            raise exc.HTTPBadRequest(explanation\u003de)"}],"source_content_type":"text/x-python","patch_set":30,"id":"5f7c97a3_07306f06","line":172,"in_reply_to":"5f7c97a3_8426bd15","updated":"2018-07-09 13:58:00.000000000","message":"Done","commit_id":"65c40395a68dc4a1dc6087250775ed46c5a568a7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"53fc43fb57028aad5de877962dd9adfc6941481c","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        vals \u003d body[\u0027server_group\u0027]"},{"line_number":164,"context_line":"        sg \u003d objects.InstanceGroup(context, rules\u003d{})"},{"line_number":165,"context_line":"        sg.project_id \u003d context.project_id"},{"line_number":166,"context_line":"        sg.user_id \u003d context.user_id"},{"line_number":167,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":38,"id":"5f7c97a3_5cbcda6a","line":164,"range":{"start_line":164,"start_character":44,"end_line":164,"end_character":52},"updated":"2018-07-10 18:08:35.000000000","message":"nit: don\u0027t really need this now","commit_id":"251523aa19cdc3d5c771043bb773e8745194c07d"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"76846ced56788ecf124b581ff45ef9e1ae9fe8a1","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            raise exc.HTTPForbidden(explanation\u003dmsg)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        vals \u003d body[\u0027server_group\u0027]"},{"line_number":164,"context_line":"        sg \u003d objects.InstanceGroup(context, rules\u003d{})"},{"line_number":165,"context_line":"        sg.project_id \u003d context.project_id"},{"line_number":166,"context_line":"        sg.user_id \u003d context.user_id"},{"line_number":167,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":38,"id":"5f7c97a3_a4286c98","line":164,"range":{"start_line":164,"start_character":44,"end_line":164,"end_character":52},"in_reply_to":"5f7c97a3_5cbcda6a","updated":"2018-07-11 02:06:19.000000000","message":"Done","commit_id":"251523aa19cdc3d5c771043bb773e8745194c07d"}],"nova/db/sqlalchemy/api_models.py":[{"author":{"_account_id":25562,"name":"Chen","email":"dstbtgagt@foxmail.com","username":"chenn2"},"change_message_id":"b743303ad0cde2849edbd5686f2c91a454fa4eb0","unresolved":false,"context_lines":[{"line_number":443,"context_line":"    _members \u003d orm.relationship(InstanceGroupMember,"},{"line_number":444,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupMember.group_id\u0027)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    @property"},{"line_number":447,"context_line":"    def policies(self):"},{"line_number":448,"context_line":"        return [self.policy.policy] if self.policy else []"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @property"},{"line_number":451,"context_line":"    def members(self):"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_5c697189","line":448,"range":{"start_line":446,"start_character":4,"end_line":448,"end_character":58},"updated":"2018-06-25 13:00:22.000000000","message":"as far as I know, there can only be one policy for a group. so maybe better rename the method and avoid returning a list;\nalso I am thinking of adding \u0027max_server_per_host\u0027 to the returned value. so basically return\n1 self.policy.policy\nor\n2 {\u0027name\u0027: self.policy.policy, \u0027rules\u0027: self.policy.rules}","commit_id":"85f90b0b530c5b98a098470c929ef3a78eac53e4"},{"author":{"_account_id":25562,"name":"Chen","email":"dstbtgagt@foxmail.com","username":"chenn2"},"change_message_id":"a22b08941cbd388706193e16dbf8c1eeffadbdd8","unresolved":false,"context_lines":[{"line_number":443,"context_line":"    _members \u003d orm.relationship(InstanceGroupMember,"},{"line_number":444,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupMember.group_id\u0027)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    @property"},{"line_number":447,"context_line":"    def policies(self):"},{"line_number":448,"context_line":"        return [self.policy.policy] if self.policy else []"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @property"},{"line_number":451,"context_line":"    def members(self):"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_1495f63c","line":448,"range":{"start_line":446,"start_character":4,"end_line":448,"end_character":58},"in_reply_to":"5f7c97a3_4f0d233c","updated":"2018-06-26 13:04:10.000000000","message":"Now I see that for newer api versions policy (L440) will be used, and for older api versions this polices property will be used. Thanks for the verification.","commit_id":"85f90b0b530c5b98a098470c929ef3a78eac53e4"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"95cb2861afedb645c4d54a0ee84b3c6e677d11dd","unresolved":false,"context_lines":[{"line_number":443,"context_line":"    _members \u003d orm.relationship(InstanceGroupMember,"},{"line_number":444,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupMember.group_id\u0027)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    @property"},{"line_number":447,"context_line":"    def policies(self):"},{"line_number":448,"context_line":"        return [self.policy.policy] if self.policy else []"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @property"},{"line_number":451,"context_line":"    def members(self):"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_4f0d233c","line":448,"range":{"start_line":446,"start_character":4,"end_line":448,"end_character":58},"in_reply_to":"5f7c97a3_5c697189","updated":"2018-06-26 01:26:51.000000000","message":"The policies property is deprecated and only for compatibility, we will also remove this deprecated property in the future.\n\nAnd the policy(L440) which is InstanceGroupPolicy, would be used directly, like [1].\n\nAm I makeing sense? and your idea?\n\n[1] https://review.openstack.org/#/c/563375/16/nova/tests/unit/objects/test_instance_group.py@L156","commit_id":"85f90b0b530c5b98a098470c929ef3a78eac53e4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":437,"context_line":"    project_id \u003d Column(String(255))"},{"line_number":438,"context_line":"    uuid \u003d Column(String(36), nullable\u003dFalse)"},{"line_number":439,"context_line":"    name \u003d Column(String(255))"},{"line_number":440,"context_line":"    policy \u003d orm.relationship(InstanceGroupPolicy,"},{"line_number":441,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupPolicy.group_id\u0027,"},{"line_number":442,"context_line":"            uselist\u003dFalse)"},{"line_number":443,"context_line":"    _members \u003d orm.relationship(InstanceGroupMember,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_6d331e14","line":440,"updated":"2018-06-27 14:45:39.000000000","message":"Do we really need to make this change? I guess it\u0027s not really a column on the instance_groups table so renaming this is just in the model.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"21ad3953dbc5e359cdd516b5c1cb0cc5c23ba883","unresolved":false,"context_lines":[{"line_number":437,"context_line":"    project_id \u003d Column(String(255))"},{"line_number":438,"context_line":"    uuid \u003d Column(String(36), nullable\u003dFalse)"},{"line_number":439,"context_line":"    name \u003d Column(String(255))"},{"line_number":440,"context_line":"    policy \u003d orm.relationship(InstanceGroupPolicy,"},{"line_number":441,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupPolicy.group_id\u0027,"},{"line_number":442,"context_line":"            uselist\u003dFalse)"},{"line_number":443,"context_line":"    _members \u003d orm.relationship(InstanceGroupMember,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d63747af","line":440,"in_reply_to":"5f7c97a3_6d331e14","updated":"2018-06-28 14:05:52.000000000","message":"Per the IRC conversation today, please split this change to the backref in the model into a separate patch - it makes the versioned object integration and new field less confusing when we don\u0027t have to also consider the backref changing in the DB model.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":439,"context_line":"    name \u003d Column(String(255))"},{"line_number":440,"context_line":"    policy \u003d orm.relationship(InstanceGroupPolicy,"},{"line_number":441,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupPolicy.group_id\u0027,"},{"line_number":442,"context_line":"            uselist\u003dFalse)"},{"line_number":443,"context_line":"    _members \u003d orm.relationship(InstanceGroupMember,"},{"line_number":444,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupMember.group_id\u0027)"},{"line_number":445,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_95b3244f","line":442,"range":{"start_line":442,"start_character":12,"end_line":442,"end_character":25},"updated":"2018-06-27 14:45:39.000000000","message":"OK this enforces the one-to-one relationship:\n\nhttp://docs.sqlalchemy.org/en/latest/orm/basic_relationships.html#one-to-one\n\nLooking at the examples in ^, it seems we could also create the InstanceGroup.policy via a backref on the InstanceGroupPolicy, e.g.:\n\nclass InstanceGroupPolicy(...):\n   ...\n   group \u003d relationship(InstanceGroup, back_populates\u003d\"policy\")\n\nI believe that will implicitly add the InstanceGroup.policy attribute.\n\nAnd you\u0027d also have:\n\nclass InstanceGroup(...):\n   ...\n   policy \u003d relationship(InstanceGroupPolicy, uselist\u003dFalse, back_populates\u003d\"group\")\n\nAnyway, it might not matter since I think we get the same thing either way.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":444,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupMember.group_id\u0027)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    @property"},{"line_number":447,"context_line":"    def policies(self):"},{"line_number":448,"context_line":"        return [self.policy.policy] if self.policy else []"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_cd2352bd","line":447,"range":{"start_line":447,"start_character":8,"end_line":447,"end_character":16},"updated":"2018-06-27 14:45:39.000000000","message":"Is this still used?","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2dd86f8efc71db0cd7005985ef8fd17087d3c3b2","unresolved":false,"context_lines":[{"line_number":444,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupMember.group_id\u0027)"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    @property"},{"line_number":447,"context_line":"    def policies(self):"},{"line_number":448,"context_line":"        return [self.policy.policy] if self.policy else []"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_f88fed73","line":447,"range":{"start_line":447,"start_character":8,"end_line":447,"end_character":16},"in_reply_to":"5f7c97a3_cd2352bd","updated":"2018-06-27 14:51:49.000000000","message":"Is this still used?\n\nLooks like it\u0027s used here:\n\nhttps://review.openstack.org/#/c/563375/17/nova/objects/instance_group.py@208\n\nand here:\n\nhttps://review.openstack.org/#/c/563375/17/nova/objects/instance_group.py@651\n\n--\n\nI think we should drop the one on L208 and and if you rebase on top of https://review.openstack.org/#/c/578192/ then we\u0027ll drop the one on L651 also.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"749f7f6cbe8c7b562421ca0cd9439f2d709b1e3a","unresolved":false,"context_lines":[{"line_number":437,"context_line":"    project_id \u003d Column(String(255))"},{"line_number":438,"context_line":"    uuid \u003d Column(String(36), nullable\u003dFalse)"},{"line_number":439,"context_line":"    name \u003d Column(String(255))"},{"line_number":440,"context_line":"    policy \u003d orm.relationship(InstanceGroupPolicy,"},{"line_number":441,"context_line":"            primaryjoin\u003d\u0027InstanceGroup.id \u003d\u003d InstanceGroupPolicy.group_id\u0027,"},{"line_number":442,"context_line":"            uselist\u003dFalse)"},{"line_number":443,"context_line":"    _members \u003d orm.relationship(InstanceGroupMember,"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_5914c7e9","line":440,"updated":"2018-06-28 15:14:13.000000000","message":"Still need to split this change into a separate patch.","commit_id":"7c17b29c61aa569593014757d8773a8aecf4380d"}],"nova/objects/instance_group.py":[{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"5f90f618ea0829c4da7b4c6b6fbf4ecfab656673","unresolved":false,"context_lines":[{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        db_group \u003d self._create_in_db(self._context, updates,"},{"line_number":472,"context_line":"                                      policies\u003dpolicies,"},{"line_number":473,"context_line":"                                      members\u003dmembers, "},{"line_number":474,"context_line":"                                      policy\u003dpolicy)"},{"line_number":475,"context_line":"        self._from_db_object(self._context, self, db_group)"},{"line_number":476,"context_line":"        payload[\u0027server_group_id\u0027] \u003d self.uuid"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f6a8fd7_8cf85c49","line":473,"range":{"start_line":473,"start_character":54,"end_line":473,"end_character":55},"updated":"2018-04-21 03:48:09.000000000","message":"pep8","commit_id":"4b3a74cad9c65166c1524dd2dabee9ce4ad78b75"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"84c488c4061f0e1ca249df1994ff87bb160110a8","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":150,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":153,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":154,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":155,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_c780e149","line":152,"updated":"2018-06-05 17:23:25.000000000","message":"Should probably add a TODO to drop this field in version 2.0 of this object.","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"62839220a2425ef797b68fe6c141b0ede37ba058","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":150,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":153,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":154,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":155,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_c6a9b9dd","line":152,"in_reply_to":"5f7c97a3_c780e149","updated":"2018-06-06 02:21:25.000000000","message":"OK,sure","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"84c488c4061f0e1ca249df1994ff87bb160110a8","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"},{"line_number":156,"context_line":"        }"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":159,"context_line":"        if \u0027policies\u0027 in kwargs:"},{"line_number":160,"context_line":"            if kwargs[\u0027policies\u0027]:"},{"line_number":161,"context_line":"                kwargs[\u0027policy\u0027] \u003d objects.InstanceGroupPolicy("},{"line_number":162,"context_line":"                    policy\u003dkwargs[\u0027policies\u0027][0], rules\u003dNone)"},{"line_number":163,"context_line":"            else:"},{"line_number":164,"context_line":"                kwargs[\u0027policy\u0027] \u003d None"},{"line_number":165,"context_line":"        super(InstanceGroup, self).__init__(*args, **kwargs)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":168,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_67c30d8c","line":165,"range":{"start_line":158,"start_character":4,"end_line":165,"end_character":60},"updated":"2018-06-05 17:23:25.000000000","message":"I\u0027m not really sure we need this, we control how this object is constructed, and besides, it\u0027s not like we can\u0027t set policies via the setattr anyway.","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"62839220a2425ef797b68fe6c141b0ede37ba058","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"},{"line_number":156,"context_line":"        }"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":159,"context_line":"        if \u0027policies\u0027 in kwargs:"},{"line_number":160,"context_line":"            if kwargs[\u0027policies\u0027]:"},{"line_number":161,"context_line":"                kwargs[\u0027policy\u0027] \u003d objects.InstanceGroupPolicy("},{"line_number":162,"context_line":"                    policy\u003dkwargs[\u0027policies\u0027][0], rules\u003dNone)"},{"line_number":163,"context_line":"            else:"},{"line_number":164,"context_line":"                kwargs[\u0027policy\u0027] \u003d None"},{"line_number":165,"context_line":"        super(InstanceGroup, self).__init__(*args, **kwargs)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":168,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_a6e4bd0f","line":165,"range":{"start_line":158,"start_character":4,"end_line":165,"end_character":60},"in_reply_to":"5f7c97a3_67c30d8c","updated":"2018-06-06 02:21:25.000000000","message":"emm, yeah, I had same question, we have some case like [1], the InstanceGroup object is construct by old style policies. I thought there are 2 way:\n1. Keep the compatible as above. Convert policies to new style policy. and after 2.0 version, we can remove these convert code. \n2. We remove all these old style test cases, adapt these cases to new style policy.\n\nI choose the 1st way, but I don\u0027t process setattr case, since I don\u0027t saw setattr case in testcase...so, should we need support it?\n\nShould we use 1st way to implement it, or any other way you suggest?\n\n[1] https://github.com/openstack/nova/blob/86574f7f8eaf192656c35942140f7eba127a9255/nova/tests/unit/compute/test_compute_utils.py#L1132","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d2fd0f0feef0a8c754d697a9bf3354806a9e094","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"},{"line_number":156,"context_line":"        }"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":159,"context_line":"        if \u0027policies\u0027 in kwargs:"},{"line_number":160,"context_line":"            if kwargs[\u0027policies\u0027]:"},{"line_number":161,"context_line":"                kwargs[\u0027policy\u0027] \u003d objects.InstanceGroupPolicy("},{"line_number":162,"context_line":"                    policy\u003dkwargs[\u0027policies\u0027][0], rules\u003dNone)"},{"line_number":163,"context_line":"            else:"},{"line_number":164,"context_line":"                kwargs[\u0027policy\u0027] \u003d None"},{"line_number":165,"context_line":"        super(InstanceGroup, self).__init__(*args, **kwargs)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":168,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_c5c2f453","line":165,"range":{"start_line":158,"start_character":4,"end_line":165,"end_character":60},"in_reply_to":"5f7c97a3_a6e4bd0f","updated":"2018-06-07 01:51:09.000000000","message":"I tend to think that all main-line and test code should be using the policy field now unless it\u0027s explicitly testing obj_make_compatible. If we have to send anything over rpc that backports this version, we have the compat code to set policies from policy below. We don\u0027t have to remove the existing tests, just adapt them to use the policy field instead of the policies field.\n\nRenaming/retyping fields in versioned objects is kind of weird though, we might want to get a second opinion from dansmith.","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d2fd0f0feef0a8c754d697a9bf3354806a9e094","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            # NOTE(danms): Before 1.7, we had an always-empty"},{"line_number":171,"context_line":"            # metadetails property"},{"line_number":172,"context_line":"            primitive[\u0027metadetails\u0027] \u003d {}"},{"line_number":173,"context_line":"        if target_version \u003c (1, 11):"},{"line_number":174,"context_line":"            # NOTE(yikun): Before 1.11, we had a policies property"},{"line_number":175,"context_line":"            # which is the list of policy name."},{"line_number":176,"context_line":"            policy \u003d primitive.get(\u0027policy\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_659f6065","line":173,"updated":"2018-06-07 01:51:09.000000000","message":"This should go before the 1.7 check above, we always check newest to oldest.","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"84c488c4061f0e1ca249df1994ff87bb160110a8","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            primitive[\u0027policies\u0027] \u003d [policy.policy] if policy else []"},{"line_number":178,"context_line":"            if \u0027policy\u0027 in primitive:"},{"line_number":179,"context_line":"                del primitive[\u0027policy\u0027]"},{"line_number":180,"context_line":"        else:"},{"line_number":181,"context_line":"            if \u0027policies\u0027 in primitive:"},{"line_number":182,"context_line":"                del primitive[\u0027policies\u0027]"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    @staticmethod"},{"line_number":185,"context_line":"    def _from_db_object(context, instance_group, db_inst):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_67da2d7a","line":182,"range":{"start_line":180,"start_character":8,"end_line":182,"end_character":41},"updated":"2018-06-05 17:23:25.000000000","message":"This shouldn\u0027t be in here.","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"62839220a2425ef797b68fe6c141b0ede37ba058","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            primitive[\u0027policies\u0027] \u003d [policy.policy] if policy else []"},{"line_number":178,"context_line":"            if \u0027policy\u0027 in primitive:"},{"line_number":179,"context_line":"                del primitive[\u0027policy\u0027]"},{"line_number":180,"context_line":"        else:"},{"line_number":181,"context_line":"            if \u0027policies\u0027 in primitive:"},{"line_number":182,"context_line":"                del primitive[\u0027policies\u0027]"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    @staticmethod"},{"line_number":185,"context_line":"    def _from_db_object(context, instance_group, db_inst):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_e6f07500","line":182,"range":{"start_line":180,"start_character":8,"end_line":182,"end_character":41},"in_reply_to":"5f7c97a3_67da2d7a","updated":"2018-06-06 02:21:25.000000000","message":"Yeah, so we just need remove ``policies`` after 2.0 version, right?","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d2fd0f0feef0a8c754d697a9bf3354806a9e094","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            primitive[\u0027policies\u0027] \u003d [policy.policy] if policy else []"},{"line_number":178,"context_line":"            if \u0027policy\u0027 in primitive:"},{"line_number":179,"context_line":"                del primitive[\u0027policy\u0027]"},{"line_number":180,"context_line":"        else:"},{"line_number":181,"context_line":"            if \u0027policies\u0027 in primitive:"},{"line_number":182,"context_line":"                del primitive[\u0027policies\u0027]"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    @staticmethod"},{"line_number":185,"context_line":"    def _from_db_object(context, instance_group, db_inst):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_45aca49e","line":182,"range":{"start_line":180,"start_character":8,"end_line":182,"end_character":41},"in_reply_to":"5f7c97a3_e6f07500","updated":"2018-06-07 01:51:09.000000000","message":"Yes.","commit_id":"cc7c4938a1af149ad1f22e6898815084bfc318cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c49f4aad663938bc1b34044e99cf2d3f635af906","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def _instance_group_get_query(context, id_field\u003dNone, id\u003dNone):"},{"line_number":40,"context_line":"    query \u003d context.session.query(api_models.InstanceGroup).\\"},{"line_number":41,"context_line":"            options(joinedload(\u0027policy\u0027)).\\"},{"line_number":42,"context_line":"            options(joinedload(\u0027_members\u0027))"},{"line_number":43,"context_line":"    if not context.is_admin:"},{"line_number":44,"context_line":"        query \u003d query.filter_by(project_id\u003dcontext.project_id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_deac5938","line":41,"updated":"2018-06-27 16:19:33.000000000","message":"If you remove this, then every time you touch it during load we will lazy-load it, which will be less efficient. Since you always need to check this in order to provide compatibility, I think you need to just *add* the new join, and leave the old one. Once the data is migrated, it should be very low-impact to join an empty thing.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b66018596c0601330b315a57985202502f4ccae6","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def _instance_group_get_query(context, id_field\u003dNone, id\u003dNone):"},{"line_number":40,"context_line":"    query \u003d context.session.query(api_models.InstanceGroup).\\"},{"line_number":41,"context_line":"            options(joinedload(\u0027policy\u0027)).\\"},{"line_number":42,"context_line":"            options(joinedload(\u0027_members\u0027))"},{"line_number":43,"context_line":"    if not context.is_admin:"},{"line_number":44,"context_line":"        query \u003d query.filter_by(project_id\u003dcontext.project_id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_bb2cf4b2","line":41,"in_reply_to":"5f7c97a3_645d747b","updated":"2018-06-28 13:59:14.000000000","message":"To summarize, I think Dan is saying we need this:\n\nquery \u003d context.session.query(api_models.InstanceGroup).\\\n            options(joinedload(\u0027_policies\u0027)).\\\n            options(joinedload(\u0027policy\u0027)).\\\n            options(joinedload(\u0027_members\u0027))\n\nIf we have any code that\u0027s doing db_policy._policies checking - however, looking below, I don\u0027t see any that is, so I\u0027m not sure why we\u0027d need both?\n\nThe InstanceGroupPolicy._policies attribute has already been changed to .policy and that\u0027s how he\u0027s accessing it now - or is the concern that older versioned object code could be trying to load ._policies? I\u0027m not sure how that can happen.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"706f54f88ebd91bf9a4f77d58fac7ae67507ed3f","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def _instance_group_get_query(context, id_field\u003dNone, id\u003dNone):"},{"line_number":40,"context_line":"    query \u003d context.session.query(api_models.InstanceGroup).\\"},{"line_number":41,"context_line":"            options(joinedload(\u0027policy\u0027)).\\"},{"line_number":42,"context_line":"            options(joinedload(\u0027_members\u0027))"},{"line_number":43,"context_line":"    if not context.is_admin:"},{"line_number":44,"context_line":"        query \u003d query.filter_by(project_id\u003dcontext.project_id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_645d747b","line":41,"in_reply_to":"5f7c97a3_c92d8190","updated":"2018-06-27 17:18:09.000000000","message":"No, this returns a SQLA db object. If you don\u0027t joinedload policies, then attempting to reference it will cause us to lazy-load (SQLA lazy-load, not objects). If we do that every time to see if there is old data there (which we should) then you\u0027re just lazy-loading that every time.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ce1aac924b69ac2b6f34838a8c6274ae7a8e224d","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def _instance_group_get_query(context, id_field\u003dNone, id\u003dNone):"},{"line_number":40,"context_line":"    query \u003d context.session.query(api_models.InstanceGroup).\\"},{"line_number":41,"context_line":"            options(joinedload(\u0027policy\u0027)).\\"},{"line_number":42,"context_line":"            options(joinedload(\u0027_members\u0027))"},{"line_number":43,"context_line":"    if not context.is_admin:"},{"line_number":44,"context_line":"        query \u003d query.filter_by(project_id\u003dcontext.project_id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_c92d8190","line":41,"in_reply_to":"5f7c97a3_deac5938","updated":"2018-06-27 16:27:36.000000000","message":"\u003e If you remove this\n\n\"This\" being the _policies field on the API model object? I\u0027m confused.\n\n\u003e we will lazy-load it\n\nobj_load_attr would blow up if you tried to lazy-load the policy field (at least how it\u0027s written today).","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"59ec63595d39d3ad15c469b19a66e316a6a68117","unresolved":false,"context_lines":[{"line_number":72,"context_line":"    return models"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def _instance_group_policy_add(context, group, policy):"},{"line_number":76,"context_line":"    model \u003d api_models.InstanceGroupPolicy()"},{"line_number":77,"context_line":"    rules \u003d jsonutils.dumps(policy.rules) if policy.rules else None"},{"line_number":78,"context_line":"    values \u003d {"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_bef5dd53","line":75,"range":{"start_line":75,"start_character":4,"end_line":75,"end_character":30},"updated":"2018-06-27 16:10:55.000000000","message":"See my comments at https://review.openstack.org/#/c/573628/6/nova/objects/instance_group_policy.py@57 - basically, we don\u0027t even need this method, we can just call policy.create() right?","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":72,"context_line":"    return models"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def _instance_group_policy_add(context, group, policy):"},{"line_number":76,"context_line":"    model \u003d api_models.InstanceGroupPolicy()"},{"line_number":77,"context_line":"    rules \u003d jsonutils.dumps(policy.rules) if policy.rules else None"},{"line_number":78,"context_line":"    values \u003d {"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_95c48462","line":75,"range":{"start_line":75,"start_character":27,"end_line":75,"end_character":30},"updated":"2018-06-27 14:45:39.000000000","message":"nit: I know this is following the _instance_group_policies_add naming convention, but I\u0027d like to avoid \"add\" and \"remove\" wording for policy since it\u0027s one-to-one, so we can\u0027t really add anything, we can update/set the policy on the group, so let\u0027s call this method _instance_group_policy_update or something like that.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":72,"context_line":"    return models"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def _instance_group_policy_add(context, group, policy):"},{"line_number":76,"context_line":"    model \u003d api_models.InstanceGroupPolicy()"},{"line_number":77,"context_line":"    rules \u003d jsonutils.dumps(policy.rules) if policy.rules else None"},{"line_number":78,"context_line":"    values \u003d {"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_6020e9f9","line":75,"range":{"start_line":75,"start_character":4,"end_line":75,"end_character":30},"in_reply_to":"5f7c97a3_bef5dd53","updated":"2018-06-28 15:30:23.000000000","message":"Yes, we can use policy.create directly.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def _instance_group_policy_add(context, group, policy):"},{"line_number":76,"context_line":"    model \u003d api_models.InstanceGroupPolicy()"},{"line_number":77,"context_line":"    rules \u003d jsonutils.dumps(policy.rules) if policy.rules else None"},{"line_number":78,"context_line":"    values \u003d {"},{"line_number":79,"context_line":"        \u0027group_id\u0027: group.id,"},{"line_number":80,"context_line":"        \u0027policy\u0027: policy.policy,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_35dd7041","line":77,"range":{"start_line":77,"start_character":12,"end_line":77,"end_character":41},"updated":"2018-06-27 14:45:39.000000000","message":"Just a thought, but alternative to this, we could store the serialized json for rules in InstanceGroupPolicy like this:\n\nInstanceGroup._rules \u003d StringField\n\nAnd then have getter and setter methods for InstanceGroup.rules which deserializes and serializes the _rules field, respectively.\n\nThen this code in _instance_group_policy_add could just use InstanceGroupPolicy._rules and not have to deal with serializing it. I\u0027m not sure it\u0027s much cleaner though so it\u0027s just a random thought.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"def _instance_group_policy_add(context, group, policy):"},{"line_number":76,"context_line":"    model \u003d api_models.InstanceGroupPolicy()"},{"line_number":77,"context_line":"    rules \u003d jsonutils.dumps(policy.rules) if policy.rules else None"},{"line_number":78,"context_line":"    values \u003d {"},{"line_number":79,"context_line":"        \u0027group_id\u0027: group.id,"},{"line_number":80,"context_line":"        \u0027policy\u0027: policy.policy,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_40236d07","line":77,"range":{"start_line":77,"start_character":12,"end_line":77,"end_character":41},"in_reply_to":"5f7c97a3_35dd7041","updated":"2018-06-28 15:30:23.000000000","message":"Great idea, and I also found a same example on migrate_data obj[1], and also I refresh my patch[2].\n\n[1] https://github.com/openstack/nova/blob/4e7672d/nova/objects/migrate_data.py#L100-L111\n[2] https://review.openstack.org/#/c/573628/9/nova/objects/instance_group.py@513","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    rules \u003d jsonutils.dumps(policy.rules) if policy.rules else None"},{"line_number":78,"context_line":"    values \u003d {"},{"line_number":79,"context_line":"        \u0027group_id\u0027: group.id,"},{"line_number":80,"context_line":"        \u0027policy\u0027: policy.policy,"},{"line_number":81,"context_line":"        \u0027rules\u0027: rules"},{"line_number":82,"context_line":"    }"},{"line_number":83,"context_line":"    model.update(values)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_750528c1","line":80,"range":{"start_line":80,"start_character":18,"end_line":80,"end_character":31},"updated":"2018-06-27 14:45:39.000000000","message":"This is confusing, I\u0027d rename the policy parameter to this method as \"instance_group_policy\" or \"policy_obj\" or something to indicate it\u0027s an object rather than a policy name.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                api_models.InstanceGroupPolicy.policy.in_(set(policies)))"},{"line_number":94,"context_line":"    return _instance_group_model_add(context, api_models.InstanceGroupPolicy,"},{"line_number":95,"context_line":"                                     policies, query.all(), \u0027policy\u0027, group.id,"},{"line_number":96,"context_line":"                                     append_to_models\u003dgroup._policies)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"def _instance_group_members_add(context, group, members):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b5b74022","line":96,"range":{"start_line":96,"start_character":54,"end_line":96,"end_character":69},"updated":"2018-06-27 14:45:39.000000000","message":"Isn\u0027t this broken now? Or it just looks like _instance_group_policies_add isn\u0027t used anymore and can be removed.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                api_models.InstanceGroupPolicy.policy.in_(set(policies)))"},{"line_number":94,"context_line":"    return _instance_group_model_add(context, api_models.InstanceGroupPolicy,"},{"line_number":95,"context_line":"                                     policies, query.all(), \u0027policy\u0027, group.id,"},{"line_number":96,"context_line":"                                     append_to_models\u003dgroup._policies)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"def _instance_group_members_add(context, group, members):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_fb16ecf2","line":96,"range":{"start_line":96,"start_character":54,"end_line":96,"end_character":69},"in_reply_to":"5f7c97a3_b5b74022","updated":"2018-06-28 15:30:23.000000000","message":"Yeah, we can remove this","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    # Version 1.4: Add add_members()"},{"line_number":134,"context_line":"    # Version 1.5: Add get_hosts()"},{"line_number":135,"context_line":"    # Version 1.6: Add get_by_name()"},{"line_number":136,"context_line":"    # Version 1.7: Deprecate metadetails"},{"line_number":137,"context_line":"    # Version 1.8: Add count_members_by_user()"},{"line_number":138,"context_line":"    # Version 1.9: Add get_by_instance_uuid()"},{"line_number":139,"context_line":"    # Version 1.10: Add hosts field"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b56f405f","line":136,"range":{"start_line":136,"start_character":29,"end_line":136,"end_character":40},"updated":"2018-06-27 14:45:39.000000000","message":"I wonder if we can follow the pattern of this field, which isn\u0027t listed in fields, and drop policies in this version, but always set policies if we\u0027re backporting to a version \u003c 1.11? Probably need to ask dansmith.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    # Version 1.4: Add add_members()"},{"line_number":134,"context_line":"    # Version 1.5: Add get_hosts()"},{"line_number":135,"context_line":"    # Version 1.6: Add get_by_name()"},{"line_number":136,"context_line":"    # Version 1.7: Deprecate metadetails"},{"line_number":137,"context_line":"    # Version 1.8: Add count_members_by_user()"},{"line_number":138,"context_line":"    # Version 1.9: Add get_by_instance_uuid()"},{"line_number":139,"context_line":"    # Version 1.10: Add hosts field"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_3814854d","line":136,"range":{"start_line":136,"start_character":29,"end_line":136,"end_character":40},"in_reply_to":"5f7c97a3_b56f405f","updated":"2018-06-27 15:13:27.000000000","message":"I think that only worked because it was always empty. If we were to actually remove it, then we wouldn\u0027t be able to receive the policies field from older clients that send it.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":151,"context_line":"        # TODO(yikun): policies is deprecated and should"},{"line_number":152,"context_line":"        # be removed on the next major version bump."},{"line_number":153,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":154,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":155,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":156,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_58c2a104","line":153,"updated":"2018-06-27 15:13:27.000000000","message":"Maybe make this readonly\u003dTrue? That will help flush out any uses of this field that aren\u0027t converted. Since the backport routine operates on the primitive, it won\u0027t have any problem converting it back.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eaccc2ff9faf5f446c4853f97c4fff58ad04c7b4","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":151,"context_line":"        # TODO(yikun): policies is deprecated and should"},{"line_number":152,"context_line":"        # be removed on the next major version bump."},{"line_number":153,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":154,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":155,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":156,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_def3f9e7","line":153,"in_reply_to":"5f7c97a3_58c2a104","updated":"2018-06-27 16:02:05.000000000","message":"Good idea, I was trying to think of something like that and forgot about the readonly option.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":151,"context_line":"        # TODO(yikun): policies is deprecated and should"},{"line_number":152,"context_line":"        # be removed on the next major version bump."},{"line_number":153,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":154,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":155,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":156,"context_line":"        \u0027policy\u0027: fields.ObjectField(\u0027InstanceGroupPolicy\u0027, nullable\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_5bdfe037","line":153,"in_reply_to":"5f7c97a3_def3f9e7","updated":"2018-06-28 15:30:23.000000000","message":"Done","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        if target_version \u003c (1, 11):"},{"line_number":162,"context_line":"            # NOTE(yikun): Before 1.11, we had a policies property"},{"line_number":163,"context_line":"            # which is the list of policy name."},{"line_number":164,"context_line":"            policy \u003d primitive.get(\u0027policy\u0027)"},{"line_number":165,"context_line":"            if policy:"},{"line_number":166,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":167,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b5a420a5","line":164,"updated":"2018-06-27 14:45:39.000000000","message":"If you do this:\n\npolicy \u003d primitive.pop(\u0027policy\u0027, None)\n\nThen you don\u0027t need the conditional on L170 below.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        if target_version \u003c (1, 11):"},{"line_number":162,"context_line":"            # NOTE(yikun): Before 1.11, we had a policies property"},{"line_number":163,"context_line":"            # which is the list of policy name."},{"line_number":164,"context_line":"            policy \u003d primitive.get(\u0027policy\u0027)"},{"line_number":165,"context_line":"            if policy:"},{"line_number":166,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":167,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b8c17551","line":164,"in_reply_to":"5f7c97a3_b5a420a5","updated":"2018-06-27 15:13:27.000000000","message":"+1","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        if target_version \u003c (1, 11):"},{"line_number":162,"context_line":"            # NOTE(yikun): Before 1.11, we had a policies property"},{"line_number":163,"context_line":"            # which is the list of policy name."},{"line_number":164,"context_line":"            policy \u003d primitive.get(\u0027policy\u0027)"},{"line_number":165,"context_line":"            if policy:"},{"line_number":166,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":167,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_dba1b0aa","line":164,"in_reply_to":"5f7c97a3_b5a420a5","updated":"2018-06-28 15:30:23.000000000","message":"Done","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            if policy:"},{"line_number":166,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":167,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":168,"context_line":"            else:"},{"line_number":169,"context_line":"                primitive[\u0027policies\u0027] \u003d []"},{"line_number":170,"context_line":"            if \u0027policy\u0027 in primitive:"},{"line_number":171,"context_line":"                del primitive[\u0027policy\u0027]"},{"line_number":172,"context_line":"        if target_version \u003c (1, 7):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_95c0042e","line":169,"range":{"start_line":168,"start_character":12,"end_line":169,"end_character":42},"updated":"2018-06-27 14:45:39.000000000","message":"Do we even need to set this to anything? If InstanceGroup.policy is not set, I\u0027m not sure why we\u0027d set \u0027policies\u0027 to an empty list in the primitive.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c49f4aad663938bc1b34044e99cf2d3f635af906","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            if policy:"},{"line_number":166,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":167,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":168,"context_line":"            else:"},{"line_number":169,"context_line":"                primitive[\u0027policies\u0027] \u003d []"},{"line_number":170,"context_line":"            if \u0027policy\u0027 in primitive:"},{"line_number":171,"context_line":"                del primitive[\u0027policy\u0027]"},{"line_number":172,"context_line":"        if target_version \u003c (1, 7):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_9e02610c","line":169,"range":{"start_line":168,"start_character":12,"end_line":169,"end_character":42},"in_reply_to":"5f7c97a3_7ed3c538","updated":"2018-06-27 16:19:33.000000000","message":"What I mean is, we\u0027re directly converting the db object to our fields now in _from_db_obj(). If the join of policies ends up with an empty list of policies if none join in, then we currently always have a list in this field, regardless. Since other code may depend on that behavior, this line will keep that behavior from changing for older clients.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            if policy:"},{"line_number":166,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":167,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":168,"context_line":"            else:"},{"line_number":169,"context_line":"                primitive[\u0027policies\u0027] \u003d []"},{"line_number":170,"context_line":"            if \u0027policy\u0027 in primitive:"},{"line_number":171,"context_line":"                del primitive[\u0027policy\u0027]"},{"line_number":172,"context_line":"        if target_version \u003c (1, 7):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d8ca316e","line":169,"range":{"start_line":168,"start_character":12,"end_line":169,"end_character":42},"in_reply_to":"5f7c97a3_95c0042e","updated":"2018-06-27 15:13:27.000000000","message":"Well, it would be to keep the behavior the same. If we alway have an empty list here after our existing query methods, then we should do this to keep things behaving the same.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eaccc2ff9faf5f446c4853f97c4fff58ad04c7b4","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            if policy:"},{"line_number":166,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":167,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":168,"context_line":"            else:"},{"line_number":169,"context_line":"                primitive[\u0027policies\u0027] \u003d []"},{"line_number":170,"context_line":"            if \u0027policy\u0027 in primitive:"},{"line_number":171,"context_line":"                del primitive[\u0027policy\u0027]"},{"line_number":172,"context_line":"        if target_version \u003c (1, 7):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_7ed3c538","line":169,"range":{"start_line":168,"start_character":12,"end_line":169,"end_character":42},"in_reply_to":"5f7c97a3_d8ca316e","updated":"2018-06-27 16:02:05.000000000","message":"OK I didn\u0027t know if policies would be None or what. I guess \"existing query methods\" meaning like the InstanceGroup.policies property in the API model object that will return an empty list if there are no policy records for the group?","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":193,"context_line":"                      \u0027deleted_at\u0027: None}"},{"line_number":194,"context_line":"            if field in ignore and not hasattr(db_inst, field):"},{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_55010c6d","line":197,"range":{"start_line":196,"start_character":12,"end_line":197,"end_character":19},"updated":"2018-06-27 14:45:39.000000000","message":"You could drop this all back an indent if you just did elif:\n\nif field in ignore...\n    ...\nelif field \u003d\u003d \u0027policies\u0027:\n    ...\nelif field \u003d\u003d \u0027policy\u0027:\n    ...\nelse:\n    ...","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":193,"context_line":"                      \u0027deleted_at\u0027: None}"},{"line_number":194,"context_line":"            if field in ignore and not hasattr(db_inst, field):"},{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_76367305","line":197,"range":{"start_line":196,"start_character":12,"end_line":197,"end_character":19},"in_reply_to":"5f7c97a3_55010c6d","updated":"2018-06-28 15:30:23.000000000","message":"Done","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c49f4aad663938bc1b34044e99cf2d3f635af906","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":201,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_fe6ef5fc","line":198,"updated":"2018-06-27 16:19:33.000000000","message":"I missed this last time, but you need to just do:\n\n if db_inst.policy\n\nhere, because .get won\u0027t lazy-load it and thus we would be missing old data to migrate. If you do the join fix I noted above, then it\u0027s already loaded so you can skip the get.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"706f54f88ebd91bf9a4f77d58fac7ae67507ed3f","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":201,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_a44aacaa","line":198,"in_reply_to":"5f7c97a3_a95e0511","updated":"2018-06-27 17:18:09.000000000","message":"No, I\u0027m talking about the SQLA object. Since the query function above isn\u0027t joined-loading them, this .get() will never return anything since we didn\u0027t load the field (.get() is a little weird in SQLA and so I *think* I\u0027m remembering this right, but regardless we should explicitly check it, not .get() it).","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ce1aac924b69ac2b6f34838a8c6274ae7a8e224d","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":201,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_a95e0511","line":198,"in_reply_to":"5f7c97a3_fe6ef5fc","updated":"2018-06-27 16:27:36.000000000","message":"\u003e  .get won\u0027t lazy-load \n\nAre you talking about the same kind of lazy-load from the obj_load_attr() method? Because db_inst is the sqlalchemy ORM object.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":201,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"},{"line_number":202,"context_line":"                        instance_group[\u0027policies\u0027] \u003d db_inst.get(\u0027policies\u0027)"},{"line_number":203,"context_line":"                    else:"},{"line_number":204,"context_line":"                        instance_group[\u0027policies\u0027] \u003d []"},{"line_number":205,"context_line":"                elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":206,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":207,"context_line":"                        db_policy \u003d db_inst[\u0027policy\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_18e2e9d5","line":204,"range":{"start_line":198,"start_character":0,"end_line":204,"end_character":55},"updated":"2018-06-27 15:13:27.000000000","message":"I think I would avoid setting this all the time now that it\u0027s deprecated. If we load a DB object with only the old-style linkage, then upconvert it to the new style.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eaccc2ff9faf5f446c4853f97c4fff58ad04c7b4","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":201,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"},{"line_number":202,"context_line":"                        instance_group[\u0027policies\u0027] \u003d db_inst.get(\u0027policies\u0027)"},{"line_number":203,"context_line":"                    else:"},{"line_number":204,"context_line":"                        instance_group[\u0027policies\u0027] \u003d []"},{"line_number":205,"context_line":"                elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":206,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":207,"context_line":"                        db_policy \u003d db_inst[\u0027policy\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_bec15d51","line":204,"range":{"start_line":198,"start_character":0,"end_line":204,"end_character":55},"in_reply_to":"5f7c97a3_18e2e9d5","updated":"2018-06-27 16:02:05.000000000","message":"\u003e If we load a DB object with only the old-style linkage\n\nIs that possible though with the changes to the InstanceGroup API model object? The instance_group_policy table records are all the same as before (sans the new rules column), the only thing that changes is how it\u0027s modeled in ORM and the versioned objects.\n\nI thought we could try and keep this change as simple as possible by only dealing with the \u0027policies\u0027 field in this object if we needed to convert down for compatibility.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c49f4aad663938bc1b34044e99cf2d3f635af906","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":196,"context_line":"            else:"},{"line_number":197,"context_line":"                if \u0027policies\u0027 \u003d\u003d field:"},{"line_number":198,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":199,"context_line":"                        policy_name \u003d db_inst[\u0027policy\u0027].get(\u0027policy\u0027)"},{"line_number":200,"context_line":"                        instance_group[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":201,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"},{"line_number":202,"context_line":"                        instance_group[\u0027policies\u0027] \u003d db_inst.get(\u0027policies\u0027)"},{"line_number":203,"context_line":"                    else:"},{"line_number":204,"context_line":"                        instance_group[\u0027policies\u0027] \u003d []"},{"line_number":205,"context_line":"                elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":206,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":207,"context_line":"                        db_policy \u003d db_inst[\u0027policy\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_de1f796f","line":204,"range":{"start_line":198,"start_character":0,"end_line":204,"end_character":55},"in_reply_to":"5f7c97a3_bec15d51","updated":"2018-06-27 16:19:33.000000000","message":"I think we\u0027re saying the same thing. I\u0027m just saying we should avoid setting the policies field all the time, and leaving it unset so that \"if policies in thing\" will tell us if we need to worry about that field elsewhere.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                        instance_group[\u0027policies\u0027] \u003d db_inst.get(\u0027policies\u0027)"},{"line_number":203,"context_line":"                    else:"},{"line_number":204,"context_line":"                        instance_group[\u0027policies\u0027] \u003d []"},{"line_number":205,"context_line":"                elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":206,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":207,"context_line":"                        db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":208,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d5399c40","line":205,"updated":"2018-06-27 14:45:39.000000000","message":"Why do we need to handle both \u0027policies\u0027 and \u0027policy\u0027 fields? Can\u0027t we just ignore the \u0027policies\u0027 field and only set things when we get the \u0027policy\u0027 field?","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":202,"context_line":"                        instance_group[\u0027policies\u0027] \u003d db_inst.get(\u0027policies\u0027)"},{"line_number":203,"context_line":"                    else:"},{"line_number":204,"context_line":"                        instance_group[\u0027policies\u0027] \u003d []"},{"line_number":205,"context_line":"                elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":206,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":207,"context_line":"                        db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":208,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_f8f12d98","line":205,"in_reply_to":"5f7c97a3_d5399c40","updated":"2018-06-27 15:13:27.000000000","message":"The code the way it is now will ignore the newer field if the old field is present, which is probably not the way we should go. Load the new field if it\u0027s set and assume that means it\u0027s been converted and ignore the old one.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":206,"context_line":"                    if db_inst.get(\u0027policy\u0027):"},{"line_number":207,"context_line":"                        db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":208,"context_line":"                    elif db_inst.get(\u0027policies\u0027):"},{"line_number":209,"context_line":"                        db_policy \u003d api_models.InstanceGroupPolicy()"},{"line_number":210,"context_line":"                        db_policy.policy \u003d db_inst[\u0027policies\u0027][0]"},{"line_number":211,"context_line":"                        db_policy.group_id \u003d db_inst[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_15095483","line":208,"range":{"start_line":208,"start_character":20,"end_line":208,"end_character":49},"updated":"2018-06-27 14:45:39.000000000","message":"If L206 is False, how is this True? Since this comes from the model:\n\nhttps://review.openstack.org/#/c/563375/17/nova/db/sqlalchemy/api_models.py@447\n\nAnd that InstanceGroup.policies property is based on the policy attribute, which you\u0027ve already checked above.\n\nIn short, I think we can just work with api_models.InstanceGroup.policy here and drop any usage of the \u0027policies\u0027 property on the model:\n\nhttps://review.openstack.org/#/c/563375/17/nova/db/sqlalchemy/api_models.py@447\n\nIt makes this all overly complicated otherwise.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                        db_policy.rules \u003d None"},{"line_number":213,"context_line":"                    else:"},{"line_number":214,"context_line":"                        db_policy \u003d None"},{"line_number":215,"context_line":"                    inst \u003d instance_group_policy.InstanceGroupPolicy(context)"},{"line_number":216,"context_line":"                    instance_group[\u0027policy\u0027] \u003d inst._from_db_object("},{"line_number":217,"context_line":"                        context, instance_group_policy.InstanceGroupPolicy(),"},{"line_number":218,"context_line":"                        db_policy)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_981cb9c3","line":215,"range":{"start_line":215,"start_character":20,"end_line":215,"end_character":24},"updated":"2018-06-27 15:13:27.000000000","message":"Probably not the best name for something that is not an objects.Instance :)","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                        db_policy.rules \u003d None"},{"line_number":213,"context_line":"                    else:"},{"line_number":214,"context_line":"                        db_policy \u003d None"},{"line_number":215,"context_line":"                    inst \u003d instance_group_policy.InstanceGroupPolicy(context)"},{"line_number":216,"context_line":"                    instance_group[\u0027policy\u0027] \u003d inst._from_db_object("},{"line_number":217,"context_line":"                        context, instance_group_policy.InstanceGroupPolicy(),"},{"line_number":218,"context_line":"                        db_policy)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b185f50c","line":215,"range":{"start_line":215,"start_character":20,"end_line":215,"end_character":24},"in_reply_to":"5f7c97a3_7ea185a8","updated":"2018-06-28 15:30:23.000000000","message":"Done","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eaccc2ff9faf5f446c4853f97c4fff58ad04c7b4","unresolved":false,"context_lines":[{"line_number":212,"context_line":"                        db_policy.rules \u003d None"},{"line_number":213,"context_line":"                    else:"},{"line_number":214,"context_line":"                        db_policy \u003d None"},{"line_number":215,"context_line":"                    inst \u003d instance_group_policy.InstanceGroupPolicy(context)"},{"line_number":216,"context_line":"                    instance_group[\u0027policy\u0027] \u003d inst._from_db_object("},{"line_number":217,"context_line":"                        context, instance_group_policy.InstanceGroupPolicy(),"},{"line_number":218,"context_line":"                        db_policy)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_7ea185a8","line":215,"range":{"start_line":215,"start_character":20,"end_line":215,"end_character":24},"in_reply_to":"5f7c97a3_981cb9c3","updated":"2018-06-27 16:02:05.000000000","message":"Yeah, call it group_policy or policy_obj or something like that.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                    else:"},{"line_number":214,"context_line":"                        db_policy \u003d None"},{"line_number":215,"context_line":"                    inst \u003d instance_group_policy.InstanceGroupPolicy(context)"},{"line_number":216,"context_line":"                    instance_group[\u0027policy\u0027] \u003d inst._from_db_object("},{"line_number":217,"context_line":"                        context, instance_group_policy.InstanceGroupPolicy(),"},{"line_number":218,"context_line":"                        db_policy)"},{"line_number":219,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_95f8044b","line":216,"range":{"start_line":216,"start_character":34,"end_line":216,"end_character":44},"updated":"2018-06-27 14:45:39.000000000","message":"Please use object dot notation when accessing fields in the instance group object, see the TODO above about removing the NovaObjectDictCompat mixin.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                    else:"},{"line_number":214,"context_line":"                        db_policy \u003d None"},{"line_number":215,"context_line":"                    inst \u003d instance_group_policy.InstanceGroupPolicy(context)"},{"line_number":216,"context_line":"                    instance_group[\u0027policy\u0027] \u003d inst._from_db_object("},{"line_number":217,"context_line":"                        context, instance_group_policy.InstanceGroupPolicy(),"},{"line_number":218,"context_line":"                        db_policy)"},{"line_number":219,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_55eb4c7a","line":216,"range":{"start_line":216,"start_character":52,"end_line":216,"end_character":67},"updated":"2018-06-27 14:45:39.000000000","message":"See my comment here:\n\nhttps://review.openstack.org/#/c/573628/6/nova/objects/instance_group_policy.py@34\n\nI think you should handle db_policy being None here in the InstanceGroup code and just set instance_group.policy \u003d None appropriately.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                    else:"},{"line_number":214,"context_line":"                        db_policy \u003d None"},{"line_number":215,"context_line":"                    inst \u003d instance_group_policy.InstanceGroupPolicy(context)"},{"line_number":216,"context_line":"                    instance_group[\u0027policy\u0027] \u003d inst._from_db_object("},{"line_number":217,"context_line":"                        context, instance_group_policy.InstanceGroupPolicy(),"},{"line_number":218,"context_line":"                        db_policy)"},{"line_number":219,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_7123dd59","line":216,"range":{"start_line":216,"start_character":52,"end_line":216,"end_character":67},"in_reply_to":"5f7c97a3_55eb4c7a","updated":"2018-06-28 15:30:23.000000000","message":"Done","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                    else:"},{"line_number":214,"context_line":"                        db_policy \u003d None"},{"line_number":215,"context_line":"                    inst \u003d instance_group_policy.InstanceGroupPolicy(context)"},{"line_number":216,"context_line":"                    instance_group[\u0027policy\u0027] \u003d inst._from_db_object("},{"line_number":217,"context_line":"                        context, instance_group_policy.InstanceGroupPolicy(),"},{"line_number":218,"context_line":"                        db_policy)"},{"line_number":219,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_51f101de","line":216,"range":{"start_line":216,"start_character":34,"end_line":216,"end_character":44},"in_reply_to":"5f7c97a3_95f8044b","updated":"2018-06-28 15:30:23.000000000","message":"Done","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            raise exception.InstanceGroupNotFound(group_uuid\u003dgroup_uuid)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        values_copy \u003d copy.copy(values)"},{"line_number":274,"context_line":"        members \u003d values_copy.pop(\u0027members\u0027, None)"},{"line_number":275,"context_line":"        policy \u003d values_copy.pop(\u0027policy\u0027, None)"},{"line_number":276,"context_line":"        policies \u003d values_copy.pop(\u0027policies\u0027, None)"},{"line_number":277,"context_line":"        members \u003d values_copy.pop(\u0027members\u0027, None)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d5719ce1","line":274,"range":{"start_line":274,"start_character":8,"end_line":274,"end_character":50},"updated":"2018-06-27 14:45:39.000000000","message":"This is now redundant with L277.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            raise exception.InstanceGroupNotFound(group_uuid\u003dgroup_uuid)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"        values_copy \u003d copy.copy(values)"},{"line_number":274,"context_line":"        members \u003d values_copy.pop(\u0027members\u0027, None)"},{"line_number":275,"context_line":"        policy \u003d values_copy.pop(\u0027policy\u0027, None)"},{"line_number":276,"context_line":"        policies \u003d values_copy.pop(\u0027policies\u0027, None)"},{"line_number":277,"context_line":"        members \u003d values_copy.pop(\u0027members\u0027, None)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_71cc3d34","line":274,"range":{"start_line":274,"start_character":8,"end_line":274,"end_character":50},"in_reply_to":"5f7c97a3_d5719ce1","updated":"2018-06-28 15:30:23.000000000","message":"Done","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        grp.update(values_copy)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        if policies and not policy:"},{"line_number":282,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":283,"context_line":"                                                 group_id\u003dgrp.id,"},{"line_number":284,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":285,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_3546d07d","line":282,"updated":"2018-06-27 14:45:39.000000000","message":"I don\u0027t think we should support internal code setting InstanceGroup.policies anymore - everything should be using the policy field now, so I\u0027d raise an ObjectActionError here if the caller is setting policies rather than policy.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        grp.update(values_copy)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        if policies and not policy:"},{"line_number":282,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":283,"context_line":"                                                 group_id\u003dgrp.id,"},{"line_number":284,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":285,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_585dc17a","line":282,"in_reply_to":"5f7c97a3_3546d07d","updated":"2018-06-27 15:13:27.000000000","message":"You can\u0027t do that for something that is remotable, as the caller may have an older version and be doing a save() on it. You still have to honor the old field, but you do not have to save it in the old place. Since this is now api_db-only, we really shouldn\u0027t have that situation, but we also shouldn\u0027t break our contract.\n\nSo: Always save it in the new format, but check the object being saved here for potentially having data in the older field that needs to be saved in the new format.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        grp.update(values_copy)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        if policies and not policy:"},{"line_number":282,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":283,"context_line":"                                                 group_id\u003dgrp.id,"},{"line_number":284,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":285,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_f1292d9c","line":282,"in_reply_to":"5f7c97a3_585dc17a","updated":"2018-06-28 15:30:23.000000000","message":"Yes, and L281 \"if policies:\" is enough(for older version remote obj), because we don\u0027t have the case that we both have policies and policy.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        if policies and not policy:"},{"line_number":305,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":306,"context_line":"                                                 group_id\u003dgroup.id,"},{"line_number":307,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":308,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_7531a80a","line":305,"updated":"2018-06-27 14:45:39.000000000","message":"same as in _save_in_db","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"cc769954c7cc03f5b5da29a3f3d8a3554983cea5","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        if policies and not policy:"},{"line_number":305,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":306,"context_line":"                                                 group_id\u003dgroup.id,"},{"line_number":307,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":308,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b904fb15","line":305,"in_reply_to":"5f7c97a3_49a5f102","updated":"2018-06-28 15:30:23.000000000","message":"\"if policies\", ditto","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5e280e78c11eb2af37eaa6be969232be37ce0b99","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        if policies and not policy:"},{"line_number":305,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":306,"context_line":"                                                 group_id\u003dgroup.id,"},{"line_number":307,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":308,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_78f0bd67","line":305,"in_reply_to":"5f7c97a3_7531a80a","updated":"2018-06-27 15:13:27.000000000","message":"Same comment as save.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eaccc2ff9faf5f446c4853f97c4fff58ad04c7b4","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        if policies and not policy:"},{"line_number":305,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":306,"context_line":"                                                 group_id\u003dgroup.id,"},{"line_number":307,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":308,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_fec35538","line":305,"in_reply_to":"5f7c97a3_78f0bd67","updated":"2018-06-27 16:02:05.000000000","message":"If we make policies a readonly field, then you can\u0027t create a new instance of this object though with policies set, right?","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ce1aac924b69ac2b6f34838a8c6274ae7a8e224d","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        if policies and not policy:"},{"line_number":305,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":306,"context_line":"                                                 group_id\u003dgroup.id,"},{"line_number":307,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":308,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_49a5f102","line":305,"in_reply_to":"5f7c97a3_e90ddd36","updated":"2018-06-27 16:27:36.000000000","message":"Ah yeah, and I guess you also can if it\u0027s set via the object construction, so nevermind me.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c49f4aad663938bc1b34044e99cf2d3f635af906","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        if policies and not policy:"},{"line_number":305,"context_line":"            policy \u003d objects.InstanceGroupPolicy(context,"},{"line_number":306,"context_line":"                                                 group_id\u003dgroup.id,"},{"line_number":307,"context_line":"                                                 policy\u003dpolicies[0],"},{"line_number":308,"context_line":"                                                 rules\u003dNone)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_e90ddd36","line":305,"in_reply_to":"5f7c97a3_fec35538","updated":"2018-06-27 16:19:33.000000000","message":"You can, but also you definitely can if you\u0027re an older service without the readonly flag on your object and we arrive here via remotable :)","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eae27883189a9a7674de4bdaee53fd8d3ab348e4","unresolved":false,"context_lines":[{"line_number":462,"context_line":"        payload \u003d dict(updates)"},{"line_number":463,"context_line":"        updates.pop(\u0027id\u0027, None)"},{"line_number":464,"context_line":"        policy \u003d updates.pop(\u0027policy\u0027, None)"},{"line_number":465,"context_line":"        policies \u003d updates.pop(\u0027policies\u0027, None)"},{"line_number":466,"context_line":"        members \u003d updates.pop(\u0027members\u0027, None)"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"        if \u0027uuid\u0027 not in updates:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_150f143a","line":465,"updated":"2018-06-27 14:45:39.000000000","message":"I think we should just fail with ObjectActionError if \u0027policies\u0027 in updates - the field should no longer be used. Then we can remove/rename the \u0027policies\u0027 kwarg to _create_in_db.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ce1aac924b69ac2b6f34838a8c6274ae7a8e224d","unresolved":false,"context_lines":[{"line_number":462,"context_line":"        payload \u003d dict(updates)"},{"line_number":463,"context_line":"        updates.pop(\u0027id\u0027, None)"},{"line_number":464,"context_line":"        policy \u003d updates.pop(\u0027policy\u0027, None)"},{"line_number":465,"context_line":"        policies \u003d updates.pop(\u0027policies\u0027, None)"},{"line_number":466,"context_line":"        members \u003d updates.pop(\u0027members\u0027, None)"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"        if \u0027uuid\u0027 not in updates:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_e9a95dde","line":465,"in_reply_to":"5f7c97a3_150f143a","updated":"2018-06-27 16:27:36.000000000","message":"Maybe nevermind this based on the comments above.","commit_id":"01892aa904146f1f48e8d7a832b66106cd2dd033"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2e72cb3572f6a7a26e1ef2399064f75e1abd637","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":134,"context_line":"        if target_version \u003c (1, 11):"},{"line_number":135,"context_line":"            # NOTE(yikun): Before 1.11, we had a policies property"},{"line_number":136,"context_line":"            # which is the list of policy name."},{"line_number":137,"context_line":"            policy \u003d primitive.pop(\u0027policy\u0027, None)"},{"line_number":138,"context_line":"            if policy:"},{"line_number":139,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"}],"source_content_type":"text/x-python","patch_set":23,"id":"5f7c97a3_2cfa8c42","line":136,"range":{"start_line":136,"start_character":20,"end_line":136,"end_character":47},"updated":"2018-07-03 17:09:40.000000000","message":"nit: might be good to also mention that even though it was a list, there was ever only one entry in the list.","commit_id":"b5404041ec570f72e0b4302a364322ba7bb51981"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2e72cb3572f6a7a26e1ef2399064f75e1abd637","unresolved":false,"context_lines":[{"line_number":165,"context_line":"                      \u0027deleted_at\u0027: None}"},{"line_number":166,"context_line":"            if field in ignore and not hasattr(db_inst, field):"},{"line_number":167,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":168,"context_line":"            elif \u0027policies\u0027 \u003d\u003d field:"},{"line_number":169,"context_line":"                continue"},{"line_number":170,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":171,"context_line":"                if db_inst.get(\u0027policy\u0027):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5f7c97a3_cce9987f","line":168,"updated":"2018-07-03 17:09:40.000000000","message":"Might want to leave a comment about why we don\u0027t set the policies field.","commit_id":"b5404041ec570f72e0b4302a364322ba7bb51981"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"746772c2f596b4b3e3dc43db581d7aeb25ddf021","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":133,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":134,"context_line":"        if target_version \u003c (1, 11):"},{"line_number":135,"context_line":"            # NOTE(yikun): Before 1.11, we had a policies property which is"},{"line_number":136,"context_line":"            # the list of policy name, even though it was a list, there was"},{"line_number":137,"context_line":"            # ever only one entry in the list."},{"line_number":138,"context_line":"            policy \u003d primitive.pop(\u0027policy\u0027, None)"},{"line_number":139,"context_line":"            if policy:"},{"line_number":140,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":141,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":142,"context_line":"            else:"},{"line_number":143,"context_line":"                primitive[\u0027policies\u0027] \u003d []"},{"line_number":144,"context_line":"        if target_version \u003c (1, 7):"},{"line_number":145,"context_line":"            # NOTE(danms): Before 1.7, we had an always-empty"},{"line_number":146,"context_line":"            # metadetails property"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_92f6315e","line":143,"range":{"start_line":134,"start_character":8,"end_line":143,"end_character":42},"updated":"2018-07-05 16:26:04.000000000","message":"You should add a unit test for this conversion to assert that an InstanceGroup with policy set will correct backport and set InstanceGroup.policies. That\u0027s especially important for supporting Queens computes that are doing the late (anti)affinity check like in this change:\n\nhttps://review.openstack.org/#/c/571465/15/nova/compute/manager.py","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":133,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":134,"context_line":"        if target_version \u003c (1, 11):"},{"line_number":135,"context_line":"            # NOTE(yikun): Before 1.11, we had a policies property which is"},{"line_number":136,"context_line":"            # the list of policy name, even though it was a list, there was"},{"line_number":137,"context_line":"            # ever only one entry in the list."},{"line_number":138,"context_line":"            policy \u003d primitive.pop(\u0027policy\u0027, None)"},{"line_number":139,"context_line":"            if policy:"},{"line_number":140,"context_line":"                policy_name \u003d policy[\u0027nova_object.data\u0027][\u0027policy\u0027]"},{"line_number":141,"context_line":"                primitive[\u0027policies\u0027] \u003d [policy_name]"},{"line_number":142,"context_line":"            else:"},{"line_number":143,"context_line":"                primitive[\u0027policies\u0027] \u003d []"},{"line_number":144,"context_line":"        if target_version \u003c (1, 7):"},{"line_number":145,"context_line":"            # NOTE(danms): Before 1.7, we had an always-empty"},{"line_number":146,"context_line":"            # metadetails property"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_91e8627b","line":143,"range":{"start_line":134,"start_character":8,"end_line":143,"end_character":42},"in_reply_to":"5f7c97a3_92f6315e","updated":"2018-07-06 12:18:27.000000000","message":"Yes, thanks for reminder, I wrote it, but it is lost it in PS18.\n\nhttps://review.openstack.org/#/c/563375/17/nova/tests/unit/objects/test_instance_group.py@341\n\nI will bring it back in next PS","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":169,"context_line":"            elif \u0027policies\u0027 \u003d\u003d field:"},{"line_number":170,"context_line":"                continue"},{"line_number":171,"context_line":"            # Note(yikun): The obj.policies is deprecated and marked as"},{"line_number":172,"context_line":"            # read_only in version 1.11, and there is no \"policies\" property"},{"line_number":173,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":174,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is loaded."}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_43b1ae90","line":171,"range":{"start_line":171,"start_character":14,"end_line":171,"end_character":18},"updated":"2018-07-05 15:30:23.000000000","message":"nit: NOTE","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                instance_group[field] \u003d ignore[field]"},{"line_number":169,"context_line":"            elif \u0027policies\u0027 \u003d\u003d field:"},{"line_number":170,"context_line":"                continue"},{"line_number":171,"context_line":"            # Note(yikun): The obj.policies is deprecated and marked as"},{"line_number":172,"context_line":"            # read_only in version 1.11, and there is no \"policies\" property"},{"line_number":173,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":174,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is loaded."}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_ccbabdbe","line":171,"range":{"start_line":171,"start_character":14,"end_line":171,"end_character":18},"in_reply_to":"5f7c97a3_43b1ae90","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            # Note(yikun): The obj.policies is deprecated and marked as"},{"line_number":172,"context_line":"            # read_only in version 1.11, and there is no \"policies\" property"},{"line_number":173,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":174,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is loaded."},{"line_number":175,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":176,"context_line":"                if db_inst.get(\u0027policy\u0027):"},{"line_number":177,"context_line":"                    db_policy \u003d db_inst[\u0027policy\u0027]"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_e3a1da3b","line":174,"range":{"start_line":174,"start_character":71,"end_line":174,"end_character":77},"updated":"2018-07-05 15:30:23.000000000","message":"s/loaded/set/?","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            # Note(yikun): The obj.policies is deprecated and marked as"},{"line_number":172,"context_line":"            # read_only in version 1.11, and there is no \"policies\" property"},{"line_number":173,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":174,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is loaded."},{"line_number":175,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":176,"context_line":"                if db_inst.get(\u0027policy\u0027):"},{"line_number":177,"context_line":"                    db_policy \u003d db_inst[\u0027policy\u0027]"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_6cab6970","line":174,"range":{"start_line":174,"start_character":71,"end_line":174,"end_character":77},"in_reply_to":"5f7c97a3_e3a1da3b","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":173,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":174,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is loaded."},{"line_number":175,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":176,"context_line":"                if db_inst.get(\u0027policy\u0027):"},{"line_number":177,"context_line":"                    db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":178,"context_line":"                else:"},{"line_number":179,"context_line":"                    db_policy \u003d None"},{"line_number":180,"context_line":"                if db_policy:"},{"line_number":181,"context_line":"                    policy_obj \u003d objects.InstanceGroupPolicy(context)"},{"line_number":182,"context_line":"                    instance_group.policy \u003d policy_obj._from_db_object("}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_035df646","line":179,"range":{"start_line":176,"start_character":16,"end_line":179,"end_character":36},"updated":"2018-07-05 15:30:23.000000000","message":"nit: this could just be:\n\n  db_policy \u003d db_inst[\u0027policy\u0027]\n\nBecause it will always be either an InstanceGroupPolicy model object or None:\n\nhttps://review.openstack.org/#/c/579113/8/nova/db/sqlalchemy/api_models.py@456","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":173,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":174,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is loaded."},{"line_number":175,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":176,"context_line":"                if db_inst.get(\u0027policy\u0027):"},{"line_number":177,"context_line":"                    db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":178,"context_line":"                else:"},{"line_number":179,"context_line":"                    db_policy \u003d None"},{"line_number":180,"context_line":"                if db_policy:"},{"line_number":181,"context_line":"                    policy_obj \u003d objects.InstanceGroupPolicy(context)"},{"line_number":182,"context_line":"                    instance_group.policy \u003d policy_obj._from_db_object("}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_6ce049cc","line":179,"range":{"start_line":176,"start_character":16,"end_line":179,"end_character":36},"in_reply_to":"5f7c97a3_035df646","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                else:"},{"line_number":179,"context_line":"                    db_policy \u003d None"},{"line_number":180,"context_line":"                if db_policy:"},{"line_number":181,"context_line":"                    policy_obj \u003d objects.InstanceGroupPolicy(context)"},{"line_number":182,"context_line":"                    instance_group.policy \u003d policy_obj._from_db_object("},{"line_number":183,"context_line":"                        context, objects.InstanceGroupPolicy(),"},{"line_number":184,"context_line":"                        db_policy)"},{"line_number":185,"context_line":"                    instance_group.policies \u003d [instance_group.policy.policy]"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_0322d6ba","line":182,"range":{"start_line":181,"start_character":20,"end_line":182,"end_character":71},"updated":"2018-07-05 15:30:23.000000000","message":"nit: since _from_db_object is static:\n\nhttps://review.openstack.org/#/c/573628/14/nova/objects/instance_group.py@517\n\nYou can just do:\n\n  policy_obj \u003d objects.InstanceGroupPolicy._from_db_object(\n                        context, objects.InstanceGroupPolicy(),\n                        db_policy)\n\nOtherwise your policy_obj variable isn\u0027t even used.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                else:"},{"line_number":179,"context_line":"                    db_policy \u003d None"},{"line_number":180,"context_line":"                if db_policy:"},{"line_number":181,"context_line":"                    policy_obj \u003d objects.InstanceGroupPolicy(context)"},{"line_number":182,"context_line":"                    instance_group.policy \u003d policy_obj._from_db_object("},{"line_number":183,"context_line":"                        context, objects.InstanceGroupPolicy(),"},{"line_number":184,"context_line":"                        db_policy)"},{"line_number":185,"context_line":"                    instance_group.policies \u003d [instance_group.policy.policy]"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_ac76610b","line":182,"range":{"start_line":181,"start_character":20,"end_line":182,"end_character":71},"in_reply_to":"5f7c97a3_0322d6ba","updated":"2018-07-06 12:18:27.000000000","message":":), I remember I wrote this for pep8, I will change it to:\n\n     instance_group.policy \u003d objects.InstanceGroupPolicy.\\\n        _from_db_object(context, objects.InstanceGroupPolicy(),\n        db_policy)","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":257,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"        if policies:"},{"line_number":260,"context_line":"            policy \u003d api_models.InstanceGroupPolicy("},{"line_number":261,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicies[0], rules\u003dNone)"},{"line_number":262,"context_line":"            group._policies \u003d [policy]"},{"line_number":263,"context_line":"            group.save(context.session)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_7e4a415b","line":260,"range":{"start_line":260,"start_character":12,"end_line":260,"end_character":18},"updated":"2018-07-05 15:30:23.000000000","message":"can we rename this to db_policy please to avoid confusion with the policy kwarg","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":257,"context_line":"            raise exception.InstanceGroupIdExists(group_uuid\u003dvalues[\u0027uuid\u0027])"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"        if policies:"},{"line_number":260,"context_line":"            policy \u003d api_models.InstanceGroupPolicy("},{"line_number":261,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicies[0], rules\u003dNone)"},{"line_number":262,"context_line":"            group._policies \u003d [policy]"},{"line_number":263,"context_line":"            group.save(context.session)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_ecdd39de","line":260,"range":{"start_line":260,"start_character":12,"end_line":260,"end_character":18},"in_reply_to":"5f7c97a3_7e4a415b","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicies[0], rules\u003dNone)"},{"line_number":262,"context_line":"            group._policies \u003d [policy]"},{"line_number":263,"context_line":"            group.save(context.session)"},{"line_number":264,"context_line":"        elif policy:"},{"line_number":265,"context_line":"            policy \u003d api_models.InstanceGroupPolicy("},{"line_number":266,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicy.policy,"},{"line_number":267,"context_line":"                rules\u003dpolicy.rules_json)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_5e20a59a","line":264,"range":{"start_line":264,"start_character":13,"end_line":264,"end_character":19},"updated":"2018-07-05 15:30:23.000000000","message":"Is this an InstanceGroupPolicy object or a policy name string? Looks like it must be an InstanceGroupPolicy object, which makes the name shadowing on L265 really confusing.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicies[0], rules\u003dNone)"},{"line_number":262,"context_line":"            group._policies \u003d [policy]"},{"line_number":263,"context_line":"            group.save(context.session)"},{"line_number":264,"context_line":"        elif policy:"},{"line_number":265,"context_line":"            policy \u003d api_models.InstanceGroupPolicy("},{"line_number":266,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicy.policy,"},{"line_number":267,"context_line":"                rules\u003dpolicy.rules_json)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_cc971deb","line":264,"range":{"start_line":264,"start_character":13,"end_line":264,"end_character":19},"in_reply_to":"5f7c97a3_5e20a59a","updated":"2018-07-06 12:18:27.000000000","message":"Yes, This is an InstanceGroupPolicy","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":262,"context_line":"            group._policies \u003d [policy]"},{"line_number":263,"context_line":"            group.save(context.session)"},{"line_number":264,"context_line":"        elif policy:"},{"line_number":265,"context_line":"            policy \u003d api_models.InstanceGroupPolicy("},{"line_number":266,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicy.policy,"},{"line_number":267,"context_line":"                rules\u003dpolicy.rules_json)"},{"line_number":268,"context_line":"            group._policies \u003d [policy]"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_1e36ade8","line":265,"range":{"start_line":265,"start_character":12,"end_line":265,"end_character":18},"updated":"2018-07-05 15:30:23.000000000","message":"let\u0027s call this db_policy","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":262,"context_line":"            group._policies \u003d [policy]"},{"line_number":263,"context_line":"            group.save(context.session)"},{"line_number":264,"context_line":"        elif policy:"},{"line_number":265,"context_line":"            policy \u003d api_models.InstanceGroupPolicy("},{"line_number":266,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicy.policy,"},{"line_number":267,"context_line":"                rules\u003dpolicy.rules_json)"},{"line_number":268,"context_line":"            group._policies \u003d [policy]"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_8cb60589","line":265,"range":{"start_line":265,"start_character":12,"end_line":265,"end_character":18},"in_reply_to":"5f7c97a3_1e36ade8","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":269,"context_line":"        else:"},{"line_number":270,"context_line":"            group._policies \u003d []"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if group._policies:"},{"line_number":273,"context_line":"            group.save(context.session)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        if members:"},{"line_number":276,"context_line":"            group._members \u003d _instance_group_members_add(context, group,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_9e787d8f","line":273,"range":{"start_line":272,"start_character":8,"end_line":273,"end_character":39},"updated":"2018-07-05 15:30:23.000000000","message":"This would be redundant if we already saved on L263. Why wouldn\u0027t we just call policy.create() if policy is not None? Otherwise you\u0027re not using objects.InstanceGroupPolicy.create() anywhere.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":269,"context_line":"        else:"},{"line_number":270,"context_line":"            group._policies \u003d []"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if group._policies:"},{"line_number":273,"context_line":"            group.save(context.session)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        if members:"},{"line_number":276,"context_line":"            group._members \u003d _instance_group_members_add(context, group,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_ac73c1b3","line":273,"range":{"start_line":272,"start_character":8,"end_line":273,"end_character":39},"in_reply_to":"5f7c97a3_9e787d8f","updated":"2018-07-06 12:18:27.000000000","message":"Yeah, I think we need remove L263.\n\nFor create method, I tried to use create method before, but if we do it, the group._policies would NOT be refreshed, and then when we access _policies will trigger a lazy-load, right?\n\nBut if we do it as above, seems it\u0027s more effective, and your idea?\n\nBTW, the create method return the policy obj, and we need refresh the policy sqla model into group._policies","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0da9560b388a5aa5e01371fc7269049953803fd3","unresolved":false,"context_lines":[{"line_number":269,"context_line":"        else:"},{"line_number":270,"context_line":"            group._policies \u003d []"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if group._policies:"},{"line_number":273,"context_line":"            group.save(context.session)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        if members:"},{"line_number":276,"context_line":"            group._members \u003d _instance_group_members_add(context, group,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_d9a4bc4e","line":273,"range":{"start_line":272,"start_character":8,"end_line":273,"end_character":39},"in_reply_to":"5f7c97a3_ac73c1b3","updated":"2018-07-06 13:49:40.000000000","message":"Does anything use group._policies? Does it matter if we don\u0027t set it? I would just prefer to use InstanceGroupPolicy.create() if we have it, otherwise there isn\u0027t much point in a separate object.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb4a41155e53c34564191db0a4ab289490e32815","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":400,"context_line":"        payload \u003d dict(updates)"},{"line_number":401,"context_line":"        updates.pop(\u0027id\u0027, None)"},{"line_number":402,"context_line":"        policies \u003d updates.pop(\u0027policies\u0027, None)"},{"line_number":403,"context_line":"        policy \u003d updates.pop(\u0027policy\u0027, None)"},{"line_number":404,"context_line":"        members \u003d updates.pop(\u0027members\u0027, None)"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_f263e5db","line":402,"range":{"start_line":402,"start_character":8,"end_line":402,"end_character":48},"updated":"2018-07-05 16:02:30.000000000","message":"What happens if this is set but policy isn\u0027t?\n\nhttps://review.openstack.org/#/c/571166/16/nova/objects/request_spec.py@213\n\nThen we\u0027d have a group with no policy.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        updates \u003d self.obj_get_changes()"},{"line_number":400,"context_line":"        payload \u003d dict(updates)"},{"line_number":401,"context_line":"        updates.pop(\u0027id\u0027, None)"},{"line_number":402,"context_line":"        policies \u003d updates.pop(\u0027policies\u0027, None)"},{"line_number":403,"context_line":"        policy \u003d updates.pop(\u0027policy\u0027, None)"},{"line_number":404,"context_line":"        members \u003d updates.pop(\u0027members\u0027, None)"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_ec521936","line":402,"range":{"start_line":402,"start_character":8,"end_line":402,"end_character":48},"in_reply_to":"5f7c97a3_f263e5db","updated":"2018-07-06 12:18:27.000000000","message":"https://review.openstack.org/#/c/563375/28/nova/objects/instance_group.py@260\n\nyep, we will transfer policies to policy with no policy.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0da9560b388a5aa5e01371fc7269049953803fd3","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        else:"},{"line_number":267,"context_line":"            group._policies \u003d []"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        if group._policies:"},{"line_number":270,"context_line":"            group.save(context.session)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if members:"},{"line_number":273,"context_line":"            group._members \u003d _instance_group_members_add(context, group,"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_b965401f","line":270,"range":{"start_line":269,"start_character":7,"end_line":270,"end_character":39},"updated":"2018-07-06 13:49:40.000000000","message":"See comment in PS28 - I\u0027m not sure why we care about group._policies anymore - we have InstanceGroupPolicy.create() so we should just use that. I don\u0027t think anything else in this flow is uses group._policies for the DB model InstanceGroup object.","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"bca8897675ca35ea78b3c1b653b40f94d38f5c05","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        else:"},{"line_number":267,"context_line":"            group._policies \u003d []"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        if group._policies:"},{"line_number":270,"context_line":"            group.save(context.session)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if members:"},{"line_number":273,"context_line":"            group._members \u003d _instance_group_members_add(context, group,"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_de61eb12","line":270,"range":{"start_line":269,"start_character":7,"end_line":270,"end_character":39},"in_reply_to":"5f7c97a3_b965401f","updated":"2018-07-09 07:29:47.000000000","message":"I tried to use .create() directly, see https://review.openstack.org/#/c/580942/\n\nbut still have a problem on _policies:\n\nnova.tests.functional.db.test_instance_group.InstanceGroupObjectTestCase.test_create\n------------------------------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n      File \"nova/tests/functional/db/test_instance_group.py\", line 61, in test_create\n        create_group \u003d self._api_group()\n      File \"nova/tests/functional/db/test_instance_group.py\", line 42, in _api_group\n        group.create()\n      File \"/usr/local/lib/python2.7/dist-packages/oslo_versionedobjects/base.py\", line 226, in wrapper\n        return fn(self, *args, **kwargs)\n      File \"nova/objects/instance_group.py\", line 410, in create\n        self._from_db_object(self._context, self, db_group)\n      File \"nova/objects/instance_group.py\", line 176, in _from_db_object\n        db_policy \u003d db_inst[\u0027policy\u0027]\n      File \"/usr/local/lib/python2.7/dist-packages/oslo_db/sqlalchemy/models.py\", line 56, in __getitem__\n        return getattr(self, key)\n      File \"nova/db/sqlalchemy/api_models.py\", line 450, in policy\n        if len(self._policies) not in (0, 1):\n      File \"/usr/local/lib/python2.7/dist-packages/sqlalchemy/orm/attributes.py\", line 242, in __get__\n        return self.impl.get(instance_state(instance), dict_)\n      File \"/usr/local/lib/python2.7/dist-packages/sqlalchemy/orm/attributes.py\", line 599, in get\n        value \u003d self.callable_(state, passive)\n      File \"/usr/local/lib/python2.7/dist-packages/sqlalchemy/orm/strategies.py\", line 596, in _load_for_state\n        (orm_util.state_str(state), self.key)\n    sqlalchemy.orm.exc.DetachedInstanceError: Parent instance \u003cInstanceGroup at 0x7fda806a1d10\u003e is not bound to a Session; lazy load operation of attribute \u0027_policies\u0027 cannot proceed (Background on this error at: http://sqlalche.me/e/bhk3)\n\n\nand I\u0027m not sure the reason of it, it seems we need refresh _policies after we change the policy.\n\nSo, how about remove the create method in policy obj, and refresh _policies mannully, like we have done in this patch.","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"99d2d22fe89628ea6458d9b4ca2e34592cc8adab","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        else:"},{"line_number":267,"context_line":"            group._policies \u003d []"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        if group._policies:"},{"line_number":270,"context_line":"            group.save(context.session)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if members:"},{"line_number":273,"context_line":"            group._members \u003d _instance_group_members_add(context, group,"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_90b27c95","line":270,"range":{"start_line":269,"start_character":7,"end_line":270,"end_character":39},"in_reply_to":"5f7c97a3_de61eb12","updated":"2018-07-09 12:36:53.000000000","message":"You could maybe work around this by doing InstanceGroupPolicy.create() and then manually setting InstanceGroup._policies like you do above, but then there isn\u0027t much reason to have that InstanceGroupPolicy.create() method.","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2742f076229eb64fa75b5630fe54741ce9f9b3b3","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue, read_only\u003dTrue),"},{"line_number":127,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":129,"context_line":"        \u0027policy\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":130,"context_line":"        # NOTE(danms): Use rules not _rules for general access"},{"line_number":131,"context_line":"        \u0027_rules\u0027: fields.DictOfStringsField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_45e5f608","line":129,"range":{"start_line":129,"start_character":37,"end_line":129,"end_character":50},"updated":"2018-07-09 21:44:47.000000000","message":"Seems sort of weird that we\u0027d allow this to be nullable - a group should always have a policy.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d7a0d4422cfa2e107a2b6dbbc9a5b8245bf2c819","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue, read_only\u003dTrue),"},{"line_number":127,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":129,"context_line":"        \u0027policy\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":130,"context_line":"        # NOTE(danms): Use rules not _rules for general access"},{"line_number":131,"context_line":"        \u0027_rules\u0027: fields.DictOfStringsField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_5a9f10b2","line":129,"range":{"start_line":129,"start_character":37,"end_line":129,"end_character":50},"in_reply_to":"5f7c97a3_2576bad6","updated":"2018-07-10 02:00:55.000000000","message":"I guess we need to leave it nullable since some compat code sets it to None, like L205.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"47c1787adc79f105c8e23e43908ae2110741e84a","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue, read_only\u003dTrue),"},{"line_number":127,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":129,"context_line":"        \u0027policy\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":130,"context_line":"        # NOTE(danms): Use rules not _rules for general access"},{"line_number":131,"context_line":"        \u0027_rules\u0027: fields.DictOfStringsField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_067b84be","line":129,"range":{"start_line":129,"start_character":37,"end_line":129,"end_character":50},"in_reply_to":"5f7c97a3_2576bad6","updated":"2018-07-10 01:43:28.000000000","message":"sounds reasonable","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cd47c3bc7b35482f39e7b8f1f52b8bc72711dfe4","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue, read_only\u003dTrue),"},{"line_number":127,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":129,"context_line":"        \u0027policy\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":130,"context_line":"        # NOTE(danms): Use rules not _rules for general access"},{"line_number":131,"context_line":"        \u0027_rules\u0027: fields.DictOfStringsField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_2576bad6","line":129,"range":{"start_line":129,"start_character":37,"end_line":129,"end_character":50},"in_reply_to":"5f7c97a3_45e5f608","updated":"2018-07-09 22:12:06.000000000","message":"I guess this is here for compatibility because policies is nullable? I think I\u0027ve asked this before, but it seems we should just make this non-nullable.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"bc2465a4d0a245c0e70bfc8573ff71a6cd7947e1","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue, read_only\u003dTrue),"},{"line_number":127,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":129,"context_line":"        \u0027policy\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":130,"context_line":"        # NOTE(danms): Use rules not _rules for general access"},{"line_number":131,"context_line":"        \u0027_rules\u0027: fields.DictOfStringsField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_1d68b2b7","line":129,"range":{"start_line":129,"start_character":37,"end_line":129,"end_character":50},"in_reply_to":"5f7c97a3_5a9f10b2","updated":"2018-07-10 03:14:02.000000000","message":"Hmm, true, thanks for pointing out.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2461df5d766fcb2052af16c4a04ec9e4b51759dc","unresolved":false,"context_lines":[{"line_number":199,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":200,"context_line":"                db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":201,"context_line":"                if db_policy:"},{"line_number":202,"context_line":"                    instance_group.policy \u003d db_policy[\u0027policy\u0027]"},{"line_number":203,"context_line":"                    instance_group.policies \u003d [instance_group.policy]"},{"line_number":204,"context_line":"                else:"},{"line_number":205,"context_line":"                    instance_group.policy \u003d None"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_252b1a7c","line":202,"range":{"start_line":202,"start_character":20,"end_line":202,"end_character":63},"updated":"2018-07-09 21:29:30.000000000","message":"This is confusing and threw me at first because I thought this needed to be db_policy[\u0027policy\u0027][\u0027policy\u0027] but I see that on L200 we get the InstanceGroupPolicy DB model object and then here we\u0027re pulling the policy column off that, which is the policy name.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2461df5d766fcb2052af16c4a04ec9e4b51759dc","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            group._policies \u003d [db_policy]"},{"line_number":281,"context_line":"            group.rules \u003d None"},{"line_number":282,"context_line":"        elif policy:"},{"line_number":283,"context_line":"            db_rules \u003d jsonutils.dumps(rules) if rules else None"},{"line_number":284,"context_line":"            db_policy \u003d api_models.InstanceGroupPolicy("},{"line_number":285,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicy,"},{"line_number":286,"context_line":"                rules\u003ddb_rules)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_0545be43","line":283,"range":{"start_line":283,"start_character":55,"end_line":283,"end_character":64},"updated":"2018-07-09 21:29:30.000000000","message":"On L190 we default to {} so it seems we should do that here too, yeah? Then this could just be:\n\ndb_rules \u003d jsonutils.dumps(rules or {})\n\nAlternatively, make _from_db_object set _rules to None.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f9d2cc11fdc545198650233263ea79e75ed99cd8","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            group._policies \u003d [db_policy]"},{"line_number":281,"context_line":"            group.rules \u003d None"},{"line_number":282,"context_line":"        elif policy:"},{"line_number":283,"context_line":"            db_rules \u003d jsonutils.dumps(rules) if rules else None"},{"line_number":284,"context_line":"            db_policy \u003d api_models.InstanceGroupPolicy("},{"line_number":285,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicy,"},{"line_number":286,"context_line":"                rules\u003ddb_rules)"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_60d6e055","line":283,"range":{"start_line":283,"start_character":55,"end_line":283,"end_character":64},"in_reply_to":"5f7c97a3_0545be43","updated":"2018-07-09 22:24:41.000000000","message":"Ah yeah.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8b1e47c418adc659c0b1442ca08123f5fd8ef7d","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":129,"context_line":"        \u0027policy\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":130,"context_line":"        # NOTE(danms): Use rules not _rules for general access"},{"line_number":131,"context_line":"        \u0027_rules\u0027: fields.DictOfStringsField(nullable\u003dTrue),"},{"line_number":132,"context_line":"        }"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def __init__(self, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":35,"id":"5f7c97a3_1ecbba1c","line":131,"range":{"start_line":131,"start_character":44,"end_line":131,"end_character":57},"updated":"2018-07-10 16:27:30.000000000","message":"I think if we\u0027re going to default to {} if rules are None in the database then we should make this not nullable and force callers (the API and RequestSpec) to pass in {} if they don\u0027t want to specify rules (or just leave it not set in the object).","commit_id":"7788e200e8a75a309f43bf158ade40439dc525df"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8b1e47c418adc659c0b1442ca08123f5fd8ef7d","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def rules(self):"},{"line_number":141,"context_line":"        if \u0027_rules\u0027 in self and not self._rules:"},{"line_number":142,"context_line":"            return None"},{"line_number":143,"context_line":"        # NOTE(danms): Coerce our rules into a typed dict for convenience"},{"line_number":144,"context_line":"        rules \u003d {}"},{"line_number":145,"context_line":"        if \u0027max_server_per_host\u0027 in self._rules:"}],"source_content_type":"text/x-python","patch_set":35,"id":"5f7c97a3_ded8e2f0","line":142,"range":{"start_line":142,"start_character":12,"end_line":142,"end_character":23},"updated":"2018-07-10 16:27:30.000000000","message":"This is inconsistent with _from_db_object and _create_in_db which default to {} if rules are None in the database - if we make the _rules field nullable\u003dFalse, then this should also return {}.","commit_id":"7788e200e8a75a309f43bf158ade40439dc525df"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8b63ee0c556ef323ad82631da820d035879d30b9","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def rules(self):"},{"line_number":141,"context_line":"        if \u0027_rules\u0027 in self and not self._rules:"},{"line_number":142,"context_line":"            return None"},{"line_number":143,"context_line":"        # NOTE(danms): Coerce our rules into a typed dict for convenience"},{"line_number":144,"context_line":"        rules \u003d {}"},{"line_number":145,"context_line":"        if \u0027max_server_per_host\u0027 in self._rules:"}],"source_content_type":"text/x-python","patch_set":36,"id":"5f7c97a3_7c851658","line":142,"range":{"start_line":142,"start_character":12,"end_line":142,"end_character":23},"updated":"2018-07-10 17:33:47.000000000","message":"This is inconsistent with _from_db_object and _create_in_db which default to {} if rules are None in the database - if we make the _rules field nullable\u003dFalse, then this should also return {}.","commit_id":"187a7c84f6b85c7d12c2ce116b2a23f735d06e3b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def rules(self):"},{"line_number":141,"context_line":"        if \u0027_rules\u0027 not in self:"},{"line_number":142,"context_line":"            return {}"},{"line_number":143,"context_line":"        # NOTE(danms): Coerce our rules into a typed dict for convenience"},{"line_number":144,"context_line":"        rules \u003d {}"},{"line_number":145,"context_line":"        if \u0027max_server_per_host\u0027 in self._rules:"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_b230a008","line":142,"range":{"start_line":141,"start_character":0,"end_line":142,"end_character":21},"updated":"2018-07-11 15:56:02.000000000","message":"this could be avoided by defaulting the _rules field to {} above but then we would nee to call obj_set_defaults in __init__ so meh.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dfb9651058818bffedc7e323fd2b42c5a4a1957a","unresolved":false,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def rules(self):"},{"line_number":141,"context_line":"        if \u0027_rules\u0027 not in self:"},{"line_number":142,"context_line":"            return {}"},{"line_number":143,"context_line":"        # NOTE(danms): Coerce our rules into a typed dict for convenience"},{"line_number":144,"context_line":"        rules \u003d {}"},{"line_number":145,"context_line":"        if \u0027max_server_per_host\u0027 in self._rules:"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_a2ffe964","line":142,"range":{"start_line":141,"start_character":0,"end_line":142,"end_character":21},"in_reply_to":"5f7c97a3_b230a008","updated":"2018-07-11 18:54:42.000000000","message":"Yeah I\u0027d rather not set defaults in the field definition if we can avoid it.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            # NOTE(yikun): The obj.policies is deprecated and marked as"},{"line_number":196,"context_line":"            # read_only in version 1.11, and there is no \"policies\" property"},{"line_number":197,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":198,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is set."},{"line_number":199,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":200,"context_line":"                db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":201,"context_line":"                if db_policy:"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_9d7015aa","line":198,"range":{"start_line":198,"start_character":29,"end_line":198,"end_character":74},"updated":"2018-07-11 15:56:02.000000000","message":"I cannot fully parse this. But I guess it means that we set the policies field based on the value of the policy field.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dfb9651058818bffedc7e323fd2b42c5a4a1957a","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            # NOTE(yikun): The obj.policies is deprecated and marked as"},{"line_number":196,"context_line":"            # read_only in version 1.11, and there is no \"policies\" property"},{"line_number":197,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":198,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is set."},{"line_number":199,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":200,"context_line":"                db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":201,"context_line":"                if db_policy:"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_02eeddb4","line":198,"range":{"start_line":198,"start_character":29,"end_line":198,"end_character":74},"in_reply_to":"5f7c97a3_9d7015aa","updated":"2018-07-11 18:54:42.000000000","message":"Yes, it just means, for compat while InstanceGroup.policies is deprecated, to continue setting it based on the policy field value.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":198,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is set."},{"line_number":199,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":200,"context_line":"                db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":201,"context_line":"                if db_policy:"},{"line_number":202,"context_line":"                    instance_group.policy \u003d db_policy[\u0027policy\u0027]"},{"line_number":203,"context_line":"                    instance_group.policies \u003d [instance_group.policy]"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_dd4e8d38","line":200,"range":{"start_line":200,"start_character":28,"end_line":200,"end_character":45},"updated":"2018-07-11 15:56:02.000000000","message":"does this calls the policy property of the db object?","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dfb9651058818bffedc7e323fd2b42c5a4a1957a","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":198,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is set."},{"line_number":199,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":200,"context_line":"                db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":201,"context_line":"                if db_policy:"},{"line_number":202,"context_line":"                    instance_group.policy \u003d db_policy[\u0027policy\u0027]"},{"line_number":203,"context_line":"                    instance_group.policies \u003d [instance_group.policy]"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_e22ac1e4","line":200,"range":{"start_line":200,"start_character":28,"end_line":200,"end_character":45},"in_reply_to":"5f7c97a3_dd4e8d38","updated":"2018-07-11 18:54:42.000000000","message":"Yeah, it\u0027s a bit confusing, I tripped over this also:\n\nhttps://review.openstack.org/#/c/563375/34/nova/objects/instance_group.py@202","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e61f4b27793efdc2ec0310284628def474dd659d","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            # in InstanceGroup model anymore, so we just skip to set"},{"line_number":198,"context_line":"            # \"policies\" and then load the \"policies\" when \"policy\" is set."},{"line_number":199,"context_line":"            elif \u0027policy\u0027 \u003d\u003d field:"},{"line_number":200,"context_line":"                db_policy \u003d db_inst[\u0027policy\u0027]"},{"line_number":201,"context_line":"                if db_policy:"},{"line_number":202,"context_line":"                    instance_group.policy \u003d db_policy[\u0027policy\u0027]"},{"line_number":203,"context_line":"                    instance_group.policies \u003d [instance_group.policy]"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_5b459215","line":200,"range":{"start_line":200,"start_character":28,"end_line":200,"end_character":45},"in_reply_to":"5f7c97a3_e22ac1e4","updated":"2018-07-12 10:29:13.000000000","message":"Still feels strange that we use the db_inst as a dict and when we access a key that will access a property of the object. But it seems it works so I rest my case.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"}],"nova/objects/instance_group_policy.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4db0b6243ee5c510dcf690dd309c31470fe013e8","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    fields \u003d {"},{"line_number":25,"context_line":"        \u0027policy\u0027: fields.StringField(),"},{"line_number":26,"context_line":"        \u0027group_id\u0027: fields.IntegerField(),"},{"line_number":27,"context_line":"        \u0027rules\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":28,"context_line":"    }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_a2203b9d","line":27,"range":{"start_line":27,"start_character":24,"end_line":27,"end_character":35},"updated":"2018-05-18 09:18:33.000000000","message":"Will this store the json serialize form of? :\n\n    {\u0027max_servers_per_host\u0027: 3}\n\nIt feels strange that the the name of the field is plural while the value is a single string.","commit_id":"213b817da001bf26553ac1699e66b376ce72215d"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"d847b5b3552aaf7044ff9e6ae57dbed24070db40","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    fields \u003d {"},{"line_number":25,"context_line":"        \u0027policy\u0027: fields.StringField(),"},{"line_number":26,"context_line":"        \u0027group_id\u0027: fields.IntegerField(),"},{"line_number":27,"context_line":"        \u0027rules\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":28,"context_line":"    }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_8db2a393","line":27,"range":{"start_line":27,"start_character":24,"end_line":27,"end_character":35},"in_reply_to":"5f7c97a3_a2203b9d","updated":"2018-05-28 03:14:59.000000000","message":"Yeah, good point, and I think we should use DictOfStringsField in here.","commit_id":"213b817da001bf26553ac1699e66b376ce72215d"}],"nova/objects/request_spec.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8fb4c831f6551906bd89e5d3560a251fd1bc57cd","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            policies \u003d list(filter_properties.get(\u0027group_policies\u0027))"},{"line_number":211,"context_line":"            hosts \u003d list(filter_properties.get(\u0027group_hosts\u0027))"},{"line_number":212,"context_line":"            members \u003d list(filter_properties.get(\u0027group_members\u0027))"},{"line_number":213,"context_line":"            sgp \u003d objects.InstanceGroupPolicy(policy\u003dpolicies[0],rules\u003dNone)"},{"line_number":214,"context_line":"            self.instance_group \u003d objects.InstanceGroup(policies\u003dsgp,"},{"line_number":215,"context_line":"                                                        hosts\u003dhosts,"},{"line_number":216,"context_line":"                                                        members\u003dmembers)"}],"source_content_type":"text/x-python","patch_set":31,"id":"5f7c97a3_3ab34250","line":213,"range":{"start_line":213,"start_character":64,"end_line":213,"end_character":65},"updated":"2018-07-09 14:03:52.000000000","message":"This is going to fail pep8.\n\nAlso, unit tests are going to fail for this change (I already did this locally).","commit_id":"c0e2d8e0eb3d66c33fb46982d316b345d2d83bb1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a382f3dd53e93a1fac438c9af4b31294771426b0","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            policies \u003d list(filter_properties.get(\u0027group_policies\u0027))"},{"line_number":211,"context_line":"            hosts \u003d list(filter_properties.get(\u0027group_hosts\u0027))"},{"line_number":212,"context_line":"            members \u003d list(filter_properties.get(\u0027group_members\u0027))"},{"line_number":213,"context_line":"            sgp \u003d objects.InstanceGroupPolicy(policy\u003dpolicies[0],rules\u003dNone)"},{"line_number":214,"context_line":"            self.instance_group \u003d objects.InstanceGroup(policies\u003dsgp,"},{"line_number":215,"context_line":"                                                        hosts\u003dhosts,"},{"line_number":216,"context_line":"                                                        members\u003dmembers)"}],"source_content_type":"text/x-python","patch_set":31,"id":"5f7c97a3_9d0cd039","line":213,"range":{"start_line":213,"start_character":64,"end_line":213,"end_character":65},"in_reply_to":"5f7c97a3_3ab34250","updated":"2018-07-09 14:39:14.000000000","message":"Done","commit_id":"c0e2d8e0eb3d66c33fb46982d316b345d2d83bb1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"53fc43fb57028aad5de877962dd9adfc6941481c","unresolved":false,"context_lines":[{"line_number":213,"context_line":"            self.instance_group \u003d objects.InstanceGroup(policy\u003dpolicies[0],"},{"line_number":214,"context_line":"                                                        hosts\u003dhosts,"},{"line_number":215,"context_line":"                                                        members\u003dmembers,"},{"line_number":216,"context_line":"                                                        rules\u003d{})"},{"line_number":217,"context_line":"            # hosts has to be not part of the updates for saving the object"},{"line_number":218,"context_line":"            self.instance_group.obj_reset_changes([\u0027hosts\u0027])"},{"line_number":219,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":38,"id":"5f7c97a3_3cb55e7d","line":216,"range":{"start_line":216,"start_character":56,"end_line":216,"end_character":64},"updated":"2018-07-10 18:08:35.000000000","message":"nit: don\u0027t really need this now","commit_id":"251523aa19cdc3d5c771043bb773e8745194c07d"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"76846ced56788ecf124b581ff45ef9e1ae9fe8a1","unresolved":false,"context_lines":[{"line_number":213,"context_line":"            self.instance_group \u003d objects.InstanceGroup(policy\u003dpolicies[0],"},{"line_number":214,"context_line":"                                                        hosts\u003dhosts,"},{"line_number":215,"context_line":"                                                        members\u003dmembers,"},{"line_number":216,"context_line":"                                                        rules\u003d{})"},{"line_number":217,"context_line":"            # hosts has to be not part of the updates for saving the object"},{"line_number":218,"context_line":"            self.instance_group.obj_reset_changes([\u0027hosts\u0027])"},{"line_number":219,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":38,"id":"5f7c97a3_a47aacb8","line":216,"range":{"start_line":216,"start_character":56,"end_line":216,"end_character":64},"in_reply_to":"5f7c97a3_3cb55e7d","updated":"2018-07-11 02:06:19.000000000","message":"Done","commit_id":"251523aa19cdc3d5c771043bb773e8745194c07d"}],"nova/tests/functional/db/test_instance_group.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def _policy_comparator(self, db, obj):"},{"line_number":45,"context_line":"        self.assertEqual(db.id, obj.id)"},{"line_number":46,"context_line":"        self.assertEqual(db.rules, obj.rules)"},{"line_number":47,"context_line":"        self.assertEqual(db.group_id, obj.group_id)"},{"line_number":48,"context_line":"        self.assertEqual(db.policy, obj.policy)"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_beb1b947","line":46,"range":{"start_line":46,"start_character":25,"end_line":46,"end_character":33},"updated":"2018-07-05 15:30:23.000000000","message":"Shouldn\u0027t the model rules be serialized json strings and obj.rules being a deserialized dict? In other words, why is this passing unless rules is always None in these tests? Shouldn\u0027t this be obj.json_rules?","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.assertIsInstance(db_group.policy, api_models.InstanceGroupPolicy)"},{"line_number":55,"context_line":"        self.assertEqual(create_group.policies[0], db_group.policy.policy)"},{"line_number":56,"context_line":"        self.assertEqual(create_group.id, db_group.policy.group_id)"},{"line_number":57,"context_line":"        self.assertIsNone(db_group.policy.rules)"},{"line_number":58,"context_line":"        ovo_fixture.compare_obj("},{"line_number":59,"context_line":"            self, create_group, db_group,"},{"line_number":60,"context_line":"            allow_missing\u003d(\u0027deleted\u0027, \u0027deleted_at\u0027, \u0027policies\u0027),"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_7e81e14c","line":57,"range":{"start_line":57,"start_character":8,"end_line":57,"end_character":48},"updated":"2018-07-05 15:30:23.000000000","message":"I guess this answers my question above.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.assertIsInstance(db_group.policy, api_models.InstanceGroupPolicy)"},{"line_number":55,"context_line":"        self.assertEqual(create_group.policies[0], db_group.policy.policy)"},{"line_number":56,"context_line":"        self.assertEqual(create_group.id, db_group.policy.group_id)"},{"line_number":57,"context_line":"        self.assertIsNone(db_group.policy.rules)"},{"line_number":58,"context_line":"        ovo_fixture.compare_obj("},{"line_number":59,"context_line":"            self, create_group, db_group,"},{"line_number":60,"context_line":"            allow_missing\u003d(\u0027deleted\u0027, \u0027deleted_at\u0027, \u0027policies\u0027),"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_6c0da93e","line":57,"range":{"start_line":57,"start_character":8,"end_line":57,"end_character":48},"in_reply_to":"5f7c97a3_7e81e14c","updated":"2018-07-06 12:18:27.000000000","message":"Ha, yeah, - -, but I think I need fix it.","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0da9560b388a5aa5e01371fc7269049953803fd3","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        group.create()"},{"line_number":43,"context_line":"        return group"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _policy_obj_comparator(self, obj1, obj2):"},{"line_number":46,"context_line":"        self._policy_comparator(obj1, obj2, db_obj_compare\u003dFalse)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    def _policy_comparator(self, obj1, obj2, db_obj_compare\u003dTrue):"},{"line_number":49,"context_line":"        self.assertEqual(obj1.id, obj2.id)"},{"line_number":50,"context_line":"        if db_obj_compare:"},{"line_number":51,"context_line":"            self.assertEqual(obj1.rules,"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_590d0c3e","line":48,"range":{"start_line":45,"start_character":4,"end_line":48,"end_character":66},"updated":"2018-07-06 13:49:40.000000000","message":"This is a bit confusing, I\u0027m not sure why we need to make a distinction between when we need to compare the serialized policy.rules or not - is that really necessary and if so, please add a comment.","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"bca8897675ca35ea78b3c1b653b40f94d38f5c05","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        group.create()"},{"line_number":43,"context_line":"        return group"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _policy_obj_comparator(self, obj1, obj2):"},{"line_number":46,"context_line":"        self._policy_comparator(obj1, obj2, db_obj_compare\u003dFalse)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    def _policy_comparator(self, obj1, obj2, db_obj_compare\u003dTrue):"},{"line_number":49,"context_line":"        self.assertEqual(obj1.id, obj2.id)"},{"line_number":50,"context_line":"        if db_obj_compare:"},{"line_number":51,"context_line":"            self.assertEqual(obj1.rules,"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_7ec5373c","line":48,"range":{"start_line":45,"start_character":4,"end_line":48,"end_character":66},"in_reply_to":"5f7c97a3_590d0c3e","updated":"2018-07-09 07:29:47.000000000","message":"Sorry for unclear defination.\n\ndb_obj_compare\u003dFalse: is a obj and obj compare\ndb_obj_compare\u003dTrue: is a obj and db compare(and the db.rules is serialized policy.rules)\n\nI will add some note and rename the method.","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                                      project_id\u003dself.context.project_id,"},{"line_number":34,"context_line":"                                      name\u003d\u0027foogroup\u0027,"},{"line_number":35,"context_line":"                                      policy\u003d\u0027anti-affinity\u0027,"},{"line_number":36,"context_line":"                                      rules\u003d{\u0027max_server_per_host\u0027: 1},"},{"line_number":37,"context_line":"                                      members\u003d[\u0027memberfoo\u0027])"},{"line_number":38,"context_line":"        group.update(values)"},{"line_number":39,"context_line":"        group.create()"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_3dc78168","line":36,"range":{"start_line":36,"start_character":68,"end_line":36,"end_character":69},"updated":"2018-07-11 15:56:02.000000000","message":"The rules field (actually the _rules field) defined as DictOfString so it is strange to pass in an int value here. It works for some strange reasons in ovo but we should not rely on that.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e61f4b27793efdc2ec0310284628def474dd659d","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                                      project_id\u003dself.context.project_id,"},{"line_number":34,"context_line":"                                      name\u003d\u0027foogroup\u0027,"},{"line_number":35,"context_line":"                                      policy\u003d\u0027anti-affinity\u0027,"},{"line_number":36,"context_line":"                                      rules\u003d{\u0027max_server_per_host\u0027: 1},"},{"line_number":37,"context_line":"                                      members\u003d[\u0027memberfoo\u0027])"},{"line_number":38,"context_line":"        group.update(values)"},{"line_number":39,"context_line":"        group.create()"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_1b1cfac0","line":36,"range":{"start_line":36,"start_character":68,"end_line":36,"end_character":69},"in_reply_to":"5f7c97a3_02433db2","updated":"2018-07-12 10:29:13.000000000","message":"It feels dangerous to rely on the fact that DictOfString allows int value as key and converts it to string. But I guess I\u0027m alone with this fear.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dfb9651058818bffedc7e323fd2b42c5a4a1957a","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                                      project_id\u003dself.context.project_id,"},{"line_number":34,"context_line":"                                      name\u003d\u0027foogroup\u0027,"},{"line_number":35,"context_line":"                                      policy\u003d\u0027anti-affinity\u0027,"},{"line_number":36,"context_line":"                                      rules\u003d{\u0027max_server_per_host\u0027: 1},"},{"line_number":37,"context_line":"                                      members\u003d[\u0027memberfoo\u0027])"},{"line_number":38,"context_line":"        group.update(values)"},{"line_number":39,"context_line":"        group.create()"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_02433db2","line":36,"range":{"start_line":36,"start_character":68,"end_line":36,"end_character":69},"in_reply_to":"5f7c97a3_3dc78168","updated":"2018-07-11 18:54:42.000000000","message":"We can pass in whatever we want but it will be stored as a string since that\u0027s how DictOfStringsField works. Also, this is how the REST API is going to accept it because the REST API schema validation for positive_integer allows strings or integers. So I don\u0027t see a need to change this.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.assertEqual(create_group.id, db_group.policy.group_id)"},{"line_number":49,"context_line":"        ovo_fixture.compare_obj("},{"line_number":50,"context_line":"            self, create_group, db_group,"},{"line_number":51,"context_line":"            comparators\u003d{\u0027policy\u0027: lambda a, b: b \u003d\u003d a.policy},"},{"line_number":52,"context_line":"            allow_missing\u003d(\u0027deleted\u0027, \u0027deleted_at\u0027, \u0027policies\u0027, \u0027_rules\u0027))"},{"line_number":53,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 1}, create_group.rules)"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_ddf76d5a","line":51,"range":{"start_line":51,"start_character":12,"end_line":51,"end_character":63},"updated":"2018-07-11 15:56:02.000000000","message":"Why do we need this? The comparator does not call the policy property of the db object? I see that the comparator calls db_obj[db_key] but we also does the same thing in https://review.openstack.org/#/c/563375/39/nova/objects/instance_group.py@200 and I assume it works there and calls the propery somehow.\nOr I\u0027m totally lost in the property jungle.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e61f4b27793efdc2ec0310284628def474dd659d","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.assertEqual(create_group.id, db_group.policy.group_id)"},{"line_number":49,"context_line":"        ovo_fixture.compare_obj("},{"line_number":50,"context_line":"            self, create_group, db_group,"},{"line_number":51,"context_line":"            comparators\u003d{\u0027policy\u0027: lambda a, b: b \u003d\u003d a.policy},"},{"line_number":52,"context_line":"            allow_missing\u003d(\u0027deleted\u0027, \u0027deleted_at\u0027, \u0027policies\u0027, \u0027_rules\u0027))"},{"line_number":53,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 1}, create_group.rules)"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_db252218","line":51,"range":{"start_line":51,"start_character":12,"end_line":51,"end_character":63},"in_reply_to":"5f7c97a3_a28289c6","updated":"2018-07-12 10:29:13.000000000","message":"I tried the test without the comparator and fails in a way that proves your explanation.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dfb9651058818bffedc7e323fd2b42c5a4a1957a","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.assertEqual(create_group.id, db_group.policy.group_id)"},{"line_number":49,"context_line":"        ovo_fixture.compare_obj("},{"line_number":50,"context_line":"            self, create_group, db_group,"},{"line_number":51,"context_line":"            comparators\u003d{\u0027policy\u0027: lambda a, b: b \u003d\u003d a.policy},"},{"line_number":52,"context_line":"            allow_missing\u003d(\u0027deleted\u0027, \u0027deleted_at\u0027, \u0027policies\u0027, \u0027_rules\u0027))"},{"line_number":53,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 1}, create_group.rules)"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_a28289c6","line":51,"range":{"start_line":51,"start_character":12,"end_line":51,"end_character":63},"in_reply_to":"5f7c97a3_ddf76d5a","updated":"2018-07-11 18:54:42.000000000","message":"I believe this is needed because we\u0027re comparing the object which has InstanceGroup.policy which is a string against an InstanceGroupPolicy db model object which has policy as a column, so b is the InstanceGroup.policy object value and a.policy is the db model value.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            self, create_group, db_group,"},{"line_number":51,"context_line":"            comparators\u003d{\u0027policy\u0027: lambda a, b: b \u003d\u003d a.policy},"},{"line_number":52,"context_line":"            allow_missing\u003d(\u0027deleted\u0027, \u0027deleted_at\u0027, \u0027policies\u0027, \u0027_rules\u0027))"},{"line_number":53,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 1}, create_group.rules)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_destroy(self):"},{"line_number":56,"context_line":"        create_group \u003d self._api_group()"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_1d672555","line":53,"range":{"start_line":53,"start_character":49,"end_line":53,"end_character":50},"updated":"2018-07-11 15:56:02.000000000","message":"That is OK to expect an int here as the rules property converts it to int","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"}],"nova/tests/unit/objects/test_instance_group.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    \u0027project_id\u0027: \u0027fake_project\u0027,"},{"line_number":39,"context_line":"    \u0027name\u0027: \u0027fake_name\u0027,"},{"line_number":40,"context_line":"    # a group can only have 1 policy associated with it"},{"line_number":41,"context_line":"    \u0027policy\u0027: {\u0027policy\u0027: \u0027anti-affinity\u0027, \u0027group_id\u0027: 1, \u0027id\u0027: 1,"},{"line_number":42,"context_line":"               \u0027rules\u0027: \u0027{\"max_server_per_host\": \"1\"}\u0027},"},{"line_number":43,"context_line":"    \u0027members\u0027: [\u0027instance_id1\u0027, \u0027instance_id2\u0027],"},{"line_number":44,"context_line":"    \u0027created_at\u0027: _TS_NOW,"},{"line_number":45,"context_line":"    \u0027updated_at\u0027: _TS_NOW"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_fed97183","line":42,"range":{"start_line":41,"start_character":15,"end_line":42,"end_character":54},"updated":"2018-07-05 15:30:23.000000000","message":"nit: put these on separate lines for readability like _INST_GROUP_DB or define a separate dict:\n\n_INST_GROUP_POLICY_DB \u003d {\n    \u0027policy\u0027: \u0027anti-affinity\u0027,\n    \u0027group_id\u0027: 1,\n    \u0027id\u0027: 1,\n    \u0027rules\u0027: \u0027{\"max_server_per_host\": \"1\"}\u0027\n}\n\n_INST_GROUP_DB \u003d {\n    ....\n    \u0027policy\u0027: _INST_GROUP_POLICY_DB,\n}","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    \u0027project_id\u0027: \u0027fake_project\u0027,"},{"line_number":39,"context_line":"    \u0027name\u0027: \u0027fake_name\u0027,"},{"line_number":40,"context_line":"    # a group can only have 1 policy associated with it"},{"line_number":41,"context_line":"    \u0027policy\u0027: {\u0027policy\u0027: \u0027anti-affinity\u0027, \u0027group_id\u0027: 1, \u0027id\u0027: 1,"},{"line_number":42,"context_line":"               \u0027rules\u0027: \u0027{\"max_server_per_host\": \"1\"}\u0027},"},{"line_number":43,"context_line":"    \u0027members\u0027: [\u0027instance_id1\u0027, \u0027instance_id2\u0027],"},{"line_number":44,"context_line":"    \u0027created_at\u0027: _TS_NOW,"},{"line_number":45,"context_line":"    \u0027updated_at\u0027: _TS_NOW"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_a002e5b8","line":42,"range":{"start_line":41,"start_character":15,"end_line":42,"end_character":54},"in_reply_to":"5f7c97a3_fed97183","updated":"2018-07-06 12:18:27.000000000","message":"Done","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08636eb8b602eff0f0f48c1f61b096d87d7e09e1","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def _return_group_obj(context):"},{"line_number":50,"context_line":"    group_db \u003d copy.deepcopy(_INST_GROUP_DB)"},{"line_number":51,"context_line":"    policy \u003d objects.InstanceGroupPolicy(group_db.pop(\u0027policy\u0027))"},{"line_number":52,"context_line":"    group_obj \u003d objects.InstanceGroup(context, **group_db)"},{"line_number":53,"context_line":"    group_obj.policy \u003d policy"},{"line_number":54,"context_line":"    return group_obj"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_9e395da4","line":51,"range":{"start_line":51,"start_character":41,"end_line":51,"end_character":63},"updated":"2018-07-05 15:30:23.000000000","message":"I doubt this is what you expect it is - the first parameter to init is a RequestContext...\n\nhttps://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L303\n\nSo this should probably be:\n\npolicy \u003d objects.InstanceGroupPolicy(context, **group_db[\u0027policy\u0027])","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"764a39bf0f44a39d1256c64671ed4bb0ff44cb62","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def _return_group_obj(context):"},{"line_number":50,"context_line":"    group_db \u003d copy.deepcopy(_INST_GROUP_DB)"},{"line_number":51,"context_line":"    policy \u003d objects.InstanceGroupPolicy(group_db.pop(\u0027policy\u0027))"},{"line_number":52,"context_line":"    group_obj \u003d objects.InstanceGroup(context, **group_db)"},{"line_number":53,"context_line":"    group_obj.policy \u003d policy"},{"line_number":54,"context_line":"    return group_obj"}],"source_content_type":"text/x-python","patch_set":28,"id":"5f7c97a3_c007e1c8","line":51,"range":{"start_line":51,"start_character":41,"end_line":51,"end_character":63},"in_reply_to":"5f7c97a3_9e395da4","updated":"2018-07-06 12:18:27.000000000","message":"Yes","commit_id":"02ac8285317ca73085cf49d1b3a7b9c2e61c3fe8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0da9560b388a5aa5e01371fc7269049953803fd3","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def _return_group_obj(context):"},{"line_number":55,"context_line":"    group_db \u003d copy.deepcopy(_INST_GROUP_DB)"},{"line_number":56,"context_line":"    group_db[\u0027policy\u0027][\u0027rules\u0027] \u003d jsonutils.loads(group_db[\u0027policy\u0027][\u0027rules\u0027])"},{"line_number":57,"context_line":"    policy \u003d objects.InstanceGroupPolicy(context, **group_db[\u0027policy\u0027])"},{"line_number":58,"context_line":"    group_db[\u0027policy\u0027] \u003d policy"},{"line_number":59,"context_line":"    group_obj \u003d objects.InstanceGroup(context, **group_db)"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_5402b309","line":56,"range":{"start_line":56,"start_character":33,"end_line":56,"end_character":78},"updated":"2018-07-06 13:49:40.000000000","message":"This threw me off but I guess it\u0027s needed because InstanceGroupPolicy._rules is back to being a DictOfStringsField so we have to pass a dict in, not a serialized dict.","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"bca8897675ca35ea78b3c1b653b40f94d38f5c05","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def _return_group_obj(context):"},{"line_number":55,"context_line":"    group_db \u003d copy.deepcopy(_INST_GROUP_DB)"},{"line_number":56,"context_line":"    group_db[\u0027policy\u0027][\u0027rules\u0027] \u003d jsonutils.loads(group_db[\u0027policy\u0027][\u0027rules\u0027])"},{"line_number":57,"context_line":"    policy \u003d objects.InstanceGroupPolicy(context, **group_db[\u0027policy\u0027])"},{"line_number":58,"context_line":"    group_db[\u0027policy\u0027] \u003d policy"},{"line_number":59,"context_line":"    group_obj \u003d objects.InstanceGroup(context, **group_db)"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_fede270e","line":56,"range":{"start_line":56,"start_character":33,"end_line":56,"end_character":78},"in_reply_to":"5f7c97a3_5402b309","updated":"2018-07-09 07:29:47.000000000","message":"Yes","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0da9560b388a5aa5e01371fc7269049953803fd3","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        dbs \u003d [_INST_GROUP_DB, none_policy_group]"},{"line_number":309,"context_line":"        data \u003d lambda x: x[\u0027nova_object.data\u0027]"},{"line_number":310,"context_line":"        for db in dbs:"},{"line_number":311,"context_line":"            ig \u003d objects.InstanceGroup(self.context)"},{"line_number":312,"context_line":"            obj \u003d ig._from_db_object(self.context, ig, db)"},{"line_number":313,"context_line":"            # Latest version obj has policy and policies"},{"line_number":314,"context_line":"            obj_primitive \u003d obj.obj_to_primitive()"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_541b936d","line":311,"range":{"start_line":311,"start_character":39,"end_line":311,"end_character":51},"updated":"2018-07-06 13:49:40.000000000","message":"nit: this isn\u0027t necessary","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"bca8897675ca35ea78b3c1b653b40f94d38f5c05","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        dbs \u003d [_INST_GROUP_DB, none_policy_group]"},{"line_number":309,"context_line":"        data \u003d lambda x: x[\u0027nova_object.data\u0027]"},{"line_number":310,"context_line":"        for db in dbs:"},{"line_number":311,"context_line":"            ig \u003d objects.InstanceGroup(self.context)"},{"line_number":312,"context_line":"            obj \u003d ig._from_db_object(self.context, ig, db)"},{"line_number":313,"context_line":"            # Latest version obj has policy and policies"},{"line_number":314,"context_line":"            obj_primitive \u003d obj.obj_to_primitive()"}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_befd0f6e","line":311,"range":{"start_line":311,"start_character":39,"end_line":311,"end_character":51},"in_reply_to":"5f7c97a3_541b936d","updated":"2018-07-09 07:29:47.000000000","message":"Done","commit_id":"5f10c4489cb530b45788c4193b8e5c5aab838fff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea1822f27b3a8fd20b9e61da1e524b4a96a9be79","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    rules\u003djsonutils.loads(_INST_GROUP_POLICY_DB[\u0027rules\u0027]))"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def _return_group_obj(context):"},{"line_number":60,"context_line":"    group_db \u003d copy.deepcopy(_INST_GROUP_DB)"},{"line_number":61,"context_line":"    group_db[\u0027policy\u0027][\u0027rules\u0027] \u003d jsonutils.loads(group_db[\u0027policy\u0027][\u0027rules\u0027])"},{"line_number":62,"context_line":"    policy \u003d objects.InstanceGroupPolicy(context, **group_db[\u0027policy\u0027])"},{"line_number":63,"context_line":"    group_db[\u0027policy\u0027] \u003d policy"},{"line_number":64,"context_line":"    group_obj \u003d objects.InstanceGroup(context, **group_db)"},{"line_number":65,"context_line":"    return group_obj"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class _TestInstanceGroupObject(object):"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_a5a88a8a","line":65,"range":{"start_line":59,"start_character":0,"end_line":65,"end_character":20},"updated":"2018-07-09 21:47:33.000000000","message":"This is unused now.","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"76846ced56788ecf124b581ff45ef9e1ae9fe8a1","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    rules\u003djsonutils.loads(_INST_GROUP_POLICY_DB[\u0027rules\u0027]))"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def _return_group_obj(context):"},{"line_number":60,"context_line":"    group_db \u003d copy.deepcopy(_INST_GROUP_DB)"},{"line_number":61,"context_line":"    group_db[\u0027policy\u0027][\u0027rules\u0027] \u003d jsonutils.loads(group_db[\u0027policy\u0027][\u0027rules\u0027])"},{"line_number":62,"context_line":"    policy \u003d objects.InstanceGroupPolicy(context, **group_db[\u0027policy\u0027])"},{"line_number":63,"context_line":"    group_db[\u0027policy\u0027] \u003d policy"},{"line_number":64,"context_line":"    group_obj \u003d objects.InstanceGroup(context, **group_db)"},{"line_number":65,"context_line":"    return group_obj"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"class _TestInstanceGroupObject(object):"}],"source_content_type":"text/x-python","patch_set":34,"id":"5f7c97a3_faba9cb7","line":65,"range":{"start_line":59,"start_character":0,"end_line":65,"end_character":20},"in_reply_to":"5f7c97a3_a5a88a8a","updated":"2018-07-11 02:06:19.000000000","message":"Done","commit_id":"f67a2f2cc3eac6ca3a450e5fec6f6a1adb7d5ecc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e55985b45c39490c21833a235300472137d1acd2","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        self.assertEqual({}, obj.rules)"},{"line_number":82,"context_line":"        self.assertIn(\u0027_rules\u0027, obj)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    @mock.patch(\u0027nova.objects.InstanceGroup._get_from_db_by_instance\u0027,"},{"line_number":86,"context_line":"                return_value\u003d_INST_GROUP_DB)"},{"line_number":87,"context_line":"    def test_get_by_instance_uuid(self, mock_api_get):"}],"source_content_type":"text/x-python","patch_set":37,"id":"5f7c97a3_3cda3e49","line":84,"updated":"2018-07-10 17:44:28.000000000","message":"too many blank lines","commit_id":"dd14ee5984535f8783db344f600e800781c88927"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3cbf89eb61c34f7c1a16acc54f7ca0658bd5ef15","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        self.assertEqual({}, obj.rules)"},{"line_number":82,"context_line":"        self.assertIn(\u0027_rules\u0027, obj)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    @mock.patch(\u0027nova.objects.InstanceGroup._get_from_db_by_instance\u0027,"},{"line_number":86,"context_line":"                return_value\u003d_INST_GROUP_DB)"},{"line_number":87,"context_line":"    def test_get_by_instance_uuid(self, mock_api_get):"}],"source_content_type":"text/x-python","patch_set":37,"id":"5f7c97a3_fc8da634","line":84,"in_reply_to":"5f7c97a3_3cda3e49","updated":"2018-07-10 17:47:11.000000000","message":"Not my fault! But I shall fix :)","commit_id":"dd14ee5984535f8783db344f600e800781c88927"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027project_id\u0027], obj.project_id)"},{"line_number":71,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027user_id\u0027], obj.user_id)"},{"line_number":72,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027name\u0027], obj.name)"},{"line_number":73,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 2},"},{"line_number":74,"context_line":"                         obj.rules)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_rules_helper(self):"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_9d46d57e","line":73,"updated":"2018-07-11 15:56:02.000000000","message":"this is the same assert as L68","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"6d04b693d2d05c882ef04636a83770789a91e1cd","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027project_id\u0027], obj.project_id)"},{"line_number":71,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027user_id\u0027], obj.user_id)"},{"line_number":72,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027name\u0027], obj.name)"},{"line_number":73,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 2},"},{"line_number":74,"context_line":"                         obj.rules)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_rules_helper(self):"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_3cdf79d8","line":73,"in_reply_to":"5f7c97a3_9d46d57e","updated":"2018-07-12 01:33:18.000000000","message":"Done","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d8d9896a19506c6202705868f8dc2d7498727ce","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027user_id\u0027], obj.user_id)"},{"line_number":72,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027name\u0027], obj.name)"},{"line_number":73,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 2},"},{"line_number":74,"context_line":"                         obj.rules)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_rules_helper(self):"},{"line_number":77,"context_line":"        obj \u003d objects.InstanceGroup()"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_5d403d60","line":74,"updated":"2018-07-11 15:56:02.000000000","message":"we should assert policies and policy fields","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dfb9651058818bffedc7e323fd2b42c5a4a1957a","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027user_id\u0027], obj.user_id)"},{"line_number":72,"context_line":"        self.assertEqual(_INST_GROUP_DB[\u0027name\u0027], obj.name)"},{"line_number":73,"context_line":"        self.assertEqual({\u0027max_server_per_host\u0027: 2},"},{"line_number":74,"context_line":"                         obj.rules)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def test_rules_helper(self):"},{"line_number":77,"context_line":"        obj \u003d objects.InstanceGroup()"}],"source_content_type":"text/x-python","patch_set":39,"id":"5f7c97a3_0271fd0d","line":74,"in_reply_to":"5f7c97a3_5d403d60","updated":"2018-07-11 18:54:42.000000000","message":"So, leave L68 untouched and add a line for checking the policy field.","commit_id":"5027b4a63e4768ca9ebe79e769873788d7a5365f"}]}