)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7679adcf5a2130f6228d7fcb33ea3bd9194a4af9","unresolved":false,"context_lines":[{"line_number":10,"context_line":"ONE to MANY model in group model, this patch change it to ONE"},{"line_number":11,"context_line":"to ONE Model."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"We also adapt create method to new model, and remove policy"},{"line_number":14,"context_line":"update support in group.save because now we don\u0027t support to"},{"line_number":15,"context_line":"update a policy of the group."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_894192a9","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":29},"updated":"2018-06-29 14:27:54.000000000","message":"Hmm, I\u0027m not sure if we should do this - it changes the behavior of the object. It\u0027s true that we don\u0027t have a PUT (update) API on server groups and when creating a server group, exactly 1 policy is required and used during create():\n\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/server_groups.py#L169\n\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/server_groups.py#L38\n\nI don\u0027t know, I\u0027m pretty sure Dan might be against this change, but we can ask.","commit_id":"81e34a47c70d06adf82ba9f88ed29ba9f1c28312"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"5292e9bb9e1e72ce45c668f5280e524ce05ee64c","unresolved":false,"context_lines":[{"line_number":10,"context_line":"ONE to MANY model in group model, this patch change it to ONE"},{"line_number":11,"context_line":"to ONE Model."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"We also adapt create method to new model, and remove policy"},{"line_number":14,"context_line":"update support in group.save because now we don\u0027t support to"},{"line_number":15,"context_line":"update a policy of the group."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_e661473a","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":29},"in_reply_to":"5f7c97a3_894192a9","updated":"2018-06-30 01:27:52.000000000","message":"emm, I try to keep same behavior using policy, but I found the old \n_instance_group_model_add, only append the new policy [1](add new, do NOT remove existing), but the expected behavior is remove old and add new.\n\nand then, I remove the wrong code(create and save), only add the code we needed (create).\n\n[1] https://review.openstack.org/#/c/579113/1/nova/objects/instance_group.py@a60","commit_id":"81e34a47c70d06adf82ba9f88ed29ba9f1c28312"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"8ebe8356b2b1ae168ad71df7d3652d232878e700","unresolved":false,"context_lines":[{"line_number":10,"context_line":"ONE to MANY model in group model, this patch change it to ONE"},{"line_number":11,"context_line":"to ONE Model."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"We also adapt create method to new model, and remove policy"},{"line_number":14,"context_line":"update support in group.save because now we don\u0027t support to"},{"line_number":15,"context_line":"update a policy of the group."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_a65b4f84","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":29},"in_reply_to":"5f7c97a3_e661473a","updated":"2018-06-30 01:30:01.000000000","message":"it\u0027s true we change the object behavior, so maybe need a version bump or something.","commit_id":"81e34a47c70d06adf82ba9f88ed29ba9f1c28312"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d7c916604465003bbd9cf272dccc3c8870764c4d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yikun Jiang \u003cyikunkero@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-03 16:07:33 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor the policies to policy"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"A group can only have 1 policy associated it, but now we have"},{"line_number":10,"context_line":"ONE to MANY model in group model, this patch change it to ONE"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5f7c97a3_dde5c43f","line":7,"updated":"2018-07-03 19:19:12.000000000","message":"nit: a title with a bit more context would be:\n\n\"Refactor policies to policy in InstanceGroup DB model\"","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"44ff436c89f2463928a46b7bc193c6277c4a33d7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yikun Jiang \u003cyikunkero@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-03 16:07:33 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor the policies to policy"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"A group can only have 1 policy associated it, but now we have"},{"line_number":10,"context_line":"ONE to MANY model in group model, this patch change it to ONE"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5f7c97a3_d9c4f310","line":7,"in_reply_to":"5f7c97a3_dde5c43f","updated":"2018-07-04 04:02:23.000000000","message":"Done","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d7c916604465003bbd9cf272dccc3c8870764c4d","unresolved":false,"context_lines":[{"line_number":10,"context_line":"ONE to MANY model in group model, this patch change it to ONE"},{"line_number":11,"context_line":"to ONE Model."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"We also adapt create method to new model, and remove policy"},{"line_number":14,"context_line":"update support in group.save because now we don\u0027t support to"},{"line_number":15,"context_line":"update a policy of the group."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Related to blueprint complex-anti-affinity-policies"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5f7c97a3_7df030db","line":15,"range":{"start_line":13,"start_character":42,"end_line":15,"end_character":29},"updated":"2018-07-03 19:19:12.000000000","message":"Even though the REST API creates a group with the given policy, and there can only be one, and we don\u0027t have any kind of update group policy API, I\u0027m not sure that we need to change the behavior of the InstanceGroup.save() method in the versioned object. I mean, it doesn\u0027t really hurt anything internally since it\u0027s not used, but it\u0027s not necessary for this series either, correct? We could just as easily leave a TODO to remove it when we remove the \u0027policies\u0027 field in a major version bump. Or does leaving policies support in save() mean you have to change the model code to also save those policies on the group - which you\u0027ve also removed in this change?","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"}],"nova/db/sqlalchemy/api_models.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7679adcf5a2130f6228d7fcb33ea3bd9194a4af9","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":2,"id":"5f7c97a3_a9f56e86","line":447,"range":{"start_line":447,"start_character":8,"end_line":447,"end_character":16},"updated":"2018-06-29 14:27:54.000000000","message":"I guess this is still used in _from_db_object to set InstanceGroup.policies.","commit_id":"81e34a47c70d06adf82ba9f88ed29ba9f1c28312"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"5292e9bb9e1e72ce45c668f5280e524ce05ee64c","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":2,"id":"5f7c97a3_a6826fc6","line":447,"range":{"start_line":447,"start_character":8,"end_line":447,"end_character":16},"in_reply_to":"5f7c97a3_a9f56e86","updated":"2018-06-30 01:27:52.000000000","message":"Yeah, and also I\u0027m going to remove this in next follow patch.","commit_id":"81e34a47c70d06adf82ba9f88ed29ba9f1c28312"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2f6a366d419781c1892249a51ed8913a2ae44a12","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":4,"id":"5f7c97a3_3d4078a2","line":442,"range":{"start_line":442,"start_character":12,"end_line":442,"end_character":25},"updated":"2018-07-03 19:38:17.000000000","message":"I think this is dangerous. We never create multiple policies in the DB right now, but if there are multiple for some reason (which the schema and some of our internal interfaces will allow), I\u0027m not sure how this will behave.\n\nWhy not just leave this as \"_policies\" and snip off the first one each time like you\u0027re doing in create() currently in policy() property?\n\nUntil the schema enforces that there are no duplicates I don\u0027t really see how we can safely change the actual model underneath. Making policies() always return a list of one, and policy() return the first one makes total sense to me though.","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"46c5ce33074b37f072c8b127066a1c477331471e","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":4,"id":"5f7c97a3_5d00d435","line":442,"range":{"start_line":442,"start_character":12,"end_line":442,"end_character":25},"in_reply_to":"5f7c97a3_3d4078a2","updated":"2018-07-03 19:45:05.000000000","message":"agree IMO if you don\u0027t have unique constraint on InstanceGroupPolicy.group_id you can\u0027t be assured this is one-to-one and SQLA will raise an error if it ever loads more than one.  What you do here is create a @property called \"policy\" that returns _policies[0] if present and if you\u0027d like have it emit a warning if len(_policies) not in (0, 1).","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"44ff436c89f2463928a46b7bc193c6277c4a33d7","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":4,"id":"5f7c97a3_7f9d7f55","line":442,"range":{"start_line":442,"start_character":12,"end_line":442,"end_character":25},"in_reply_to":"5f7c97a3_5d00d435","updated":"2018-07-04 04:02:23.000000000","message":"Done, and thanks much for your advice.","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d7c916604465003bbd9cf272dccc3c8870764c4d","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":4,"id":"5f7c97a3_fdd1a057","line":447,"updated":"2018-07-03 19:19:12.000000000","message":"OK this gets removed in the next patch:\n\nhttps://review.openstack.org/#/c/563375/23/nova/db/sqlalchemy/api_models.py","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"e26f5f8ebb161aca22e937d14039a9b9892215f4","unresolved":false,"context_lines":[{"line_number":450,"context_line":"        if len(self._policies) in (0, 1):"},{"line_number":451,"context_line":"            return self._policies[0] if self._policies else None"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"        msg \u003d (\"More than one policy (%(policies)s) is associated with\""},{"line_number":454,"context_line":"               \" group %(group_name)s.\")"},{"line_number":455,"context_line":"        LOG.warning(msg, {\"policies\": self.policies,"},{"line_number":456,"context_line":"                          \"group_name\": self.name})"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    @property"},{"line_number":459,"context_line":"    def policies(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_51c0b74a","line":456,"range":{"start_line":453,"start_character":8,"end_line":456,"end_character":51},"updated":"2018-07-04 07:41:51.000000000","message":"Shouldn\u0027t you return something and then log?","commit_id":"297f4c9fb902daebf6fb351a135f17da90dc210f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd46e73ad24efe4fa507e82886fd0304f1cc25ea","unresolved":false,"context_lines":[{"line_number":451,"context_line":"            msg \u003d (\"More than one policy (%(policies)s) is associated with \""},{"line_number":452,"context_line":"                   \"group %(group_name)s, only the first one in the list \""},{"line_number":453,"context_line":"                   \"would be returned.\")"},{"line_number":454,"context_line":"            LOG.warning(msg, {\"policies\": [p.policy for p in self._policies],"},{"line_number":455,"context_line":"                              \"group_name\": self.name})"},{"line_number":456,"context_line":"        return self._policies[0] if self._policies else None"},{"line_number":457,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_636e2ac9","line":454,"range":{"start_line":454,"start_character":42,"end_line":454,"end_character":76},"updated":"2018-07-05 14:46:10.000000000","message":"nit: this could just be self.policies here right?","commit_id":"f4f092abda941d3810de4ea020a3a783d337a3e1"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"fc5ff11ba59ce89e96fd43a95532537392de96b9","unresolved":false,"context_lines":[{"line_number":451,"context_line":"            msg \u003d (\"More than one policy (%(policies)s) is associated with \""},{"line_number":452,"context_line":"                   \"group %(group_name)s, only the first one in the list \""},{"line_number":453,"context_line":"                   \"would be returned.\")"},{"line_number":454,"context_line":"            LOG.warning(msg, {\"policies\": [p.policy for p in self._policies],"},{"line_number":455,"context_line":"                              \"group_name\": self.name})"},{"line_number":456,"context_line":"        return self._policies[0] if self._policies else None"},{"line_number":457,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_1a46afa8","line":454,"range":{"start_line":454,"start_character":42,"end_line":454,"end_character":76},"in_reply_to":"5f7c97a3_636e2ac9","updated":"2018-07-06 08:09:55.000000000","message":"Yes, but we will remove the policies property in next follow patch, so I use it directly.","commit_id":"f4f092abda941d3810de4ea020a3a783d337a3e1"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"cdfa9560a447326d2400754349f45545037dfbfb","unresolved":false,"context_lines":[{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    @property"},{"line_number":449,"context_line":"    def policy(self):"},{"line_number":450,"context_line":"        if len(self._policies) not in (0, 1):"},{"line_number":451,"context_line":"            msg \u003d (\"More than one policy (%(policies)s) is associated with \""},{"line_number":452,"context_line":"                   \"group %(group_name)s, only the first one in the list \""},{"line_number":453,"context_line":"                   \"would be returned.\")"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_06ee24b4","line":450,"range":{"start_line":450,"start_character":8,"end_line":450,"end_character":45},"updated":"2018-07-10 01:27:12.000000000","message":"why not: if len(self._policies) \u003e 1?","commit_id":"afc7650e64753ab7687ae2c4f2714d4bb78a4e5a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9bbec2ebb6432f092a822a0d15f3c1f7a1b7cdea","unresolved":false,"context_lines":[{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    @property"},{"line_number":449,"context_line":"    def policy(self):"},{"line_number":450,"context_line":"        if len(self._policies) not in (0, 1):"},{"line_number":451,"context_line":"            msg \u003d (\"More than one policy (%(policies)s) is associated with \""},{"line_number":452,"context_line":"                   \"group %(group_name)s, only the first one in the list \""},{"line_number":453,"context_line":"                   \"would be returned.\")"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_9907e981","line":450,"range":{"start_line":450,"start_character":8,"end_line":450,"end_character":45},"in_reply_to":"5f7c97a3_06ee24b4","updated":"2018-07-10 13:50:28.000000000","message":"Yeah, this is very weird :)\n\nYikun, Can you fix this in a follow-up?","commit_id":"afc7650e64753ab7687ae2c4f2714d4bb78a4e5a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ad30fa9a6e4e10338ff0505daabfe4b2b7154c73","unresolved":false,"context_lines":[{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    @property"},{"line_number":449,"context_line":"    def policy(self):"},{"line_number":450,"context_line":"        if len(self._policies) not in (0, 1):"},{"line_number":451,"context_line":"            msg \u003d (\"More than one policy (%(policies)s) is associated with \""},{"line_number":452,"context_line":"                   \"group %(group_name)s, only the first one in the list \""},{"line_number":453,"context_line":"                   \"would be returned.\")"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_9e2fca87","line":450,"range":{"start_line":450,"start_character":8,"end_line":450,"end_character":45},"in_reply_to":"5f7c97a3_9907e981","updated":"2018-07-10 16:00:35.000000000","message":"This is what Mike Bayer suggested in an earlier review comment. It made sense at the time...","commit_id":"afc7650e64753ab7687ae2c4f2714d4bb78a4e5a"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"9f62534a1b407f80ae02caf9c6841c1aa76451f7","unresolved":false,"context_lines":[{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    @property"},{"line_number":449,"context_line":"    def policy(self):"},{"line_number":450,"context_line":"        if len(self._policies) not in (0, 1):"},{"line_number":451,"context_line":"            msg \u003d (\"More than one policy (%(policies)s) is associated with \""},{"line_number":452,"context_line":"                   \"group %(group_name)s, only the first one in the list \""},{"line_number":453,"context_line":"                   \"would be returned.\")"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_a415cce8","line":450,"range":{"start_line":450,"start_character":8,"end_line":450,"end_character":45},"in_reply_to":"5f7c97a3_9e2fca87","updated":"2018-07-11 01:30:43.000000000","message":"ha, yeah, I will change it in a follow-up","commit_id":"afc7650e64753ab7687ae2c4f2714d4bb78a4e5a"}],"nova/objects/instance_group.py":[{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"7e9f9eac8fe89d99a34a02466f5f8c4d50145b83","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        already_existing.add(getattr(db_item, field))"},{"line_number":57,"context_line":"        models.append(db_item)"},{"line_number":58,"context_line":"    for item in items:"},{"line_number":59,"context_line":"        if item in already_existing:"},{"line_number":60,"context_line":"            continue"},{"line_number":61,"context_line":"        model \u003d model_class()"},{"line_number":62,"context_line":"        values \u003d {\u0027group_id\u0027: group_id}"},{"line_number":63,"context_line":"        values[field] \u003d item"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_01efa01d","side":"PARENT","line":60,"range":{"start_line":59,"start_character":0,"end_line":60,"end_character":20},"updated":"2018-06-29 10:18:27.000000000","message":"^","commit_id":"094e4d31c5ec122cac5ed74d855a2b2c78cfcb80"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"7e9f9eac8fe89d99a34a02466f5f8c4d50145b83","unresolved":false,"context_lines":[{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        grp.update(values_copy)"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        if policies is not None:"},{"line_number":224,"context_line":"            _instance_group_policies_add(context, grp, policies)"},{"line_number":225,"context_line":"        if members is not None:"},{"line_number":226,"context_line":"            _instance_group_members_add(context, grp, members)"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_217f3c67","side":"PARENT","line":224,"range":{"start_line":223,"start_character":0,"end_line":224,"end_character":64},"updated":"2018-06-29 10:18:27.000000000","message":"Note that in here, we only append the policy in here, but do NOT remove the old policy.\n\nAnd now we don\u0027t have the case to update policy in group, so I raise a save exception in [1]. \n\n[1] https://review.openstack.org/#/c/579113/1/nova/objects/instance_group.py@327","commit_id":"094e4d31c5ec122cac5ed74d855a2b2c78cfcb80"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"c5fe43f6a591fd55dc4a9a872021f98d977ef1ee","unresolved":false,"context_lines":[{"line_number":324,"context_line":"        if \u0027hosts\u0027 in updates:"},{"line_number":325,"context_line":"            raise exception.InstanceGroupSaveException(field\u003d\u0027hosts\u0027)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        # NOTE(yikun): Now we don\u0027t have a case to update policy, so we don\u0027t"},{"line_number":328,"context_line":"        # support update policies(which is a list of policy name). In"},{"line_number":329,"context_line":"        # addition, a group can only have 1 policy associated it, if we"},{"line_number":330,"context_line":"        # want to support policies updating, we need remove old policy first,"},{"line_number":331,"context_line":"        # and then add a new policy."},{"line_number":332,"context_line":"        if \u0027policies\u0027 in updates:"},{"line_number":333,"context_line":"            raise exception.InstanceGroupSaveException(field\u003d\u0027policies\u0027)"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        if not updates:"},{"line_number":336,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_13b7f5f7","line":333,"range":{"start_line":327,"start_character":0,"end_line":333,"end_character":72},"updated":"2018-06-29 13:09:41.000000000","message":"We will mark policies as read-only in next patch, so, I think no need to do this.","commit_id":"3fc370661577ddc743112e04075d4adb3cb68547"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d7c916604465003bbd9cf272dccc3c8870764c4d","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":123,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":126,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":127,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_9d242c62","line":125,"updated":"2018-07-03 19:19:12.000000000","message":"If this can\u0027t be updated (you removed the save() support), then shouldn\u0027t this be read_only\u003dTrue so it can only be set on create? You do that in the next patch in the series, but maybe it should be in this change.\n\nhttps://review.openstack.org/#/c/563375/23/nova/objects/instance_group.py","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"44ff436c89f2463928a46b7bc193c6277c4a33d7","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":123,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":126,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":127,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_3fbce7b3","line":125,"in_reply_to":"5f7c97a3_3deb98a4","updated":"2018-07-04 04:02:23.000000000","message":"Done","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2f6a366d419781c1892249a51ed8913a2ae44a12","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":123,"context_line":"        \u0027name\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        \u0027policies\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":126,"context_line":"        \u0027members\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":127,"context_line":"        \u0027hosts\u0027: fields.ListOfStringsField(nullable\u003dTrue),"},{"line_number":128,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_3deb98a4","line":125,"in_reply_to":"5f7c97a3_9d242c62","updated":"2018-07-03 19:38:17.000000000","message":"Yeah.","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d7c916604465003bbd9cf272dccc3c8870764c4d","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # field explicitly, we prefer to raise an Exception so the developer"},{"line_number":322,"context_line":"        # knows he has to call obj_reset_changes([\u0027hosts\u0027]) right after setting"},{"line_number":323,"context_line":"        # the field."},{"line_number":324,"context_line":"        if \u0027hosts\u0027 in updates:"},{"line_number":325,"context_line":"            raise exception.InstanceGroupSaveException(field\u003d\u0027hosts\u0027)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if not updates:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_fdd20033","line":324,"updated":"2018-07-03 19:19:12.000000000","message":"You should likely also have a check like this for policies if we\u0027re not going to honor changes to the policies field in the group.","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"44ff436c89f2463928a46b7bc193c6277c4a33d7","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # field explicitly, we prefer to raise an Exception so the developer"},{"line_number":322,"context_line":"        # knows he has to call obj_reset_changes([\u0027hosts\u0027]) right after setting"},{"line_number":323,"context_line":"        # the field."},{"line_number":324,"context_line":"        if \u0027hosts\u0027 in updates:"},{"line_number":325,"context_line":"            raise exception.InstanceGroupSaveException(field\u003d\u0027hosts\u0027)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if not updates:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_ffdf4f09","line":324,"in_reply_to":"5f7c97a3_3d7878bf","updated":"2018-07-04 04:02:23.000000000","message":"Done","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ff86119c41be8ce241d6dbafee92779caf6103c5","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # field explicitly, we prefer to raise an Exception so the developer"},{"line_number":322,"context_line":"        # knows he has to call obj_reset_changes([\u0027hosts\u0027]) right after setting"},{"line_number":323,"context_line":"        # the field."},{"line_number":324,"context_line":"        if \u0027hosts\u0027 in updates:"},{"line_number":325,"context_line":"            raise exception.InstanceGroupSaveException(field\u003d\u0027hosts\u0027)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if not updates:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_3d7878bf","line":324,"in_reply_to":"5f7c97a3_7dc7d066","updated":"2018-07-03 19:49:56.000000000","message":"Yeah there are no external group update APIs and you have to provide exactly one policy on group create:\n\nhttps://developer.openstack.org/api-ref/compute/#create-server-group","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2f6a366d419781c1892249a51ed8913a2ae44a12","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # field explicitly, we prefer to raise an Exception so the developer"},{"line_number":322,"context_line":"        # knows he has to call obj_reset_changes([\u0027hosts\u0027]) right after setting"},{"line_number":323,"context_line":"        # the field."},{"line_number":324,"context_line":"        if \u0027hosts\u0027 in updates:"},{"line_number":325,"context_line":"            raise exception.InstanceGroupSaveException(field\u003d\u0027hosts\u0027)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if not updates:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_7dc7d066","line":324,"in_reply_to":"5f7c97a3_fdd20033","updated":"2018-07-03 19:38:17.000000000","message":"It\u0027s technically a behavioral change as the object would let you have and save multiples, but I guess it\u0027s worth optimizing here and just throwing this in without a bunch of compat.\n\nJust to be clear, such a change is assuming that we have no existing uses of updating the policy for a group after it is created. Is that correct?","commit_id":"31fa42a8fa4b92483a329377a730ef9c4f4f8e18"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd46e73ad24efe4fa507e82886fd0304f1cc25ea","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        if policies:"},{"line_number":227,"context_line":"            policy \u003d api_models.InstanceGroupPolicy("},{"line_number":228,"context_line":"                group_id\u003dgroup[\u0027id\u0027], policy\u003dpolicies[0], rules\u003dNone)"},{"line_number":229,"context_line":"            group._policies \u003d [policy]"},{"line_number":230,"context_line":"            group.save(context.session)"},{"line_number":231,"context_line":"        else:"},{"line_number":232,"context_line":"            group._policies \u003d []"},{"line_number":233,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5f7c97a3_0395f687","line":230,"range":{"start_line":229,"start_character":12,"end_line":230,"end_character":39},"updated":"2018-07-05 14:46:10.000000000","message":"I\u0027m not sure if it matters, but will the entry in group._policies here have it\u0027s id primary key set after group.save() is called? I don\u0027t think it matters because we just pass the group variable to _from_db_object to populate this versioned object with it\u0027s id and created_at after it\u0027s created.","commit_id":"f4f092abda941d3810de4ea020a3a783d337a3e1"}],"nova/tests/functional/db/test_instance_group.py":[{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"2fc5da8f8c19d3be526c82c5f25e21fa071c3e9f","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                                      user_id\u003dself.context.user_id,"},{"line_number":32,"context_line":"                                      project_id\u003dself.context.project_id,"},{"line_number":33,"context_line":"                                      name\u003d\u0027foogroup\u0027,"},{"line_number":34,"context_line":"                                      policies\u003d[\u0027foo1\u0027, \u0027foo2\u0027],"},{"line_number":35,"context_line":"                                      members\u003d[\u0027memberfoo\u0027])"},{"line_number":36,"context_line":"        group.update(values)"},{"line_number":37,"context_line":"        group.create()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_cf1f58b6","line":34,"range":{"start_line":34,"start_character":38,"end_line":34,"end_character":63},"updated":"2018-07-03 06:57:26.000000000","message":"should you change this?","commit_id":"46925cdd8f825c3bbbe39886ed6a17ebf2d804cd"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"7bf44cba5c880bdf9147b6a3ff0a81ae5f84deb6","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                                      user_id\u003dself.context.user_id,"},{"line_number":32,"context_line":"                                      project_id\u003dself.context.project_id,"},{"line_number":33,"context_line":"                                      name\u003d\u0027foogroup\u0027,"},{"line_number":34,"context_line":"                                      policies\u003d[\u0027foo1\u0027, \u0027foo2\u0027],"},{"line_number":35,"context_line":"                                      members\u003d[\u0027memberfoo\u0027])"},{"line_number":36,"context_line":"        group.update(values)"},{"line_number":37,"context_line":"        group.create()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_ef6c9308","line":34,"range":{"start_line":34,"start_character":38,"end_line":34,"end_character":63},"in_reply_to":"5f7c97a3_cf1f58b6","updated":"2018-07-03 08:07:43.000000000","message":"Done","commit_id":"46925cdd8f825c3bbbe39886ed6a17ebf2d804cd"}],"nova/tests/unit/objects/test_instance_group.py":[{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"c5fe43f6a591fd55dc4a9a872021f98d977ef1ee","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        mock_notify.assert_called_once_with(self.context, \"update\","},{"line_number":100,"context_line":"                                               {\u0027name\u0027: \u0027new_name\u0027,"},{"line_number":101,"context_line":"                                                \u0027members\u0027: [\u0027instance_id1\u0027],"},{"line_number":102,"context_line":"                                                \u0027policies\u0027: [\u0027policy1\u0027],"},{"line_number":103,"context_line":"                                                \u0027server_group_id\u0027: _DB_UUID})"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    @mock.patch(\u0027nova.compute.utils.notify_about_server_group_update\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_53d48d1f","line":102,"range":{"start_line":102,"start_character":0,"end_line":102,"end_character":72},"updated":"2018-06-29 13:09:41.000000000","message":"remove","commit_id":"3fc370661577ddc743112e04075d4adb3cb68547"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7679adcf5a2130f6228d7fcb33ea3bd9194a4af9","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    \u0027user_id\u0027: \u0027fake_user\u0027,"},{"line_number":37,"context_line":"    \u0027project_id\u0027: \u0027fake_project\u0027,"},{"line_number":38,"context_line":"    \u0027name\u0027: \u0027fake_name\u0027,"},{"line_number":39,"context_line":"    # a group can only have 1 policy associated it"},{"line_number":40,"context_line":"    \u0027policies\u0027: [\u0027policy1\u0027],"},{"line_number":41,"context_line":"    \u0027members\u0027: [\u0027instance_id1\u0027, \u0027instance_id2\u0027],"},{"line_number":42,"context_line":"    \u0027created_at\u0027: _TS_NOW,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_8948b2b3","line":39,"range":{"start_line":39,"start_character":48,"end_line":39,"end_character":50},"updated":"2018-06-29 14:27:54.000000000","message":"with it","commit_id":"81e34a47c70d06adf82ba9f88ed29ba9f1c28312"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"5292e9bb9e1e72ce45c668f5280e524ce05ee64c","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    \u0027user_id\u0027: \u0027fake_user\u0027,"},{"line_number":37,"context_line":"    \u0027project_id\u0027: \u0027fake_project\u0027,"},{"line_number":38,"context_line":"    \u0027name\u0027: \u0027fake_name\u0027,"},{"line_number":39,"context_line":"    # a group can only have 1 policy associated it"},{"line_number":40,"context_line":"    \u0027policies\u0027: [\u0027policy1\u0027],"},{"line_number":41,"context_line":"    \u0027members\u0027: [\u0027instance_id1\u0027, \u0027instance_id2\u0027],"},{"line_number":42,"context_line":"    \u0027created_at\u0027: _TS_NOW,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_46691b24","line":39,"range":{"start_line":39,"start_character":48,"end_line":39,"end_character":50},"in_reply_to":"5f7c97a3_8948b2b3","updated":"2018-06-30 01:27:52.000000000","message":"Done","commit_id":"81e34a47c70d06adf82ba9f88ed29ba9f1c28312"}]}