)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"9ebd084ed03d3facb352170fe6ff68864094c816","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a1cceea2_37999f00","updated":"2023-08-16 05:33:26.000000000","message":"thanks for the review.","commit_id":"7daa2f9d1439b82f9f8452fca63135ad42ee43f6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"24b068b1_8bece2cb","updated":"2023-08-17 13:47:55.000000000","message":"Thank you for negotiating the various opinions on this! I think the principle is good, but there\u0027s a couple of issues to fix - I left suggestions here https://review.opendev.org/c/openstack/swift/+/891723 for you to squash if you are happy with them","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"36105f1106538bc4b64abcf04a9fd2b88dce9ee7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"7e35f573_ef4df6ef","updated":"2023-08-17 05:24:29.000000000","message":"thanks for the reviews!","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e2bef0f2779f4eae4e9b3e0eb93829f0b04ade16","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d4140952_b7061a8f","updated":"2023-08-18 09:15:22.000000000","message":"Thanks Jianjian\n\nI tried this out on a VSAIO.\n\nRan a (hacked) probe test to get a sharded container, then extra_params let\u0027s me direct get the shards, with some constraints:\n\n```\n\u003e\u003e\u003e hdrs, shards\u003ddc.direct_get_container(nodes[0], 617, \u0027AUTH_test\u0027, \u0027container-b130d716-52fe-4071-90a0-dab0b8f37676\u0027, headers\u003d{\u0027X-Backend-Record-Type\u0027: \u0027shard\u0027, \u0027X-Backend-Include-Deleted\u0027: \u0027true\u0027})\n\n\u003e\u003e\u003e print(\u0027\\n\u0027.join([str(utils.ShardRange.from_dict(s)) for s in shards]))\nShardRange\u003cMinBound to \u0027beta024\u0027 as of 1692348755.11384, (0, 0) as of 1692348776.68700, active as of 1692348755.11384\u003e\nShardRange\u003c\u0027beta024\u0027 to \u0027beta049\u0027 as of 1692348755.11384, (0, 0) as of 1692348776.71879, active as of 1692348755.11384\u003e\nShardRange\u003c\u0027beta049\u0027 to \u0027obj-0024\u0027 as of 1692348755.11384, (0, 0) as of 1692348776.75351, active as of 1692348755.11384\u003e\nShardRange\u003cMinBound to \u0027obj-0024\u0027 as of 1692348762.94916, (0, 0) as of 1692348762.95894, sharded as of 1692348762.94916\u003e\nShardRange\u003c\u0027obj-0024\u0027 to MaxBound as of 1692348749.79366, (0, 0) as of 1692348776.62610, active as of 1692348749.79366\u003e\n\n\u003e\u003e\u003e hdrs, shards\u003ddc.direct_get_container(nodes[0], 617, \u0027AUTH_test\u0027, \u0027container-b130d716-52fe-4071-90a0-dab0b8f37676\u0027, headers\u003d{\u0027X-Backend-Record-Type\u0027: \u0027shard\u0027, \u0027X-Backend-Include-Deleted\u0027: \u0027true\u0027}, extra_params\u003d{\u0027states\u0027: \u0027sharded\u0027, \u0027reverse\u0027: \u0027yes\u0027})\n\n\u003e\u003e\u003e print(\u0027\\n\u0027.join([str(utils.ShardRange.from_dict(s)) for s in shards]))\nShardRange\u003cMinBound to \u0027obj-0024\u0027 as of 1692348762.94916, (0, 0) as of 1692348762.95894, sharded as of 1692348762.94916\u003e\n\n\u003e\u003e\u003e hdrs, shards\u003ddc.direct_get_container(nodes[0], 617, \u0027AUTH_test\u0027, \u0027container-b130d716-52fe-4071-90a0-dab0b8f37676\u0027, headers\u003d{\u0027X-Backend-Record-Type\u0027: \u0027shard\u0027, \u0027X-Backend-Include-Deleted\u0027: \u0027true\u0027}, extra_params\u003d{\u0027states\u0027: \u0027active\u0027, \u0027reverse\u0027: \u0027yes\u0027})\n\n\u003e\u003e\u003e print(\u0027\\n\u0027.join([str(utils.ShardRange.from_dict(s)) for s in shards]))\nShardRange\u003c\u0027obj-0024\u0027 to MaxBound as of 1692348749.79366, (0, 0) as of 1692348776.62610, active as of 1692348749.79366\u003e\nShardRange\u003c\u0027beta049\u0027 to \u0027obj-0024\u0027 as of 1692348755.11384, (0, 0) as of 1692348776.75351, active as of 1692348755.11384\u003e\nShardRange\u003c\u0027beta024\u0027 to \u0027beta049\u0027 as of 1692348755.11384, (0, 0) as of 1692348776.71879, active as of 1692348755.11384\u003e\nShardRange\u003cMinBound to \u0027beta024\u0027 as of 1692348755.11384, (0, 0) as of 1692348776.68700, active as of 1692348755.11384\u003e\n\n\u003e\u003e\u003e hdrs, shards\u003ddc.direct_get_container(nodes[0], 617, \u0027AUTH_test\u0027, \u0027container-b130d716-52fe-4071-90a0-dab0b8f37676\u0027, headers\u003d{\u0027X-Backend-Record-Type\u0027: \u0027shard\u0027, \u0027X-Backend-Include-Deleted\u0027: \u0027true\u0027}, extra_params\u003d{\u0027states\u0027: \u0027active\u0027, \u0027includes\u0027: \u0027obj-101\u0027})\n\n\u003e\u003e\u003e print(\u0027\\n\u0027.join([str(utils.ShardRange.from_dict(s)) for s in shards]))\nShardRange\u003c\u0027obj-0024\u0027 to MaxBound as of 1692348749.79366, (0, 0) as of 1692348776.62610, active as of 1692348749.79366\u003e\n\n\u003e\u003e\u003e hdrs, shards\u003ddc.direct_get_container(nodes[0], 617, \u0027AUTH_test\u0027, \u0027container-b130d716-52fe-4071-90a0-dab0b8f37676\u0027, headers\u003d{\u0027X-Backend-Record-Type\u0027: \u0027shard\u0027, \u0027X-Backend-Include-Deleted\u0027: \u0027true\u0027}, extra_params\u003d{\u0027states\u0027: \u0027active\u0027, \u0027marker\u0027: \u0027c\u0027})\n\n\u003e\u003e\u003e print(\u0027\\n\u0027.join([str(utils.ShardRange.from_dict(s)) for s in shards]))\nShardRange\u003c\u0027beta049\u0027 to \u0027obj-0024\u0027 as of 1692348755.11384, (0, 0) as of 1692348776.75351, active as of 1692348755.11384\u003e\nShardRange\u003c\u0027obj-0024\u0027 to MaxBound as of 1692348749.79366, (0, 0) as of 1692348776.62610, active as of 1692348749.79366\u003e\n\n\u003e\u003e\u003e hdrs, shards\u003ddc.direct_get_container(nodes[0], 617, \u0027AUTH_test\u0027, \u0027container-b130d716-52fe-4071-90a0-dab0b8f37676\u0027, headers\u003d{\u0027X-Backend-Record-Type\u0027: \u0027shard\u0027, \u0027X-Backend-Include-Deleted\u0027: \u0027true\u0027}, extra_params\u003d{\u0027states\u0027: \u0027active\u0027, \u0027marker\u0027: \u0027c\u0027}, marker\u003d\u0027c\u0027)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"/vagrant/swift/swift/common/direct_client.py\", line 340, in direct_get_container\n    return _get_direct_account_container(path, \"Container\", node,\n  File \"/vagrant/swift/swift/common/direct_client.py\", line 176, in _get_direct_account_container\n    raise TypeError(\u0027duplicate values for keyword arg: marker\u0027)\nTypeError: duplicate values for keyword arg: marker\n```","commit_id":"ac524832e99b63edbad2f0984a6ced56d9dae49e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"f3f6f02d193a901f8df35a8dc70d0a7afeae8dd2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"70850c03_88add35f","updated":"2023-08-17 21:42:33.000000000","message":"recheck\n\"connection TimeoutError: timed out\" in temptest-ipv6-only","commit_id":"ac524832e99b63edbad2f0984a6ced56d9dae49e"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"302c7673_53523f69","updated":"2023-08-17 18:58:26.000000000","message":"thanks for fixing those failed test cases for py27!","commit_id":"ac524832e99b63edbad2f0984a6ced56d9dae49e"}],"swift/common/direct_client.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"932df84f5c6449f00cf85fb959ea68d5546830da","unresolved":true,"context_lines":[{"line_number":336,"context_line":"        for each individual use case. For example, [\u0027states\u003dupdating\u0027] can be"},{"line_number":337,"context_line":"        used with shard_range/namespace listing. Note: common parameters passed"},{"line_number":338,"context_line":"        into ``extra_params`` will be ignored, and this interface will only"},{"line_number":339,"context_line":"        verify the basic format of extra parameters."},{"line_number":340,"context_line":"    :returns: a tuple of (response headers, a list of objects) The response"},{"line_number":341,"context_line":"              headers will be a HeaderKeyDict."},{"line_number":342,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"f6a2ff20_cfc492f5","line":339,"updated":"2023-08-15 13:22:10.000000000","message":"I\u0027d prefer to pass in a dict of params.\n\nAlso, I would suggest that it should be an error to repeat a param as a keyword error and in the extra_params, not just silently ignored. It could be a TypeError, similar to repeating a keyword arg, for example:\n\n```\nkw\u003d{\u0027reverse\u0027: True}\nl.sort(reverse\u003dTrue, **kw)\nTraceback (most recent call last):\n  File \"/Applications/PyCharm CE.app/Contents/plugins/python-ce/helpers/pydev/pydevconsole.py\", line 364, in runcode\n    coro \u003d func()\n  File \"\u003cinput\u003e\", line 1, in \u003cmodule\u003e\nTypeError: sort() got multiple values for keyword argument \u0027reverse\u0027\n```","commit_id":"d754e387bd2b45f2417a62c47f130d18d49d1b26"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"9ebd084ed03d3facb352170fe6ff68864094c816","unresolved":false,"context_lines":[{"line_number":336,"context_line":"        for each individual use case. For example, [\u0027states\u003dupdating\u0027] can be"},{"line_number":337,"context_line":"        used with shard_range/namespace listing. Note: common parameters passed"},{"line_number":338,"context_line":"        into ``extra_params`` will be ignored, and this interface will only"},{"line_number":339,"context_line":"        verify the basic format of extra parameters."},{"line_number":340,"context_line":"    :returns: a tuple of (response headers, a list of objects) The response"},{"line_number":341,"context_line":"              headers will be a HeaderKeyDict."},{"line_number":342,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"3721e3d9_f26966b9","line":339,"in_reply_to":"f6a2ff20_cfc492f5","updated":"2023-08-16 05:33:26.000000000","message":"Done","commit_id":"d754e387bd2b45f2417a62c47f130d18d49d1b26"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b417eac21bc31dd18777831801736150bb66203","unresolved":true,"context_lines":[{"line_number":158,"context_line":"                                  conn_timeout\u003d5, response_timeout\u003d15,"},{"line_number":159,"context_line":"                                  end_marker\u003dNone, reverse\u003dNone, headers\u003dNone,"},{"line_number":160,"context_line":"                                  extra_params\u003dNone):"},{"line_number":161,"context_line":"    \"\"\"Base class for get direct account and container."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    Do not use directly use the direct_get_account or"},{"line_number":164,"context_line":"    direct_get_container instead."}],"source_content_type":"text/x-python","patch_set":4,"id":"95a6def5_2bacdd49","line":161,"range":{"start_line":161,"start_character":12,"end_line":161,"end_character":17},"updated":"2023-08-16 11:22:55.000000000","message":"nit/off-topic: could you please correct this: s/class/function/ I guess","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ac2dcd5928845b28d1dfd2dbb6bea4ffce78fc87","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                                  conn_timeout\u003d5, response_timeout\u003d15,"},{"line_number":159,"context_line":"                                  end_marker\u003dNone, reverse\u003dNone, headers\u003dNone,"},{"line_number":160,"context_line":"                                  extra_params\u003dNone):"},{"line_number":161,"context_line":"    \"\"\"Base class for get direct account and container."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    Do not use directly use the direct_get_account or"},{"line_number":164,"context_line":"    direct_get_container instead."}],"source_content_type":"text/x-python","patch_set":4,"id":"7006c470_2681f7d8","line":161,"range":{"start_line":161,"start_character":12,"end_line":161,"end_character":17},"in_reply_to":"95a6def5_2bacdd49","updated":"2023-08-16 22:15:19.000000000","message":"Done","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b417eac21bc31dd18777831801736150bb66203","unresolved":true,"context_lines":[{"line_number":169,"context_line":"    valid_extra_params \u003d []"},{"line_number":170,"context_line":"    if extra_params:"},{"line_number":171,"context_line":"        for param, value in extra_params.items():"},{"line_number":172,"context_line":"            if not value:"},{"line_number":173,"context_line":"                raise ValueError(\u0027Empty value for param %s\u0027 % param)"},{"line_number":174,"context_line":"            if ("},{"line_number":175,"context_line":"                not value or param \u003d\u003d \u0027marker\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"01d231f4_f11bb284","line":172,"range":{"start_line":172,"start_character":15,"end_line":172,"end_character":24},"updated":"2023-08-16 11:22:55.000000000","message":"`if value is None` ?\n\nand extra param value might be an empty string or 0","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ac2dcd5928845b28d1dfd2dbb6bea4ffce78fc87","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    valid_extra_params \u003d []"},{"line_number":170,"context_line":"    if extra_params:"},{"line_number":171,"context_line":"        for param, value in extra_params.items():"},{"line_number":172,"context_line":"            if not value:"},{"line_number":173,"context_line":"                raise ValueError(\u0027Empty value for param %s\u0027 % param)"},{"line_number":174,"context_line":"            if ("},{"line_number":175,"context_line":"                not value or param \u003d\u003d \u0027marker\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"ed3fc9f6_8b39a3fe","line":172,"range":{"start_line":172,"start_character":15,"end_line":172,"end_character":24},"in_reply_to":"01d231f4_f11bb284","updated":"2023-08-16 22:15:19.000000000","message":"I wanted to follow the same style of the existing keyword params. https://review.opendev.org/c/openstack/swift/+/891387/4/swift/common/direct_client.py#b171\nbut on the second thought, for those extra params, we should leave add more flexibility.","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b417eac21bc31dd18777831801736150bb66203","unresolved":true,"context_lines":[{"line_number":172,"context_line":"            if not value:"},{"line_number":173,"context_line":"                raise ValueError(\u0027Empty value for param %s\u0027 % param)"},{"line_number":174,"context_line":"            if ("},{"line_number":175,"context_line":"                not value or param \u003d\u003d \u0027marker\u0027"},{"line_number":176,"context_line":"                or param \u003d\u003d \u0027limit\u0027 or param \u003d\u003d \u0027prefix\u0027"},{"line_number":177,"context_line":"                or param \u003d\u003d \u0027marker\u0027 or param \u003d\u003d \u0027delimiter\u0027"},{"line_number":178,"context_line":"                or param \u003d\u003d \u0027end_marker\u0027 or param \u003d\u003d \u0027reverse\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"e8e1f73d_0e3e10fa","line":175,"range":{"start_line":175,"start_character":16,"end_line":175,"end_character":25},"updated":"2023-08-16 11:22:55.000000000","message":"already tested this condition","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ac2dcd5928845b28d1dfd2dbb6bea4ffce78fc87","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            if not value:"},{"line_number":173,"context_line":"                raise ValueError(\u0027Empty value for param %s\u0027 % param)"},{"line_number":174,"context_line":"            if ("},{"line_number":175,"context_line":"                not value or param \u003d\u003d \u0027marker\u0027"},{"line_number":176,"context_line":"                or param \u003d\u003d \u0027limit\u0027 or param \u003d\u003d \u0027prefix\u0027"},{"line_number":177,"context_line":"                or param \u003d\u003d \u0027marker\u0027 or param \u003d\u003d \u0027delimiter\u0027"},{"line_number":178,"context_line":"                or param \u003d\u003d \u0027end_marker\u0027 or param \u003d\u003d \u0027reverse\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"6fe855bb_1b7938b9","line":175,"range":{"start_line":175,"start_character":16,"end_line":175,"end_character":25},"in_reply_to":"e8e1f73d_0e3e10fa","updated":"2023-08-16 22:15:19.000000000","message":"oh, my bad. didn\u0027t get it removed.","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b417eac21bc31dd18777831801736150bb66203","unresolved":true,"context_lines":[{"line_number":179,"context_line":"            ):"},{"line_number":180,"context_line":"                raise TypeError("},{"line_number":181,"context_line":"                    \u0027direct_get_container() got multiple values for keyword \u0027"},{"line_number":182,"context_line":"                    \u0027argument: %s\u0027 % param)"},{"line_number":183,"context_line":"            valid_extra_params.append(\u0027%s\u003d%s\u0027 % (param, value))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    params \u003d [\u0027format\u003djson\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"68837c1e_f88bc041","line":182,"updated":"2023-08-16 11:22:55.000000000","message":"OIC! I was hoping that extra_params could be used instead of the existing args, i.e. that this would be allowed:\n\n```\n_get_direct_account_container(path, stype, node, part, extra_params\u003d{\u0027marker\u0027: \u0027foo\u0027, \u0027states\u0027: \u0027listing\u0027}\n```\n\nso the caller could just pass in a dict of all the required params.\n\nAnd this would also be OK:\n\n```\n_get_direct_account_container(path, stype, node, part, marker\u003dNone, extra_params\u003d{\u0027marker\u0027: \u0027foo\u0027, \u0027states\u0027: \u0027listing\u0027}\n```\n\nBut this would be an error:\n\n```\n_get_direct_account_container(path, stype, node, part, marker\u003d\u0027foo\u0027, extra_params\u003d{\u0027marker\u0027: \u0027foo\u0027, \u0027states\u0027: \u0027listing\u0027}\n```","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"36105f1106538bc4b64abcf04a9fd2b88dce9ee7","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            ):"},{"line_number":180,"context_line":"                raise TypeError("},{"line_number":181,"context_line":"                    \u0027direct_get_container() got multiple values for keyword \u0027"},{"line_number":182,"context_line":"                    \u0027argument: %s\u0027 % param)"},{"line_number":183,"context_line":"            valid_extra_params.append(\u0027%s\u003d%s\u0027 % (param, value))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    params \u003d [\u0027format\u003djson\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"57071a15_c294236c","line":182,"in_reply_to":"68837c1e_f88bc041","updated":"2023-08-17 05:24:29.000000000","message":"okay, I made the changes to follow this suggestion.","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0b417eac21bc31dd18777831801736150bb66203","unresolved":true,"context_lines":[{"line_number":333,"context_line":"        prefix, delimiter, end_marker, reverse) are common parameters among"},{"line_number":334,"context_line":"        those usages, and ``extra_params`` is only used to pass private"},{"line_number":335,"context_line":"        parameters for each individual use case. For example,"},{"line_number":336,"context_line":"        [\u0027states\u003dupdating\u0027] can be used with shard_range/namespace listing."},{"line_number":337,"context_line":"    :returns: a tuple of (response headers, a list of objects) The response"},{"line_number":338,"context_line":"              headers will be a HeaderKeyDict."},{"line_number":339,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"7da983b6_0065b315","line":336,"updated":"2023-08-16 11:22:55.000000000","message":"I was assuming that extra_params would be an alternative to the keyword args, rather than purely additional. I can imagine wanting to maintain a dict of params and pass them all in together, not having to split out marker etc from other params","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ac2dcd5928845b28d1dfd2dbb6bea4ffce78fc87","unresolved":true,"context_lines":[{"line_number":333,"context_line":"        prefix, delimiter, end_marker, reverse) are common parameters among"},{"line_number":334,"context_line":"        those usages, and ``extra_params`` is only used to pass private"},{"line_number":335,"context_line":"        parameters for each individual use case. For example,"},{"line_number":336,"context_line":"        [\u0027states\u003dupdating\u0027] can be used with shard_range/namespace listing."},{"line_number":337,"context_line":"    :returns: a tuple of (response headers, a list of objects) The response"},{"line_number":338,"context_line":"              headers will be a HeaderKeyDict."},{"line_number":339,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"fea78f60_d2196817","line":336,"in_reply_to":"7da983b6_0065b315","updated":"2023-08-16 22:15:19.000000000","message":"To make the interface clearer, I only want \"extra_params\" to be purely additional parameters, since we have those keyword args already. See my new added comments \"extra_params: a dict of extra parameters to be included in the request. The usages for this function includes shard_range/namespace listing and object listing, while the named kwargs (marker, limit, prefix, delimiter, end_marker, reverse) are common parameters among those usages, and ``extra_params`` is only used to pass private parameters for each individual use case.\"","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"36105f1106538bc4b64abcf04a9fd2b88dce9ee7","unresolved":false,"context_lines":[{"line_number":333,"context_line":"        prefix, delimiter, end_marker, reverse) are common parameters among"},{"line_number":334,"context_line":"        those usages, and ``extra_params`` is only used to pass private"},{"line_number":335,"context_line":"        parameters for each individual use case. For example,"},{"line_number":336,"context_line":"        [\u0027states\u003dupdating\u0027] can be used with shard_range/namespace listing."},{"line_number":337,"context_line":"    :returns: a tuple of (response headers, a list of objects) The response"},{"line_number":338,"context_line":"              headers will be a HeaderKeyDict."},{"line_number":339,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"583eabb6_a88d4fc7","line":336,"in_reply_to":"fea78f60_d2196817","updated":"2023-08-17 05:24:29.000000000","message":"After discussion with Clay who is also on board with this suggestion, I made the changes to follow the idea from Alistair.","commit_id":"1f7ec243435c4fab5eb5eeb0382407b59000d1c8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":true,"context_lines":[{"line_number":170,"context_line":"    if extra_params:"},{"line_number":171,"context_line":"        for key, value in extra_params.items():"},{"line_number":172,"context_line":"            if value is None:"},{"line_number":173,"context_line":"                raise ValueError(\u0027Empty value for param %s\u0027 % key)"},{"line_number":174,"context_line":"            params[key] \u003d value"},{"line_number":175,"context_line":"    if marker:"},{"line_number":176,"context_line":"        if \u0027marker\u0027 in params:"}],"source_content_type":"text/x-python","patch_set":6,"id":"000f851c_288b29ab","line":173,"updated":"2023-08-17 13:47:55.000000000","message":"I\u0027m not sure this is necessary, or desirable - the param could just be ignored if None, just like a keyword arg param with value None is ignored.","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    if extra_params:"},{"line_number":171,"context_line":"        for key, value in extra_params.items():"},{"line_number":172,"context_line":"            if value is None:"},{"line_number":173,"context_line":"                raise ValueError(\u0027Empty value for param %s\u0027 % key)"},{"line_number":174,"context_line":"            params[key] \u003d value"},{"line_number":175,"context_line":"    if marker:"},{"line_number":176,"context_line":"        if \u0027marker\u0027 in params:"}],"source_content_type":"text/x-python","patch_set":6,"id":"13d40000_5ac60e7a","line":173,"in_reply_to":"000f851c_288b29ab","updated":"2023-08-17 18:58:26.000000000","message":"I felt extra_params is different from the keyword args, I was thinking why someone would add a parameter to extra_params with value None. But anymore, let\u0027s remove the exception, keep it as same as keyword args.","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":true,"context_lines":[{"line_number":174,"context_line":"            params[key] \u003d value"},{"line_number":175,"context_line":"    if marker:"},{"line_number":176,"context_line":"        if \u0027marker\u0027 in params:"},{"line_number":177,"context_line":"            raise TypeError(\u0027dupliate values for keyword arg: marker\u0027)"},{"line_number":178,"context_line":"        params[\u0027marker\u0027] \u003d quote(marker)"},{"line_number":179,"context_line":"    if limit:"},{"line_number":180,"context_line":"        if \u0027limit\u0027 in params:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1e1057c2_bcae965a","line":177,"range":{"start_line":177,"start_character":29,"end_line":177,"end_character":37},"updated":"2023-08-17 13:47:55.000000000","message":"typo: duplicate","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            params[key] \u003d value"},{"line_number":175,"context_line":"    if marker:"},{"line_number":176,"context_line":"        if \u0027marker\u0027 in params:"},{"line_number":177,"context_line":"            raise TypeError(\u0027dupliate values for keyword arg: marker\u0027)"},{"line_number":178,"context_line":"        params[\u0027marker\u0027] \u003d quote(marker)"},{"line_number":179,"context_line":"    if limit:"},{"line_number":180,"context_line":"        if \u0027limit\u0027 in params:"}],"source_content_type":"text/x-python","patch_set":6,"id":"6ecfa05d_76522b47","line":177,"range":{"start_line":177,"start_character":29,"end_line":177,"end_character":37},"in_reply_to":"1e1057c2_bcae965a","updated":"2023-08-17 18:58:26.000000000","message":"Ack","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":true,"context_lines":[{"line_number":329,"context_line":"    :param reverse: reverse the returned listing"},{"line_number":330,"context_line":"    :param headers: headers to be included in the request"},{"line_number":331,"context_line":"    :param extra_params: a dict of extra parameters to be included in the"},{"line_number":332,"context_line":"        request. It can be used to pass additioinal parameters, e.g,"},{"line_number":333,"context_line":"        {\u0027states\u0027:\u0027updating\u0027} can be used with shard_range/namespace listing."},{"line_number":334,"context_line":"        It can also be used to pass the existing keyword args, like \u0027marker\u0027 or"},{"line_number":335,"context_line":"        \u0027limit\u0027, but if the same parameter appears twice in both keyword arg"}],"source_content_type":"text/x-python","patch_set":6,"id":"fb498ac5_94c211e2","line":332,"range":{"start_line":332,"start_character":40,"end_line":332,"end_character":51},"updated":"2023-08-17 13:47:55.000000000","message":"typo: additional","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    :param reverse: reverse the returned listing"},{"line_number":330,"context_line":"    :param headers: headers to be included in the request"},{"line_number":331,"context_line":"    :param extra_params: a dict of extra parameters to be included in the"},{"line_number":332,"context_line":"        request. It can be used to pass additioinal parameters, e.g,"},{"line_number":333,"context_line":"        {\u0027states\u0027:\u0027updating\u0027} can be used with shard_range/namespace listing."},{"line_number":334,"context_line":"        It can also be used to pass the existing keyword args, like \u0027marker\u0027 or"},{"line_number":335,"context_line":"        \u0027limit\u0027, but if the same parameter appears twice in both keyword arg"}],"source_content_type":"text/x-python","patch_set":6,"id":"61a8349b_e5982a6f","line":332,"range":{"start_line":332,"start_character":40,"end_line":332,"end_character":51},"in_reply_to":"fb498ac5_94c211e2","updated":"2023-08-17 18:58:26.000000000","message":"Ack","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"}],"test/unit/common/test_direct_client.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":true,"context_lines":[{"line_number":446,"context_line":"                self.assertIn(\u0027format\u003djson\u0027, conn.query_string)"},{"line_number":447,"context_line":"                for k, v in req_params.items():"},{"line_number":448,"context_line":"                    if v is None:"},{"line_number":449,"context_line":"                        self.assertNotIn(\u0027\u0026%s\u0027 % k, conn.query_string)"},{"line_number":450,"context_line":"                    else:"},{"line_number":451,"context_line":"                        self.assertIn(\u0027\u0026%s\u003d%s\u0027 % (k, v), conn.query_string)"},{"line_number":452,"context_line":"            except AssertionError as err:"}],"source_content_type":"text/x-python","patch_set":6,"id":"c5d87b79_394a9fa3","line":449,"range":{"start_line":449,"start_character":42,"end_line":449,"end_character":45},"updated":"2023-08-17 13:47:55.000000000","message":"this fails under py27 because the order of the params in the query string is no longer deterministic (dict.items() is not ordered by insertion until py3), so \"format\u003djson\" may not be the first param, and some other param therefore does not have the leading \u0027\u0026\u0027\n\nThe test is too brittle - looks like I wrote it :( and it would be better to parse the query string and assert it has all expected key-value pairs, without asserting their order.","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[{"line_number":446,"context_line":"                self.assertIn(\u0027format\u003djson\u0027, conn.query_string)"},{"line_number":447,"context_line":"                for k, v in req_params.items():"},{"line_number":448,"context_line":"                    if v is None:"},{"line_number":449,"context_line":"                        self.assertNotIn(\u0027\u0026%s\u0027 % k, conn.query_string)"},{"line_number":450,"context_line":"                    else:"},{"line_number":451,"context_line":"                        self.assertIn(\u0027\u0026%s\u003d%s\u0027 % (k, v), conn.query_string)"},{"line_number":452,"context_line":"            except AssertionError as err:"}],"source_content_type":"text/x-python","patch_set":6,"id":"233ee24e_04273bc5","line":449,"range":{"start_line":449,"start_character":42,"end_line":449,"end_character":45},"in_reply_to":"c5d87b79_394a9fa3","updated":"2023-08-17 18:58:26.000000000","message":"yeah. actually dict becomes sorted until python version 3.6. I didn\u0027t have other python environment anymore, so I pushed up the changes and used the CI pipeline to test the changes. Thanks for fixing those failed test cases.","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":true,"context_lines":[{"line_number":500,"context_line":"                self.assertEqual(headers, resp_headers)"},{"line_number":501,"context_line":"                self.assertEqual(json.loads(body), resp)"},{"line_number":502,"context_line":"                for k, v in expected_params.items():"},{"line_number":503,"context_line":"                    self.assertIn(\u0027%s\u003d%s\u0027 % (k, v), conn.query_string)"},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"        req_params \u003d dict(marker\u003d\u0027my-marker\u0027, prefix\u003d\u0027my-prefix\u0027,"},{"line_number":506,"context_line":"                          delimiter\u003d\u0027my-delimiter\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f200980_38198209","line":503,"updated":"2023-08-17 13:47:55.000000000","message":"nit: this does not assert that there are no repeats","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[{"line_number":500,"context_line":"                self.assertEqual(headers, resp_headers)"},{"line_number":501,"context_line":"                self.assertEqual(json.loads(body), resp)"},{"line_number":502,"context_line":"                for k, v in expected_params.items():"},{"line_number":503,"context_line":"                    self.assertIn(\u0027%s\u003d%s\u0027 % (k, v), conn.query_string)"},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"        req_params \u003d dict(marker\u003d\u0027my-marker\u0027, prefix\u003d\u0027my-prefix\u0027,"},{"line_number":506,"context_line":"                          delimiter\u003d\u0027my-delimiter\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9960b5af_5be65d40","line":503,"in_reply_to":"1f200980_38198209","updated":"2023-08-17 18:58:26.000000000","message":"Ack","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":true,"context_lines":[{"line_number":518,"context_line":"                          limit\u003d10, end_marker\u003d\u0027my-endmarker\u0027, reverse\u003d\u0027on\u0027,"},{"line_number":519,"context_line":"                          extra_params\u003d{\u0027states\u0027: \u0027\u0027})"},{"line_number":520,"context_line":"        expected_err \u003d ValueError(\u0027Empty value for param states\u0027)"},{"line_number":521,"context_line":"        do_test(req_params, expected_params\u003dNone, expected_err\u003dexpected_err)"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"        req_params \u003d dict(marker\u003d\u0027my-marker\u0027, prefix\u003d\u0027my-prefix\u0027,"},{"line_number":524,"context_line":"                          delimiter\u003d\u0027my-delimiter\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"f3d19e08_dfa0ef89","line":521,"updated":"2023-08-17 13:47:55.000000000","message":"this should fail - {\u0027states\u0027: \u0027\u0027} is allowed - but the test does not check that an expected exception is raised\n\nI\u0027d suggest using self.assertRaises to test the error cases","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[{"line_number":518,"context_line":"                          limit\u003d10, end_marker\u003d\u0027my-endmarker\u0027, reverse\u003d\u0027on\u0027,"},{"line_number":519,"context_line":"                          extra_params\u003d{\u0027states\u0027: \u0027\u0027})"},{"line_number":520,"context_line":"        expected_err \u003d ValueError(\u0027Empty value for param states\u0027)"},{"line_number":521,"context_line":"        do_test(req_params, expected_params\u003dNone, expected_err\u003dexpected_err)"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"        req_params \u003d dict(marker\u003d\u0027my-marker\u0027, prefix\u003d\u0027my-prefix\u0027,"},{"line_number":524,"context_line":"                          delimiter\u003d\u0027my-delimiter\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"48857e71_7f64dbc4","line":521,"in_reply_to":"f3d19e08_dfa0ef89","updated":"2023-08-17 18:58:26.000000000","message":"In previous patch, I did check exception type and message. https://review.opendev.org/c/openstack/swift/+/891387/6/test/unit/common/test_direct_client.py#492\nBut yes, I like self.assertRaises better.","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"470a6314105babb70439f37d20681155b181fb11","unresolved":true,"context_lines":[{"line_number":559,"context_line":"        expected_params \u003d dict(marker\u003d\u0027my-marker\u0027, prefix\u003d\u0027others\u0027,"},{"line_number":560,"context_line":"                               delimiter\u003d\u0027my-delimiter\u0027, limit\u003d10,"},{"line_number":561,"context_line":"                               end_marker\u003d\u0027my-endmarker\u0027, reverse\u003d\u0027on\u0027)"},{"line_number":562,"context_line":"        do_test(req_params, expected_params)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"    def test_direct_delete_container(self):"},{"line_number":565,"context_line":"        with mocked_http_conn(200) as conn:"}],"source_content_type":"text/x-python","patch_set":6,"id":"d4f23ed7_6b561071","line":562,"updated":"2023-08-17 13:47:55.000000000","message":"it would be good to have a case when extra_params has a non-string value e.g. 0","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"b1fd8f774daded8bb5ceb5117ea2a6612e7813c1","unresolved":false,"context_lines":[{"line_number":559,"context_line":"        expected_params \u003d dict(marker\u003d\u0027my-marker\u0027, prefix\u003d\u0027others\u0027,"},{"line_number":560,"context_line":"                               delimiter\u003d\u0027my-delimiter\u0027, limit\u003d10,"},{"line_number":561,"context_line":"                               end_marker\u003d\u0027my-endmarker\u0027, reverse\u003d\u0027on\u0027)"},{"line_number":562,"context_line":"        do_test(req_params, expected_params)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"    def test_direct_delete_container(self):"},{"line_number":565,"context_line":"        with mocked_http_conn(200) as conn:"}],"source_content_type":"text/x-python","patch_set":6,"id":"545e9aa2_7f83d7dd","line":562,"in_reply_to":"d4f23ed7_6b561071","updated":"2023-08-17 18:58:26.000000000","message":"Ack","commit_id":"6170d96395da24e52a26f3e44d9c3b11bbd76506"}]}