)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"f643af581b65db2b1883381394d0d57388753708","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Don\u0027t store duplicate policies for server_group"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It doesn\u0027t make sense to store same policies in a server_group."},{"line_number":10,"context_line":"We only need to store one and ignore the duplicate policies."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I4f3ad544aef78cbbc076c7a47cca04832a2f5b4b"},{"line_number":13,"context_line":"Closes-Bug: #1324348"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_0ece5b83","line":10,"updated":"2014-06-18 12:09:28.000000000","message":"The above message is against current behavior of this patch.\n\nWe need to deny the duplicate policies.","commit_id":"128178a8278cd11a135ccfe50d38dbde2d391e6a"},{"author":{"_account_id":8021,"name":"wingwj","email":"wingwj@gmail.com","username":"wingwj"},"change_message_id":"0408118cc63b28be2a9e0acd2f64521bd0ad2068","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Don\u0027t store duplicate policies for server_group"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It doesn\u0027t make sense to store same policies in a server_group."},{"line_number":10,"context_line":"We only need to store one and ignore the duplicate policies."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I4f3ad544aef78cbbc076c7a47cca04832a2f5b4b"},{"line_number":13,"context_line":"Closes-Bug: #1324348"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_d2c0767e","line":10,"in_reply_to":"1ae5cdf2_0ece5b83","updated":"2014-06-19 01:21:30.000000000","message":"Done","commit_id":"128178a8278cd11a135ccfe50d38dbde2d391e6a"}],"nova/api/openstack/compute/contrib/server_groups.py":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"4b74412474e6fbae189f083d0079a9ca2541c6cf","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        try:"},{"line_number":267,"context_line":"            sg.name \u003d vals.get(\u0027name\u0027)"},{"line_number":268,"context_line":"            # Note(wingwj): It doesn\u0027t make sense to store duplicate policies."},{"line_number":269,"context_line":"            sg.policies \u003d list(set(vals.get(\u0027policies\u0027)))"},{"line_number":270,"context_line":"            sg.create(context)"},{"line_number":271,"context_line":"        except ValueError as e:"},{"line_number":272,"context_line":"            raise exc.HTTPBadRequest(explanation\u003de)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ae8d1ca_3ecc45bc","line":269,"updated":"2014-05-29 09:45:36.000000000","message":"How about returning a BadRequest response instead of merging them automatically?\n\nbecause clients can know their mistakes.","commit_id":"23fe2861e303d04a2a841ce5e3cbec342da11ab6"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"387759f3f39bcc239cdb7a4496bb8f09acb62dd6","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        try:"},{"line_number":267,"context_line":"            sg.name \u003d vals.get(\u0027name\u0027)"},{"line_number":268,"context_line":"            # Note(wingwj): It doesn\u0027t make sense to store duplicate policies."},{"line_number":269,"context_line":"            sg.policies \u003d list(set(vals.get(\u0027policies\u0027)))"},{"line_number":270,"context_line":"            sg.create(context)"},{"line_number":271,"context_line":"        except ValueError as e:"},{"line_number":272,"context_line":"            raise exc.HTTPBadRequest(explanation\u003de)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_85217db6","line":269,"in_reply_to":"1ae5cdf2_25a18947","updated":"2014-06-03 02:48:31.000000000","message":"Thanks, wingwj!\nI\u0027m looking forward to a new patch;-)","commit_id":"23fe2861e303d04a2a841ce5e3cbec342da11ab6"},{"author":{"_account_id":8021,"name":"wingwj","email":"wingwj@gmail.com","username":"wingwj"},"change_message_id":"f53a8aa77b90b2ef941bd582f404eb4a572fb8eb","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        try:"},{"line_number":267,"context_line":"            sg.name \u003d vals.get(\u0027name\u0027)"},{"line_number":268,"context_line":"            # Note(wingwj): It doesn\u0027t make sense to store duplicate policies."},{"line_number":269,"context_line":"            sg.policies \u003d list(set(vals.get(\u0027policies\u0027)))"},{"line_number":270,"context_line":"            sg.create(context)"},{"line_number":271,"context_line":"        except ValueError as e:"},{"line_number":272,"context_line":"            raise exc.HTTPBadRequest(explanation\u003de)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_25a18947","line":269,"in_reply_to":"3ae8d1ca_3ecc45bc","updated":"2014-06-03 02:46:15.000000000","message":"Ok, I\u0027ll change it to raise an exception, like we discussed in IRC channel. :)","commit_id":"23fe2861e303d04a2a841ce5e3cbec342da11ab6"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"9bee546f48df7c75bb97213dc88c398aa4756fd7","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        # Note(wingwj): It doesn\u0027t make sense to store duplicate policies."},{"line_number":185,"context_line":"        if list(set(policies)) !\u003d policies:"},{"line_number":186,"context_line":"            msg \u003d _(\"Duplicate policies configured!\")"},{"line_number":187,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_2667a94e","line":185,"updated":"2014-06-11 16:10:15.000000000","message":"This isn\u0027t going to be effective; the ordering of the list generated from the set is arbitrary.  Consider this:\n\n    \u003e\u003e\u003e list(set([\u0027a\u0027, \u0027b\u0027, \u0027c\u0027, \u0027d\u0027]))\n    [\u0027a\u0027, \u0027c\u0027, \u0027b\u0027, \u0027d\u0027]\n\nThe list [\u0027a\u0027, \u0027b\u0027, \u0027c\u0027, \u0027d\u0027] should be acceptable by your criteria, but it would be rejected with an InvalidInput because of the ordering difference.\n\nThere are two possible approaches here.  The first would be to simply eliminate the duplications by doing:\n\n    policies \u003d list(set(policies))\n\nor something similar.  The second would be to sort the two lists, which would change your if condition to something like:\n\n    if sorted(set(policies)) !\u003d sorted(policies):\n\nI would prefer the first option, I think, as it would be more efficient and would likely avoid breaking anyone that\u0027s currently expecting idempotency from this call…","commit_id":"87198ae9641c20300fccc8dae92df8562aba8347"},{"author":{"_account_id":8021,"name":"wingwj","email":"wingwj@gmail.com","username":"wingwj"},"change_message_id":"b6f4d4d08cf3188e6983418274ffa859783ed9dd","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        # Note(wingwj): It doesn\u0027t make sense to store duplicate policies."},{"line_number":185,"context_line":"        if list(set(policies)) !\u003d policies:"},{"line_number":186,"context_line":"            msg \u003d _(\"Duplicate policies configured!\")"},{"line_number":187,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_786f7cfc","line":185,"in_reply_to":"1ae5cdf2_2667a94e","updated":"2014-06-12 02:34:21.000000000","message":"Hi Kevin, thanks again for your kindly help.\n\nThe original patch I submitted in PS1 is just the first approach you mentioned.\n\nAt that time I found there\u0027s no overall standard about this issue in OpenStack now.. I found a similar discussion in ceilometer(http://osdir.com/ml/openstack-dev/2014-04/msg01164.html), it preferred not to disturb user. And I also checked this in neutron, but it will raise an exception if you give duplicate security_group_rules.. That\u0027s not the same reaction here..\n\nSo I discussed this issue with @Ken\u0027ichi in IRC, after that I decided to modify the issue as this. But we didn\u0027t think about the backwards compatibility of this API.\n\nSo, should I need to revert my patch to PS1 as the first approach you mentioned here?","commit_id":"87198ae9641c20300fccc8dae92df8562aba8347"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"4d75845816de48457b9a75a0e1a83cd38c7cdbe9","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        # Note(wingwj): It doesn\u0027t make sense to store duplicate policies."},{"line_number":185,"context_line":"        if list(set(policies)) !\u003d policies:"},{"line_number":186,"context_line":"            msg \u003d _(\"Duplicate policies configured!\")"},{"line_number":187,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_bb481e2e","line":185,"in_reply_to":"1ae5cdf2_786f7cfc","updated":"2014-06-12 02:50:36.000000000","message":"yes, I asked BadRequest response for this case in previous review.\nbecause it seems difficult to distinguish the duplicated policies are intention or not. If client specified the duplicated policies due to some typo, clients cannot know it from the elimination.","commit_id":"87198ae9641c20300fccc8dae92df8562aba8347"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"46a49bab4d2c616c3c3f6db5d286574c375a645d","unresolved":false,"context_lines":[{"line_number":182,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        # Note(wingwj): It doesn\u0027t make sense to store duplicate policies."},{"line_number":185,"context_line":"        if list(set(policies)) !\u003d policies:"},{"line_number":186,"context_line":"            msg \u003d _(\"Duplicate policies configured!\")"},{"line_number":187,"context_line":"            raise nova.exception.InvalidInput(reason\u003dmsg)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_6f1ebe71","line":185,"in_reply_to":"1ae5cdf2_bb481e2e","updated":"2014-06-12 16:39:53.000000000","message":"So, my personal feeling remains that the first approach—modifying the user instructions to eliminate duplicates—is the best approach.  However, if you choose to take the second approach—due to comments by other reviewers, say ;)—that\u0027s fine too.  My objection is only that the code you had wasn\u0027t going to accomplish that correctly :)","commit_id":"87198ae9641c20300fccc8dae92df8562aba8347"}],"nova/tests/api/openstack/compute/contrib/test_server_groups.py":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"10e8b43d2223c3a97621ad1fbe4ddf543cafe5da","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        sgroup \u003d server_group_template()"},{"line_number":233,"context_line":"        policies \u003d [\u0027affinity\u0027, \u0027affinity\u0027]"},{"line_number":234,"context_line":"        sgroup[\u0027policies\u0027] \u003d policies"},{"line_number":235,"context_line":"        res_dict \u003d self.controller.create(req, {\u0027server_group\u0027: sgroup})"},{"line_number":236,"context_line":"        self.assertEqual(res_dict[\u0027server_group\u0027][\u0027name\u0027], \u0027test\u0027)"},{"line_number":237,"context_line":"        self.assertTrue(uuidutils.is_uuid_like(res_dict[\u0027server_group\u0027][\u0027id\u0027]))"},{"line_number":238,"context_line":"        self.assertEqual(res_dict[\u0027server_group\u0027][\u0027policies\u0027], [\u0027affinity\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_1cd83a7a","line":235,"updated":"2014-06-04 02:07:30.000000000","message":"Before changing Tempest side, we need to fix this unit test as the first step because of the following failure.\n\nTraceback (most recent call last):\n  File \"nova/tests/api/openstack/compute/contrib/test_server_groups.py\", line 235, in test_create_server_group_with_duplicate_policies\n    res_dict \u003d self.controller.create(req, {\u0027server_group\u0027: sgroup})\n  File \"nova/api/openstack/compute/contrib/server_groups.py\", line 265, in create\n    raise exc.HTTPBadRequest(explanation\u003de.format_message())\nHTTPBadRequest: Invalid input received: Duplicate policies configured!\n\nSo here should be\n\n self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,\n                   req, {\u0027server_group\u0027: sgroup})","commit_id":"0e4e202409ee04bb9ca6f673b0641d6ab4e1dce4"},{"author":{"_account_id":8021,"name":"wingwj","email":"wingwj@gmail.com","username":"wingwj"},"change_message_id":"436b0866b53929883d87e0c336641026c52ed09d","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        sgroup \u003d server_group_template()"},{"line_number":233,"context_line":"        policies \u003d [\u0027affinity\u0027, \u0027affinity\u0027]"},{"line_number":234,"context_line":"        sgroup[\u0027policies\u0027] \u003d policies"},{"line_number":235,"context_line":"        res_dict \u003d self.controller.create(req, {\u0027server_group\u0027: sgroup})"},{"line_number":236,"context_line":"        self.assertEqual(res_dict[\u0027server_group\u0027][\u0027name\u0027], \u0027test\u0027)"},{"line_number":237,"context_line":"        self.assertTrue(uuidutils.is_uuid_like(res_dict[\u0027server_group\u0027][\u0027id\u0027]))"},{"line_number":238,"context_line":"        self.assertEqual(res_dict[\u0027server_group\u0027][\u0027policies\u0027], [\u0027affinity\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_3c9ed607","line":235,"in_reply_to":"1ae5cdf2_1cd83a7a","updated":"2014-06-04 02:30:54.000000000","message":"Done","commit_id":"0e4e202409ee04bb9ca6f673b0641d6ab4e1dce4"}]}