)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17444,"name":"Jay.Shen","email":"shenhong09@huawei.com","username":"Jay.Shen"},"change_message_id":"b0a11e4ba6d5480907c79b13325dc6a2a9311ff5","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support \u0027--all-tenants\u0027 for cinder backup-list"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"cinder backup-list is always listing all tenants\u0027s backups for admin."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"APIImpact"},{"line_number":12,"context_line":"Closes-Bug: #1422046"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a4dcd0f_3c626dae","line":9,"updated":"2015-08-04 14:29:01.000000000","message":"S/cinder/Cinder","commit_id":"02a568230d560def5f5eb5b9aa6d927f1e674cce"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yuriy Nesenenko \u003cynesenenko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-08-17 14:09:33 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support \u0027--all-tenants\u0027 for cinder backup-list"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Cinder backup-list doesn\u0027t support \u0027--all-tenants\u0027 argument for admin."},{"line_number":10,"context_line":"So it is always listing all tenants\u0027s backups for admin not only admin\u0027s"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"fa1b9901_09ee9522","line":7,"updated":"2015-08-17 12:21:05.000000000","message":"It\u0027s adding more commands, not only --all-tenants, it should be mentioned in the commit message.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add support \u0027--all-tenants\u0027 for cinder backup-list"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Cinder backup-list doesn\u0027t support \u0027--all-tenants\u0027 argument for admin."},{"line_number":10,"context_line":"So it is always listing all tenants\u0027s backups for admin not only admin\u0027s"},{"line_number":11,"context_line":"tenant. This patch fixes it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"APIImpact"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"fa1b9901_0e09bb95","line":10,"updated":"2015-08-17 12:21:05.000000000","message":"Are you sure it is always listing all tenant\u0027s backups?  Because looking at the server code it should only be returning backups by the project_id","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":10,"context_line":"So it is always listing all tenants\u0027s backups for admin not only admin\u0027s"},{"line_number":11,"context_line":"tenant. This patch fixes it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"APIImpact"},{"line_number":14,"context_line":"Closes-Bug: #1422046"},{"line_number":15,"context_line":"Depends On: I73f6377c7d6fd92d0464d13f9c8dd6682fef78e3"},{"line_number":16,"context_line":"Change-Id: I5f2ab6370a8333a9ee498c6158037b0433f36a23"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"fa1b9901_29b9192f","line":13,"updated":"2015-08-17 12:21:05.000000000","message":"This has DocImpact, but no APIImpact, it\u0027s changing the client not the server.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"}],"cinderclient/tests/unit/v1/test_shell.py":[{"author":{"_account_id":7764,"name":"Peter Hamilton","email":"peter.hamilton@jhuapl.edu","username":"Peter"},"change_message_id":"cdd5681f05877ad88c509090d94ec4e4b5407718","unresolved":false,"context_lines":[{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    def test_backup_list_all_tenants(self):"},{"line_number":506,"context_line":"        self.run_command(\u0027backup-list --all-tenants\u003d1\u0027)"},{"line_number":507,"context_line":"        self.assert_called(\u0027GET\u0027, \u0027/backups/detail?all_tenants\u003d1\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_0d940ce6","line":507,"updated":"2015-07-31 14:33:15.000000000","message":"The tests should also cover --display-name, --status, and --volume-id.","commit_id":"ef94a0e7950f0912258c1d7ad768a696b70fed52"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"2c11eb155282558fe91942cf5a50ee7c33bdd814","unresolved":false,"context_lines":[{"line_number":504,"context_line":""},{"line_number":505,"context_line":"    def test_backup_list_all_tenants(self):"},{"line_number":506,"context_line":"        self.run_command(\u0027backup-list --all-tenants\u003d1\u0027)"},{"line_number":507,"context_line":"        self.assert_called(\u0027GET\u0027, \u0027/backups/detail?all_tenants\u003d1\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_63527f37","line":507,"in_reply_to":"3a50d1a3_0d940ce6","updated":"2015-07-31 15:12:11.000000000","message":"This is out of scope of this patch.","commit_id":"ef94a0e7950f0912258c1d7ad768a696b70fed52"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":499,"context_line":"        self.assert_called(\u0027GET\u0027, \u0027/os-volume-transfer/detail?all_tenants\u003d1\u0027)"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"    def test_backup_list_all_tenants(self):"},{"line_number":502,"context_line":"        self.run_command(\u0027backup-list --all-tenants\u003d1 --display-name\u003dbc \u0027"},{"line_number":503,"context_line":"                         \u0027--status\u003davailable --volume-id\u003d1234\u0027)"},{"line_number":504,"context_line":"        self.assert_called(\u0027GET\u0027, \u0027/backups/detail?\u0027"},{"line_number":505,"context_line":"                                  \u0027all_tenants\u003d1\u0026display_name\u003dbc\u0026\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_0e7e9be4","line":502,"updated":"2015-08-17 12:21:05.000000000","message":"You should add a test to check your request when all-tenants is not specified.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"}],"cinderclient/tests/unit/v2/test_shell.py":[{"author":{"_account_id":7764,"name":"Peter Hamilton","email":"peter.hamilton@jhuapl.edu","username":"Peter"},"change_message_id":"cdd5681f05877ad88c509090d94ec4e4b5407718","unresolved":false,"context_lines":[{"line_number":954,"context_line":""},{"line_number":955,"context_line":"    def test_backup_list_all_tenants(self):"},{"line_number":956,"context_line":"        self.run_command(\u0027backup-list --all-tenants\u003d1\u0027)"},{"line_number":957,"context_line":"        self.assert_called(\u0027GET\u0027, \u0027/backups/detail?all_tenants\u003d1\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_4d9214f8","line":957,"updated":"2015-07-31 14:33:15.000000000","message":"The tests should also cover --display-name, --status, and --volume-id.","commit_id":"ef94a0e7950f0912258c1d7ad768a696b70fed52"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"2c11eb155282558fe91942cf5a50ee7c33bdd814","unresolved":false,"context_lines":[{"line_number":954,"context_line":""},{"line_number":955,"context_line":"    def test_backup_list_all_tenants(self):"},{"line_number":956,"context_line":"        self.run_command(\u0027backup-list --all-tenants\u003d1\u0027)"},{"line_number":957,"context_line":"        self.assert_called(\u0027GET\u0027, \u0027/backups/detail?all_tenants\u003d1\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_03493b8e","line":957,"in_reply_to":"3a50d1a3_4d9214f8","updated":"2015-07-31 15:12:11.000000000","message":"I agree with you but this is out of scope of this patch.","commit_id":"ef94a0e7950f0912258c1d7ad768a696b70fed52"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a06cf896d917c59ef450b51b8972b32765c5fc72","unresolved":false,"context_lines":[{"line_number":948,"context_line":"        self.assert_called(\u0027POST\u0027, \u0027/volumes/1234/action\u0027,"},{"line_number":949,"context_line":"                           body\u003dexpected)"},{"line_number":950,"context_line":""},{"line_number":951,"context_line":"    def test_backup_list_all_tenants(self):"},{"line_number":952,"context_line":"        self.run_command(\u0027backup-list --all-tenants\u003d1 --display-name\u003dbc \u0027"},{"line_number":953,"context_line":"                         \u0027--status\u003davailable --volume-id\u003d1234\u0027)"},{"line_number":954,"context_line":"        expected \u003d {"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_38d59eb8","line":951,"range":{"start_line":951,"start_character":8,"end_line":951,"end_character":36},"updated":"2015-08-18 17:25:22.000000000","message":"Maybe add tests for --all-tenants\u003d0 and --all-tenants without a value.","commit_id":"5348d8d5e6a3cafce63c0941715e00e9945bd5b9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a06cf896d917c59ef450b51b8972b32765c5fc72","unresolved":false,"context_lines":[{"line_number":958,"context_line":"            \u0027volume_id\u0027: [\u00271234\u0027],"},{"line_number":959,"context_line":"        }"},{"line_number":960,"context_line":""},{"line_number":961,"context_line":"        method, url \u003d self.shell.cs.client.callstack[-1][0:2]"},{"line_number":962,"context_line":"        path, query \u003d parse.splitquery(url)"},{"line_number":963,"context_line":"        params \u003d parse.parse_qs(query)"},{"line_number":964,"context_line":"        self.assertEqual(\u0027/backups/detail\u0027, path)"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_58617238","line":961,"range":{"start_line":961,"start_character":22,"end_line":961,"end_character":26},"updated":"2015-08-18 17:25:22.000000000","message":"You can create a method similar to assert_called that does what you are doing here, so others may benefit from your code in other tests.","commit_id":"5348d8d5e6a3cafce63c0941715e00e9945bd5b9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a06cf896d917c59ef450b51b8972b32765c5fc72","unresolved":false,"context_lines":[{"line_number":964,"context_line":"        self.assertEqual(\u0027/backups/detail\u0027, path)"},{"line_number":965,"context_line":""},{"line_number":966,"context_line":"        for k in params.keys():"},{"line_number":967,"context_line":"            self.assertEqual(expected[k], params[k])"},{"line_number":968,"context_line":""},{"line_number":969,"context_line":"    def test_backup_list_volume_id(self):"},{"line_number":970,"context_line":"        self.run_command(\u0027backup-list --volume-id\u003d1234\u0027)"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_98f38a9e","line":967,"range":{"start_line":967,"start_character":17,"end_line":967,"end_character":28},"updated":"2015-08-18 17:25:22.000000000","message":"Should check we have the same number of keys in params as we do in expected.","commit_id":"5348d8d5e6a3cafce63c0941715e00e9945bd5b9"}],"cinderclient/v1/shell.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    _translate_keys(collection, convert)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"def _translate_volume_backup_keys(collection):"},{"line_number":115,"context_line":"    convert \u003d [(\u0027displayName\u0027, \u0027display_name\u0027), (\u0027volumeId\u0027, \u0027volume_id\u0027)]"},{"line_number":116,"context_line":"    _translate_keys(collection, convert)"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_2ee59fa7","line":114,"updated":"2015-08-17 12:21:05.000000000","message":"This is the same as for snapshot, so you should do some refactor to use the same method.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":922,"context_line":"           nargs\u003d\u0027?\u0027,"},{"line_number":923,"context_line":"           type\u003dint,"},{"line_number":924,"context_line":"           const\u003d1,"},{"line_number":925,"context_line":"           default\u003d0,"},{"line_number":926,"context_line":"           help\u003d\u0027Shows details for all tenants. Admin only.\u0027)"},{"line_number":927,"context_line":"@utils.arg(\u0027--all_tenants\u0027,"},{"line_number":928,"context_line":"           nargs\u003d\u0027?\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_0e9a1b15","line":925,"updated":"2015-08-17 12:21:05.000000000","message":"It it\u0027s really returning now all tenants, then the default here should be 1 and not 0 to keep it backward compatible.\n\nAlthough like I mentioned in the commit message I think it shouldn\u0027t be returning all-tenants right now.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":947,"context_line":"    all_tenants \u003d int(os.environ.get(\"ALL_TENANTS\", args.all_tenants))"},{"line_number":948,"context_line":""},{"line_number":949,"context_line":"    search_opts \u003d {"},{"line_number":950,"context_line":"        \u0027all_tenants\u0027: all_tenants,"},{"line_number":951,"context_line":"        \u0027display_name\u0027: args.display_name,"},{"line_number":952,"context_line":"        \u0027status\u0027: args.status,"},{"line_number":953,"context_line":"        \u0027volume_id\u0027: args.volume_id,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_2eaa7f1a","line":950,"updated":"2015-08-17 12:21:05.000000000","message":"Have you tested this patch to confirm that you don\u0027t see all tenants when you don\u0027t specify all-tenants argument?\n\nBecause looking at the server side code there seems to be a bug in there and with the code in this patch as it is you will *always* get backups for all tenants.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"}],"cinderclient/v1/volume_backups.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        qparams \u003d {}"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        for opt, val in six.iteritems(search_opts):"},{"line_number":74,"context_line":"            if val:"},{"line_number":75,"context_line":"                qparams[opt] \u003d val"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_299fd960","line":73,"updated":"2015-08-17 12:21:05.000000000","message":"You can use dict comprehension:\n\n qparams \u003d {opt: val for opt, val in search_opts.items() if val}\n\nAnd you should use search_opts.items() instead of six.iteritems because that\u0027s how it was decided to make code python3 compatible in Cinder.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                qparams[opt] \u003d val"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        if qparams:"},{"line_number":78,"context_line":"            new_qparams \u003d sorted(qparams.items(), key\u003dlambda x: x[0])"},{"line_number":79,"context_line":"            query_string \u003d \"?%s\" % urlencode(new_qparams)"},{"line_number":80,"context_line":"        else:"},{"line_number":81,"context_line":"            query_string \u003d \"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_091775a4","line":78,"updated":"2015-08-17 12:21:05.000000000","message":"Why do we want to sort the parameters?","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"439f80de39d87248761573abc83701a0c5646a3f","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                qparams[opt] \u003d val"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        if qparams:"},{"line_number":78,"context_line":"            new_qparams \u003d sorted(qparams.items(), key\u003dlambda x: x[0])"},{"line_number":79,"context_line":"            query_string \u003d \"?%s\" % urlencode(new_qparams)"},{"line_number":80,"context_line":"        else:"},{"line_number":81,"context_line":"            query_string \u003d \"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_efbb2174","line":78,"in_reply_to":"fa1b9901_091775a4","updated":"2015-08-17 13:19:06.000000000","message":"Because in tests we\u0027d got : AssertionError: Expected GET /backups/detail?all_tenants\u003d1\u0026display_name\u003dbc\u0026status\u003davailable\u0026volume_id\u003d1234; got GET /backups/detail?status\u003davailable\u0026all_tenants\u003d1\u0026display_name\u003dbc\u0026volume_id\u003d1234","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"633308dcf9e296035f5c4ca00362852c2247d366","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                qparams[opt] \u003d val"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        if qparams:"},{"line_number":78,"context_line":"            new_qparams \u003d sorted(qparams.items(), key\u003dlambda x: x[0])"},{"line_number":79,"context_line":"            query_string \u003d \"?%s\" % urlencode(new_qparams)"},{"line_number":80,"context_line":"        else:"},{"line_number":81,"context_line":"            query_string \u003d \"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_012bc9ec","line":78,"in_reply_to":"fa1b9901_efbb2174","updated":"2015-08-17 13:22:46.000000000","message":"Then tests need fixing, you don\u0027t adapt your code to the tests, tests must adapt to the code.\n\nSorting here must be removed.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        else:"},{"line_number":81,"context_line":"            query_string \u003d \"\""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        detail \u003d \"\""},{"line_number":84,"context_line":"        if detailed:"},{"line_number":85,"context_line":"            detail \u003d \"/detail\""},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_09cc5534","line":83,"updated":"2015-08-17 12:21:05.000000000","message":"Why not include this in an else of `if detailed`?\n\nOr do:\n\n detail \u003d \u0027/detail\u0027 if detailed else \u0027\u0027","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"}],"cinderclient/v2/shell.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":1240,"context_line":"           nargs\u003d\u0027?\u0027,"},{"line_number":1241,"context_line":"           type\u003dint,"},{"line_number":1242,"context_line":"           const\u003d1,"},{"line_number":1243,"context_line":"           default\u003d0,"},{"line_number":1244,"context_line":"           help\u003d\u0027Shows details for all tenants. Admin only.\u0027)"},{"line_number":1245,"context_line":"@utils.arg(\u0027--all_tenants\u0027,"},{"line_number":1246,"context_line":"           nargs\u003d\u0027?\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_ae43ef6b","line":1243,"updated":"2015-08-17 12:21:05.000000000","message":"Same as with v1 shell.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":1272,"context_line":"    all_tenants \u003d int(os.environ.get(\"ALL_TENANTS\", args.all_tenants))"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"    search_opts \u003d {"},{"line_number":1275,"context_line":"        \u0027all_tenants\u0027: all_tenants,"},{"line_number":1276,"context_line":"        \u0027display_name\u0027: args.name,"},{"line_number":1277,"context_line":"        \u0027status\u0027: args.status,"},{"line_number":1278,"context_line":"        \u0027volume_id\u0027: args.volume_id,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_693ab1cf","line":1275,"updated":"2015-08-17 12:21:05.000000000","message":"Same as with v1.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"}],"cinderclient/v2/volume_backups.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        qparams \u003d {}"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        for opt, val in six.iteritems(search_opts):"},{"line_number":77,"context_line":"            if val:"},{"line_number":78,"context_line":"                qparams[opt] \u003d val"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_8992a556","line":76,"updated":"2015-08-17 12:21:05.000000000","message":"You can use dict comprehension:\n\n qparams \u003d {opt: val for opt, val in search_opts.items() if val}\n\nAnd you should use search_opts.items() instead of six.iteritems because that\u0027s how it was decided to make code python3 compatible in Cinder.","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                qparams[opt] \u003d val"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        if qparams:"},{"line_number":81,"context_line":"            new_qparams \u003d sorted(qparams.items(), key\u003dlambda x: x[0])"},{"line_number":82,"context_line":"            query_string \u003d \"?%s\" % urlencode(new_qparams)"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            query_string \u003d \"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_294919d1","line":81,"updated":"2015-08-17 12:21:05.000000000","message":"Why do we want to sort the parameters?","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"31360d92e3308b31fe67698ef49da6150dde80b6","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            query_string \u003d \"\""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        detail \u003d \"\""},{"line_number":87,"context_line":"        if detailed:"},{"line_number":88,"context_line":"            detail \u003d \"/detail\""},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa1b9901_494b2db1","line":86,"updated":"2015-08-17 12:21:05.000000000","message":"Why not include this in an else of `if detailed`?\n\nOr do:\n\n detail \u003d \u0027/detail\u0027 if detailed else \u0027\u0027","commit_id":"ac33ccecf6dd5d36ca48ff90e973cd736d77017f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a99d280ba1edf941fe850ca28a80c633df25e602","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        qparams \u003d dict((key, val) for key, val in search_opts.items() if val)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        if qparams:"},{"line_number":76,"context_line":"            new_qparams \u003d sorted(qparams.items(), key\u003dlambda x: x[0])"},{"line_number":77,"context_line":"            query_string \u003d \"?%s\" % urlencode(new_qparams)"},{"line_number":78,"context_line":"        else:"},{"line_number":79,"context_line":"            query_string \u003d \"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"fa1b9901_778d9c7a","line":76,"updated":"2015-08-17 17:46:23.000000000","message":"Sorting should be removed, we don\u0027t need it.","commit_id":"a4c22e6c8e3ca143ce3ee98a7d721b2c4d294d46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3331a63d3e85a35d1d878c3c3f576de1713a8453","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        qparams \u003d dict((key, val) for key, val in search_opts.items() if val)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        # Transform the dict to a sequence of two-element tuples in fixed"},{"line_number":76,"context_line":"        # order, then the encoded string will be consistent in Python 2\u00263."},{"line_number":77,"context_line":"        if qparams:"},{"line_number":78,"context_line":"            new_qparams \u003d sorted(qparams.items(), key\u003dlambda x: x[0])"},{"line_number":79,"context_line":"            query_string \u003d \"?%s\" % urlencode(new_qparams)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa1b9901_330dd83e","line":76,"updated":"2015-08-18 09:44:28.000000000","message":"We don\u0027t need to get the exact same query string in python 2 and 3 as long as all the elements are there with the same values, because query strings have the commutative property and order is irrelevant.\n\nAs I mentioned before, this is a test issue, and server code should not be bound by laziness in test creation.\n\nTo solve the test issue comparison can be done by parsing the url string and comparing the paths and parameters\u0027 dictionaries instead of using a simple string comparison.  Yes it\u0027s a little more work, but that\u0027s the right way to do it.","commit_id":"4d0f3966ab8fe5db7bd88e5f808dd2f4395b697b"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"041aa77149053b5046977fb9220d701633063995","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        qparams \u003d dict((key, val) for key, val in search_opts.items() if val)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        # Transform the dict to a sequence of two-element tuples in fixed"},{"line_number":76,"context_line":"        # order, then the encoded string will be consistent in Python 2\u00263."},{"line_number":77,"context_line":"        if qparams:"},{"line_number":78,"context_line":"            new_qparams \u003d sorted(qparams.items(), key\u003dlambda x: x[0])"},{"line_number":79,"context_line":"            query_string \u003d \"?%s\" % urlencode(new_qparams)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa1b9901_79825102","line":76,"in_reply_to":"fa1b9901_330dd83e","updated":"2015-08-18 10:08:34.000000000","message":"I agree with you, but it should be done for volumes and snapshots as well and get it standardized.  Look at https://github.com/openstack/python-cinderclient/blob/master/cinderclient/v2/volumes.py#L355 and https://github.com/openstack/python-cinderclient/blob/master/cinderclient/v2/volume_snapshots.py#L119 This is not the task of this scope. I think it should be done another patch.","commit_id":"4d0f3966ab8fe5db7bd88e5f808dd2f4395b697b"}]}