)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":23276,"name":"Nicholas Jones","email":"nj762h@att.com","username":"nj762h"},"change_message_id":"77d44c0c65fbb9b19ea6051458466fb82fc74ab0","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove \"list_volume\" from filter"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"v3.25 introduced \"list_volume\" parameter for querying groups with"},{"line_number":10,"context_line":"volumes. But it was used for filter and dit not work as expected."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch pop it from the filter and let it work"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa31d9ce_68a3e24f","line":10,"range":{"start_line":10,"start_character":40,"end_line":10,"end_character":43},"updated":"2017-02-13 19:34:39.000000000","message":"s/dit/did/","commit_id":"277a65bac2fb0dd591fa1d6f2e9c2e7a800f3b65"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"b0ab71c9262438749e2728addd8d32aaff5c473f","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove \"list_volume\" from filter"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"v3.25 introduced \"list_volume\" parameter for querying groups with"},{"line_number":10,"context_line":"volumes. But it was used for filter and dit not work as expected."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch pop it from the filter and let it work"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa31d9ce_643863f0","line":10,"range":{"start_line":10,"start_character":40,"end_line":10,"end_character":43},"in_reply_to":"fa31d9ce_68a3e24f","updated":"2017-02-14 01:16:54.000000000","message":"Done","commit_id":"277a65bac2fb0dd591fa1d6f2e9c2e7a800f3b65"}],"cinder/api/v3/groups.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"45f6f5d6ff0176f9cb793ec37aa19fa59a87ae0b","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        sort_keys, sort_dirs \u003d common.get_sort_params(filters)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # v3.25 introduced \"list_volume\" parameter to query groups with"},{"line_number":170,"context_line":"        # volumes, but it is not a filter."},{"line_number":171,"context_line":"        filters.pop(\u0027list_volume\u0027, None)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        groups \u003d self.group_api.get_all("}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_7cbaca6f","line":170,"updated":"2017-03-09 09:33:35.000000000","message":"nit: We don\u0027t need such a long explanation.","commit_id":"3d3a2751b458b5997f24535285e2cbe4c24f1999"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"c09c2ad11aa1993821f000d02f7d3d4ab35636de","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        sort_keys, sort_dirs \u003d common.get_sort_params(filters)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # v3.25 introduced \"list_volume\" parameter to query groups with"},{"line_number":170,"context_line":"        # volumes, but it is not a filter."},{"line_number":171,"context_line":"        filters.pop(\u0027list_volume\u0027, None)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        groups \u003d self.group_api.get_all("}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_fa8cfa80","line":170,"in_reply_to":"9a30ddce_7cbaca6f","updated":"2017-03-09 10:06:19.000000000","message":"Done","commit_id":"3d3a2751b458b5997f24535285e2cbe4c24f1999"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"748816a5e5e1522a2a771115da83668aad0b401f","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        marker, limit, offset \u003d common.get_pagination_params(filters)"},{"line_number":167,"context_line":"        sort_keys, sort_dirs \u003d common.get_sort_params(filters)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        filters.pop(\u0027list_volume\u0027, None)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        groups \u003d self.group_api.get_all("},{"line_number":172,"context_line":"            context, filters\u003dfilters, marker\u003dmarker, limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_8b00bb67","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":40},"updated":"2017-03-11 15:40:44.000000000","message":"I\u0027m unclear on how this fixes the bug, or why we would target and remove some fileds that were passed in via a generic filter?\n\nThe fact that the db filter may not do what\u0027s expected is kind of irrelevant here I think.  Picking and choosing things to pop off that were passed in via filters seems like an ugly way to go.\n\nI\u0027d rather wait and leverage the user-visible-resource-fitlers list for things like this if they\u0027re really necessary.","commit_id":"57930687b65aa11bbe1b724ec0397af5eb441337"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f48f672499879ce7c8ca2d069aebc6d67d20f6f1","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        marker, limit, offset \u003d common.get_pagination_params(filters)"},{"line_number":167,"context_line":"        sort_keys, sort_dirs \u003d common.get_sort_params(filters)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        filters.pop(\u0027list_volume\u0027, None)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        groups \u003d self.group_api.get_all("},{"line_number":172,"context_line":"            context, filters\u003dfilters, marker\u003dmarker, limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_d8b3504b","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":40},"in_reply_to":"9a30ddce_2ea8b876","updated":"2017-03-13 11:17:20.000000000","message":"@John, sometimes it\u0027s not easy to know what\u0027s what around the API, since the params include arguments for pagination, sorting, offset, sorting, all_tenants, list_volume, as well as filters that are interpreted by the DBMS itself.  So we have to remove everything that\u0027s not meant to go on the DBMS from the filter variable (it\u0027s a copy of the params dict) but leave the original params dict intact so we can use it in the view builder, where we actually use the list_volume parameter (don\u0027t ask me why we are doing it this way  ;-)  ).","commit_id":"57930687b65aa11bbe1b724ec0397af5eb441337"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"a2ea660c525a9b5f3e47ad9c65f804f5ebcb9f1c","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        marker, limit, offset \u003d common.get_pagination_params(filters)"},{"line_number":167,"context_line":"        sort_keys, sort_dirs \u003d common.get_sort_params(filters)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        filters.pop(\u0027list_volume\u0027, None)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        groups \u003d self.group_api.get_all("},{"line_number":172,"context_line":"            context, filters\u003dfilters, marker\u003dmarker, limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_2ea8b876","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":40},"in_reply_to":"9a30ddce_33710145","updated":"2017-03-12 14:58:58.000000000","message":"Oh I see, sorry; it\u0027s intended to be an arg to the call and not truly a filter anyway.  So I guess we can explicitly remove it like you have here, but if it\u0027s invalid it should just be ignored anyway no?","commit_id":"57930687b65aa11bbe1b724ec0397af5eb441337"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"5ce29b250e63d51b14c6f0358d77a1ae6e4b0285","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        marker, limit, offset \u003d common.get_pagination_params(filters)"},{"line_number":167,"context_line":"        sort_keys, sort_dirs \u003d common.get_sort_params(filters)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        filters.pop(\u0027list_volume\u0027, None)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        groups \u003d self.group_api.get_all("},{"line_number":172,"context_line":"            context, filters\u003dfilters, marker\u003dmarker, limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_33710145","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":40},"in_reply_to":"9a30ddce_8b00bb67","updated":"2017-03-12 11:23:53.000000000","message":"the \"list_volumes here\" is the same like \"all_tenant\" in list volume/snapshot/group[1]. It\u0027s not a filter to used by db layer. So I pop it. And the the description in spec is the same as well[2].\n\nIt\u0027s used in cinder/api/v3/view/groups.py\n\n[1]: https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L554\n[2]: http://specs.openstack.org/openstack/cinder-specs/specs/newton/improvement-to-query-consistency-group-detail.html","commit_id":"57930687b65aa11bbe1b724ec0397af5eb441337"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"a0dd22eeddc7213516a16dc14ef9c46f3a7ec4f8","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        marker, limit, offset \u003d common.get_pagination_params(filters)"},{"line_number":167,"context_line":"        sort_keys, sort_dirs \u003d common.get_sort_params(filters)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        filters.pop(\u0027list_volume\u0027, None)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        groups \u003d self.group_api.get_all("},{"line_number":172,"context_line":"            context, filters\u003dfilters, marker\u003dmarker, limit\u003dlimit,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffe62b97_1c658a1e","line":169,"range":{"start_line":169,"start_character":8,"end_line":169,"end_character":40},"in_reply_to":"9a30ddce_d8b3504b","updated":"2017-03-28 01:30:01.000000000","message":"@John, it is used here:\nhttps://github.com/openstack/cinder/blob/master/cinder/api/v3/views/groups.py#L69-L72","commit_id":"57930687b65aa11bbe1b724ec0397af5eb441337"}],"cinder/tests/unit/api/v3/test_groups.py":[{"author":{"_account_id":23276,"name":"Nicholas Jones","email":"nj762h@att.com","username":"nj762h"},"change_message_id":"77d44c0c65fbb9b19ea6051458466fb82fc74ab0","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # \"volumes\" should not be contained in the response body when list"},{"line_number":191,"context_line":"        # groups without detail."},{"line_number":192,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v3/%s/groups/detail?list_volume\u003dTrue\u0027 %"},{"line_number":193,"context_line":"                                      fake.PROJECT_ID,"},{"line_number":194,"context_line":"                                      version\u003d\u00273.25\u0027)"},{"line_number":195,"context_line":"        res_dict \u003d self.controller.index(req)"},{"line_number":196,"context_line":"        self.assertEqual(3, len(res_dict[\u0027groups\u0027]))"},{"line_number":197,"context_line":"        self.assertIsNone(res_dict[\u0027groups\u0027][0].get(\u0027volumes\u0027, None))"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_7a23a9a7","line":194,"range":{"start_line":192,"start_character":7,"end_line":194,"end_character":53},"updated":"2017-02-13 19:34:39.000000000","message":"This is the same request on 164, I\u0027m not quite sure why you\u0027re expecting different results, and I could be missing something here.","commit_id":"277a65bac2fb0dd591fa1d6f2e9c2e7a800f3b65"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"b0ab71c9262438749e2728addd8d32aaff5c473f","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # \"volumes\" should not be contained in the response body when list"},{"line_number":191,"context_line":"        # groups without detail."},{"line_number":192,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v3/%s/groups/detail?list_volume\u003dTrue\u0027 %"},{"line_number":193,"context_line":"                                      fake.PROJECT_ID,"},{"line_number":194,"context_line":"                                      version\u003d\u00273.25\u0027)"},{"line_number":195,"context_line":"        res_dict \u003d self.controller.index(req)"},{"line_number":196,"context_line":"        self.assertEqual(3, len(res_dict[\u0027groups\u0027]))"},{"line_number":197,"context_line":"        self.assertIsNone(res_dict[\u0027groups\u0027][0].get(\u0027volumes\u0027, None))"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_e4aff327","line":194,"range":{"start_line":192,"start_character":7,"end_line":194,"end_character":53},"in_reply_to":"fa31d9ce_7a23a9a7","updated":"2017-02-14 01:16:54.000000000","message":"It\u0027s a mistake here. The request here should not contain \"detail\". Thanks!\n\nyou can see the difference between L195 and L167. It\u0027s the reason why the result is different. index request at L195 just returns the group\u0027s id and name, so the volumes should not be contained there. And detail request returns all the group\u0027s information, so the volumes should be there.","commit_id":"277a65bac2fb0dd591fa1d6f2e9c2e7a800f3b65"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"45f6f5d6ff0176f9cb793ec37aa19fa59a87ae0b","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                                      version\u003d\u00273.25\u0027)"},{"line_number":177,"context_line":"        res_dict \u003d self.controller.detail(req)"},{"line_number":178,"context_line":"        self.assertEqual(3, len(res_dict[\u0027groups\u0027]))"},{"line_number":179,"context_line":"        self.assertIsNone(res_dict[\u0027groups\u0027][0].get(\u0027volumes\u0027, None))"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        # If the microversion \u003c 3.25, \"volumes\" should not be contained in the"},{"line_number":182,"context_line":"        # response body."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_1c32ae10","line":179,"updated":"2017-03-09 09:33:35.000000000","message":"-1: Each test should be in its own method, not 4 tests in one method, unless you are using ddt","commit_id":"3d3a2751b458b5997f24535285e2cbe4c24f1999"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"c09c2ad11aa1993821f000d02f7d3d4ab35636de","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                                      version\u003d\u00273.25\u0027)"},{"line_number":177,"context_line":"        res_dict \u003d self.controller.detail(req)"},{"line_number":178,"context_line":"        self.assertEqual(3, len(res_dict[\u0027groups\u0027]))"},{"line_number":179,"context_line":"        self.assertIsNone(res_dict[\u0027groups\u0027][0].get(\u0027volumes\u0027, None))"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        # If the microversion \u003c 3.25, \"volumes\" should not be contained in the"},{"line_number":182,"context_line":"        # response body."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a30ddce_5a7d066d","line":179,"in_reply_to":"9a30ddce_1c32ae10","updated":"2017-03-09 10:06:19.000000000","message":"rewrote with ddt.","commit_id":"3d3a2751b458b5997f24535285e2cbe4c24f1999"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd0fd02c00ae7808b64b361f42f53959546bc8","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                                           objects\u003dvol_type_objs)"},{"line_number":163,"context_line":"        mock_vol_type_get_all_by_group.return_value \u003d vol_types"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v3/%s/groups/detail\u0027 %"},{"line_number":166,"context_line":"                                      fake.PROJECT_ID,"},{"line_number":167,"context_line":"                                      version\u003dversion)"},{"line_number":168,"context_line":"        if has_list_volume:"},{"line_number":169,"context_line":"            req \u003d fakes.HTTPRequest.blank("},{"line_number":170,"context_line":"                \u0027/v3/%s/groups/detail?list_volume\u003dTrue\u0027 % fake.PROJECT_ID,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_b0d703be","line":167,"range":{"start_line":165,"start_character":0,"end_line":167,"end_character":54},"updated":"2017-03-09 11:03:45.000000000","message":"nit: This should go in the else clause","commit_id":"ff5efd7b17120dbe20543661f769ad20172841d9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd0fd02c00ae7808b64b361f42f53959546bc8","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        if has_list_volume:"},{"line_number":169,"context_line":"            req \u003d fakes.HTTPRequest.blank("},{"line_number":170,"context_line":"                \u0027/v3/%s/groups/detail?list_volume\u003dTrue\u0027 % fake.PROJECT_ID,"},{"line_number":171,"context_line":"                version\u003d\u00273.25\u0027)"},{"line_number":172,"context_line":"        res_dict \u003d self.controller.detail(req)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        # If the microversion \u003e\u003d 3.25 and \"list_volume\u003dTrue\", \"volumes\" should"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_10c8cf21","line":171,"updated":"2017-03-09 11:03:45.000000000","message":"-1: This doesn\u0027t seem reasonable, you are literally ignoring the version parameter based on the on the has_list_volume parameter, so you are actually testing the same thing in 2 of the test cases.","commit_id":"ff5efd7b17120dbe20543661f769ad20172841d9"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"113a4240bdcd8038b5fdd64539f3d3098416c935","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        if has_list_volume:"},{"line_number":169,"context_line":"            req \u003d fakes.HTTPRequest.blank("},{"line_number":170,"context_line":"                \u0027/v3/%s/groups/detail?list_volume\u003dTrue\u0027 % fake.PROJECT_ID,"},{"line_number":171,"context_line":"                version\u003d\u00273.25\u0027)"},{"line_number":172,"context_line":"        res_dict \u003d self.controller.detail(req)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        # If the microversion \u003e\u003d 3.25 and \"list_volume\u003dTrue\", \"volumes\" should"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_6bdbd94f","line":171,"in_reply_to":"9a30ddce_10c8cf21","updated":"2017-03-09 13:42:17.000000000","message":"It\u0027a mistake.","commit_id":"ff5efd7b17120dbe20543661f769ad20172841d9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd0fd02c00ae7808b64b361f42f53959546bc8","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        # be contained in the response body. Else,\"volumes\" should not be"},{"line_number":176,"context_line":"        # contained in the response body."},{"line_number":177,"context_line":"        if (version, has_list_volume) \u003d\u003d (\u00273.25\u0027, True):"},{"line_number":178,"context_line":"            self.assertEqual(3, len(res_dict[\u0027groups\u0027]))"},{"line_number":179,"context_line":"            self.assertEqual([fake.VOLUME_ID],"},{"line_number":180,"context_line":"                             res_dict[\u0027groups\u0027][0][\u0027volumes\u0027])"},{"line_number":181,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_5036c752","line":178,"updated":"2017-03-09 11:03:45.000000000","message":"-1: This is common to both cases","commit_id":"ff5efd7b17120dbe20543661f769ad20172841d9"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"113a4240bdcd8038b5fdd64539f3d3098416c935","unresolved":false,"context_lines":[{"line_number":175,"context_line":"        # be contained in the response body. Else,\"volumes\" should not be"},{"line_number":176,"context_line":"        # contained in the response body."},{"line_number":177,"context_line":"        if (version, has_list_volume) \u003d\u003d (\u00273.25\u0027, True):"},{"line_number":178,"context_line":"            self.assertEqual(3, len(res_dict[\u0027groups\u0027]))"},{"line_number":179,"context_line":"            self.assertEqual([fake.VOLUME_ID],"},{"line_number":180,"context_line":"                             res_dict[\u0027groups\u0027][0][\u0027volumes\u0027])"},{"line_number":181,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a30ddce_8be00dfc","line":178,"in_reply_to":"9a30ddce_5036c752","updated":"2017-03-09 13:42:17.000000000","message":"ditto","commit_id":"ff5efd7b17120dbe20543661f769ad20172841d9"}]}
