)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"da4c684c0e664245f1b58a71535fde68683d8278","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Instance groups: use list comprehension for unsupported policy check"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In addition to this this removes an unused import and completed the"},{"line_number":10,"context_line":"policy validations."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This is part of blueprint instance-group-api-extension."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"AAAAWH%2F%2Fzlc%3D","line":10,"updated":"2014-03-13 17:32:20.000000000","message":"This is backwards to me. The list comprehension change is an extremely minor thing, as is the import removal. Can you please title this according to the policy check? I\u0027d even prefer the two cleanups to be in a separate cleanup patch, but a _minimum_ this should be properly titled, IMHO.","commit_id":"99c74a6edc6c0e2b8ec46775ad6b1c524ade5abb"}],"nova/api/openstack/compute/contrib/server_groups.py":[{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"af4b6a5886560d061e52480edab5996909926a6f","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        if not policies:"},{"line_number":172,"context_line":"            return"},{"line_number":173,"context_line":"        if (\u0027anti-affinity\u0027 in policies and"},{"line_number":174,"context_line":"            \u0027affinity\u0027 in policies):"},{"line_number":175,"context_line":"            msg \u003d _(\"Conflicting policies configured!\")"},{"line_number":176,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":177,"context_line":"        not_supported \u003d [policy for policy in policies"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAVn%2F%2FC7Y%3D","line":174,"updated":"2014-03-10 13:07:13.000000000","message":"So this is not your fault, but since you\u0027re touching this code at the same time, at this point there is no check that policies is actually a list (and say not an int). So we can end up with an exception that isn\u0027t handled.\n\nIf you do fix it you might want to rebase on https://review.openstack.org/#/c/79295/ which fixes some other input validation issues. Maybe policy validation should come after the rest of the input validation? Which in light of this patch now I see isn\u0027t quite right either because we really do need to enforce that policies is a list rather than leaving it to the objects layer to enforce.","commit_id":"abd226ca4a6a37ee34ab5294ae43a47f9fc888fc"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"a1d217cbabfe8d48bc2903a3659057c1f1d4704a","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        Validates that there are no contradicting policies, for example"},{"line_number":159,"context_line":"        \u0027anti-affinity\u0027 and \u0027affinity\u0027 in the same group. In"},{"line_number":160,"context_line":"        addition to this validates that the deifned policies are supported."},{"line_number":161,"context_line":"        :param policies:     the given policies of the server_group"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        if not policies:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Bw3c%3D","line":160,"updated":"2014-03-12 10:41:53.000000000","message":"nit - deifned -\u003e defined","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ca543d971a8db40fce80ce6f1de2fb7527f26f92","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        Validates that there are no contradicting policies, for example"},{"line_number":159,"context_line":"        \u0027anti-affinity\u0027 and \u0027affinity\u0027 in the same group. In"},{"line_number":160,"context_line":"        addition to this validates that the deifned policies are supported."},{"line_number":161,"context_line":"        :param policies:     the given policies of the server_group"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        if not policies:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Bwpo%3D","line":160,"in_reply_to":"AAAAVn%2F%2Bw3c%3D","updated":"2014-03-12 10:58:17.000000000","message":"Done","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"a1d217cbabfe8d48bc2903a3659057c1f1d4704a","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        addition to this validates that the deifned policies are supported."},{"line_number":161,"context_line":"        :param policies:     the given policies of the server_group"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        if not policies:"},{"line_number":164,"context_line":"            return"},{"line_number":165,"context_line":"        if (\u0027anti-affinity\u0027 in policies and"},{"line_number":166,"context_line":"            \u0027affinity\u0027 in policies):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2BwzE%3D","line":163,"updated":"2014-03-12 10:41:53.000000000","message":"Don\u0027t need this now because of line 185 which runs before its called","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ca543d971a8db40fce80ce6f1de2fb7527f26f92","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        addition to this validates that the deifned policies are supported."},{"line_number":161,"context_line":"        :param policies:     the given policies of the server_group"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        if not policies:"},{"line_number":164,"context_line":"            return"},{"line_number":165,"context_line":"        if (\u0027anti-affinity\u0027 in policies and"},{"line_number":166,"context_line":"            \u0027affinity\u0027 in policies):"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2BwpM%3D","line":163,"in_reply_to":"AAAAVn%2F%2BwzE%3D","updated":"2014-03-12 10:58:17.000000000","message":"Done","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"a1d217cbabfe8d48bc2903a3659057c1f1d4704a","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        expected_fields \u003d [\u0027name\u0027, \u0027policies\u0027]"},{"line_number":183,"context_line":"        for field in expected_fields:"},{"line_number":184,"context_line":"            value \u003d subbody.pop(field, None)"},{"line_number":185,"context_line":"            if not value:"},{"line_number":186,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is either missing or empty.\") % field"},{"line_number":187,"context_line":"                raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":188,"context_line":"            if field \u003d\u003d \u0027name\u0027:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2BwyY%3D","line":185,"updated":"2014-03-12 10:41:53.000000000","message":"So this means that an empty policy list is invalid - I think this is correct because it doesn\u0027t really make sense to make a server group with no policy?","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ca543d971a8db40fce80ce6f1de2fb7527f26f92","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        expected_fields \u003d [\u0027name\u0027, \u0027policies\u0027]"},{"line_number":183,"context_line":"        for field in expected_fields:"},{"line_number":184,"context_line":"            value \u003d subbody.pop(field, None)"},{"line_number":185,"context_line":"            if not value:"},{"line_number":186,"context_line":"                msg \u003d _(\"\u0027%s\u0027 is either missing or empty.\") % field"},{"line_number":187,"context_line":"                raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":188,"context_line":"            if field \u003d\u003d \u0027name\u0027:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2BwnU%3D","line":185,"in_reply_to":"AAAAVn%2F%2BwyY%3D","updated":"2014-03-12 10:58:17.000000000","message":"Correct. Basically a server group withou a policy will not have any affect","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"c1195da5875e624cdbe7f823f741ca3e9e292f5a","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        Validates that there are no contradicting policies, for example"},{"line_number":159,"context_line":"        \u0027anti-affinity\u0027 and \u0027affinity\u0027 in the same group. In"},{"line_number":160,"context_line":"        addition to this validates that the defined policies are supported."},{"line_number":161,"context_line":"        :param policies:     the given policies of the server_group"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        if (\u0027anti-affinity\u0027 in policies and"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAWH%2F%2FdQE%3D","line":160,"updated":"2014-03-17 19:39:48.000000000","message":"Why did you remove this text?  It also does what you removed","commit_id":"f01ae7f7d2512885d6b3a40f52456e2e98853407"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"3cabffe577448c6bf959fce5560f711ab5c9735f","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        Validates that there are no contradicting policies, for example"},{"line_number":159,"context_line":"        \u0027anti-affinity\u0027 and \u0027affinity\u0027 in the same group. In"},{"line_number":160,"context_line":"        addition to this validates that the defined policies are supported."},{"line_number":161,"context_line":"        :param policies:     the given policies of the server_group"},{"line_number":162,"context_line":"        \"\"\""},{"line_number":163,"context_line":"        if (\u0027anti-affinity\u0027 in policies and"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAWH%2F%2ByuE%3D","line":160,"in_reply_to":"AAAAWH%2F%2FdQE%3D","updated":"2014-03-24 10:25:14.000000000","message":"I am not sure that I understand your comment here. I added that the method also validates that the defined policies are supported - for example if a user decides to set a policy that is called \u0027same-rack\u0027 then it will fail as the policy is not supported.\nPlease advise","commit_id":"f01ae7f7d2512885d6b3a40f52456e2e98853407"}],"nova/tests/api/openstack/compute/contrib/test_server_groups.py":[{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"a1d217cbabfe8d48bc2903a3659057c1f1d4704a","unresolved":false,"context_lines":[{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def test_create_server_group_with_illegal_policies(self):"},{"line_number":148,"context_line":"        # blank policy"},{"line_number":149,"context_line":"        sgroup \u003d server_group_template(name\u003d\u0027fake-name\u0027, policies\u003d\u0027\u0027)"},{"line_number":150,"context_line":"        # policy as integer"},{"line_number":151,"context_line":"        sgroup \u003d server_group_template(name\u003d\u0027fake-name\u0027, policies\u003d7)"},{"line_number":152,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/os-server-groups\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Bww0%3D","line":149,"updated":"2014-03-12 10:41:53.000000000","message":"I think you\u0027re missing the actual test code after this bit before the policy as integer test?","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ca543d971a8db40fce80ce6f1de2fb7527f26f92","unresolved":false,"context_lines":[{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def test_create_server_group_with_illegal_policies(self):"},{"line_number":148,"context_line":"        # blank policy"},{"line_number":149,"context_line":"        sgroup \u003d server_group_template(name\u003d\u0027fake-name\u0027, policies\u003d\u0027\u0027)"},{"line_number":150,"context_line":"        # policy as integer"},{"line_number":151,"context_line":"        sgroup \u003d server_group_template(name\u003d\u0027fake-name\u0027, policies\u003d7)"},{"line_number":152,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/os-server-groups\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAVn%2F%2Bwoo%3D","line":149,"in_reply_to":"AAAAVn%2F%2Bww0%3D","updated":"2014-03-12 10:58:17.000000000","message":"Done","commit_id":"e55e4e7ee407ff866e724606f7d7f2b94e8e7a29"}]}