)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f8f6f1bca44939392ad8811b8e857175083adc90","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2023-12-21 21:54:37 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add txn in error output"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Ice9cc9fe68684563f18ee527996e5a4292230a96"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"c8662c86_8acd309e","line":8,"updated":"2024-01-10 15:44:38.000000000","message":"this would be worth writing up as a launchpad bug (if it\u0027s not already) https://bugs.launchpad.net/python-swiftclient\n\nthen reference in the commit message using a tag:\n\nCloses-Bug: #bug-number","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"e2ecd049601aeb6b14d7721bd93a943432c1e0c2","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2023-12-21 21:54:37 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add txn in error output"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Ice9cc9fe68684563f18ee527996e5a4292230a96"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"d75ff2f1_981b2342","line":8,"in_reply_to":"c8662c86_8acd309e","updated":"2024-01-10 21:36:32.000000000","message":"Acknowledged","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Yan Xiao \u003cyanxiao@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-01-18 21:54:21 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5935a4e2_1fcbe402","line":7,"updated":"2024-01-20 00:18:10.000000000","message":"We want a blank line between the subject line and the rest of the commit message -- as it is, gerrit seems to think you\u0027ve got one really, _really_ long subject 😄","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Yan Xiao \u003cyanxiao@nvidia.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-01-18 21:54:21 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb5a958_c08b583c","line":7,"in_reply_to":"5935a4e2_1fcbe402","updated":"2024-01-22 20:36:27.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-01-18 21:54:21 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"77066c04_981c78eb","line":8,"range":{"start_line":8,"start_character":48,"end_line":8,"end_character":63},"updated":"2024-01-20 00:18:10.000000000","message":"😕\n```\nvagrant@saio:~/python-swiftclient$ swift list test_segments\ntox.ini/slo/1705701295.575089/2935/1024/00000000\ntox.ini/slo/1705701295.575089/2935/1024/00000001\ntox.ini/slo/1705701295.575089/2935/1024/00000002\nvagrant@saio:~/python-swiftclient$ swift delete test_segments tox.ini/slo/1705701295.575089/2935/1024/00000001\ntox.ini/slo/1705701295.575089/2935/1024/00000001\nvagrant@saio:~/python-swiftclient$ swift download --no-download test tox.ini --retries 2\nError downloading object \u0027test/tox.ini\u0027: Object GET failed: http://saio:8080/v1/AUTH_test/test/tox.ini 500 Internal Server Error  [first 60 chars of response] b\u0027Traceback (most recent call last):\\n  File \"/usr/local/lib/py\u0027\n```\nStill no txn id?\n\nAh -- looks like the server really never sent it:\n```\nINFO:swiftclient:REQ: curl -i http://saio:8080/v1/AUTH_test/test/tox.ini -X GET -H \"X-Auth-Token: AUTH_tk1c2c08b388e1411e9afa1c8028a2b629\"\nINFO:swiftclient:RESP STATUS: 500 Internal Server Error\nINFO:swiftclient:RESP HEADERS: {\u0027Content-type\u0027: \u0027text/plain\u0027, \u0027Content-length\u0027: \u0027757\u0027, \u0027Date\u0027: \u0027Fri, 19 Jan 2024 22:34:17 GMT\u0027, \u0027Connection\u0027: \u0027close\u0027}\nINFO:swiftclient:RESP BODY: b\u0027Traceback (most recent call last):\\n  File \"/usr/local/lib/python3.8/dist-packages/eventlet/wsgi.py\", line 589, in handle_one_response\\n    for data in result:\\n  File \"/vagrant/swift/swift/common/middleware/catch_errors.py\", line 39, in enforce_byte_count\\n    for chunk in inner_iter:\\n  File \"/vagrant/swift/swift/common/utils/__init__.py\", line 3704, in __next__\\n    return next(self.chained_iter)\\n  File \"/vagrant/swift/swift/common/middleware/proxy_logging.py\", line 444, in iter_response\\n    for chunk in iterator:\\n  File \"/vagrant/swift/swift/common/middleware/catch_errors.py\", line 51, in enforce_byte_count\\n    raise BadResponseLength(\\\u0027Expected another %d bytes\\\u0027 % (\\nswift.common.middleware.catch_errors.BadResponseLength: Expected another 1911 bytes\\n\u0027\n```\nI guess https://review.opendev.org/c/openstack/swift/+/887904 didn\u0027t go far enough...\n\nOh! But I didn\u0027t use large enough segments (I guess... apparently we never tweak the HttpProtocol\u0027s `minimum_chunk_size`...)\n\n---\n\nOK, now I\u0027m in business -- naive curl gives me a truncated response:\n```\n$ curl -i http://saio:8080/v1/AUTH_test/test/tox.ini -X GET -H \"X-Auth-Token: AUTH_tk1c2c08b388e1411e9afa1c8028a2b629\"\nHTTP/1.1 200 OK\nContent-Type: application/octet-stream\nX-Object-Meta-Mtime: 1705701295.575089\nX-Static-Large-Object: True\nEtag: \"5af4d74b3bb98258d3c600a41dd52bf5\"\nLast-Modified: Fri, 19 Jan 2024 22:32:18 GMT\nX-Timestamp: 1705703537.92039\nAccept-Ranges: bytes\nContent-Length: 2935\nX-Manifest-Etag: 256a179accf26b46f13dfd2042ef2802\nX-Trans-Id: tx15e17ab6ca694556a6eb1-0065aafec0\nX-Openstack-Request-Id: tx15e17ab6ca694556a6eb1-0065aafec0\nDate: Fri, 19 Jan 2024 22:59:12 GMT\n\n[tox]\nenvlist \u003d py3,pep8\n\u003csnip\u003e\nallowlist_externals \u003d\n    coverage\ncurl: (18) transfer closed with 1911 bytes remaining to read\n```\nAnd swiftclient gives me...\n```\nError downloading object \u0027test/tox.ini\u0027: (\u0027Connection broken: IncompleteRead(1024 bytes read, 1911 more expected)\u0027, IncompleteRead(1024 bytes read, 1911 more expected)) (txn: txfc0b67b08c124e9bb9d1c-0065ab0313)\n```\nGood! Transaction ID is there! And it\u0027s not on the parent commit. But the `IncompleteRead` error is odd -- I was expecting the client to retry, then get back a 409... Looks like that\u0027s because we only catch `socket.error` and `RequestException` as we\u0027re reading the body...","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"f3560307757f95ac240eb976d98a6f8b67af5dc3","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-01-18 21:54:21 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"a1644c6a_3c40ac69","line":8,"range":{"start_line":8,"start_character":48,"end_line":8,"end_character":63},"in_reply_to":"51844911_a3a2b8fc","updated":"2024-01-23 21:16:29.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-01-18 21:54:21 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"51844911_a3a2b8fc","line":8,"range":{"start_line":8,"start_character":48,"end_line":8,"end_character":63},"in_reply_to":"77066c04_981c78eb","updated":"2024-01-22 20:36:27.000000000","message":"Right urllib3.exceptions.ProtocolError returns e.g. \u0027Connection broken: IncompleteRead ...\u0027 for this","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change response headers returned by client.py Connection APIs"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"8241c4e2_a89cb336","line":9,"range":{"start_line":9,"start_character":39,"end_line":9,"end_character":58},"updated":"2024-01-20 00:18:10.000000000","message":"Huh. I would\u0027ve expected that to get handled where we\u0027re already special-casing 404s: https://github.com/openstack/python-swiftclient/blob/master/swiftclient/shell.py#L482-L484\n\nThough I guess the HEAD case doesn\u0027t have that...","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"2043ee6c9467d4487405edbbfe062f5ea0499fef","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change response headers returned by client.py Connection APIs"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"6ddd016d_03f79741","line":9,"range":{"start_line":9,"start_character":39,"end_line":9,"end_character":58},"in_reply_to":"292ad655_1b5ba646","updated":"2024-01-22 21:35:22.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add transaction id to errors to help troubleshoot, including:"},{"line_number":8,"context_line":"  - error when downloading object with truncted/missing segment"},{"line_number":9,"context_line":"  - error when downloading or stat\u0027ing non-existent object"},{"line_number":10,"context_line":"  - error when stat\u0027ing or listing non-existent container"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change response headers returned by client.py Connection APIs"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"292ad655_1b5ba646","line":9,"range":{"start_line":9,"start_character":39,"end_line":9,"end_character":58},"in_reply_to":"8241c4e2_a89cb336","updated":"2024-01-22 20:36:27.000000000","message":"Actually you are correct, stat\u0027ing non-existent object already has transaction id; downloading non-existent object doesn\u0027t have because of shell.py formats for these errors. Updating this.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"775f09f6_415e46cc","updated":"2023-12-15 16:47:52.000000000","message":"nice!  looks good - but needs testing.  Every change should have some sort of justification via tests; if we can revert any part of your diff and no tests fail then we need to write more tests!\n\nLuckily I think you already have some tests to get you started:\n\n```\nvagrant@saio:~$ pytest python-swiftclient/test/unit/test_shell.py::TestShell\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d test session starts \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nplatform linux -- Python 3.10.12, pytest-7.4.3, pluggy-1.3.0\nrootdir: /home/vagrant/python-swiftclient\nplugins: requests-mock-1.11.0, cov-4.1.0\ncollected 85 items                                                                                                                                                                           \n\npython-swiftclient/test/unit/test_shell.py ................................................F....................................                                                       [100%]\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d FAILURES \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n___________________________________________________________________________ TestShell.test_post_account_not_found ____________________________________________________________________________\n\nself \u003d \u003ctest.unit.test_shell.TestShell testMethod\u003dtest_post_account_not_found\u003e, connection \u003d \u003cMagicMock name\u003d\u0027Connection\u0027 id\u003d\u0027139892997585360\u0027\u003e\n\n    @mock.patch(\u0027swiftclient.service.Connection\u0027)\n    def test_post_account_not_found(self, connection):\n        argv \u003d [\"\", \"post\"]\n        connection.return_value.post_account.side_effect \u003d \\\n            swiftclient.ClientException(\u0027test\u0027, http_status\u003d404)\n    \n        with CaptureOutput() as output:\n            with self.assertRaises(SystemExit):\n                swiftclient.shell.main(argv)\n    \n\u003e           self.assertEqual(output.err, \u0027Account not found\\n\u0027)\nE           AssertionError: \u0027Account not found (None)\\n\u0027 !\u003d \u0027Account not found\\n\u0027\nE           - Account not found (None)\nE           ?                  -------\nE           + Account not found\n\npython-swiftclient/test/unit/test_shell.py:1904: AssertionError\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d short test summary info \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nFAILED python-swiftclient/test/unit/test_shell.py::TestShell::test_post_account_not_found - AssertionError: \u0027Account not found (None)\\n\u0027 !\u003d \u0027Account not found\\n\u0027\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d 1 failed, 84 passed in 2.27s \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n```\n\nThanks for working on this!","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"488c644891e768fd6ea30fcea160325fe6ae0cd3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f8c56d70_61a4fb62","updated":"2024-01-22 11:26:44.000000000","message":"@Yan like Tim I am re-familiarising myself with swiftclient so this review may be a bit of a journey of learning together!\n\nI like the suggestion to move towards a single place of formatting error output in the shell.\n\nThis is your first swiftclient contribution (thanks!), so please note that -1 votes are *normal* in reviews. They mean \"This patch needs further work before it can be merged\" and should not be interpreted as a negative judgement on your work.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f8489019_f4fa7e42","updated":"2024-01-20 00:18:10.000000000","message":"Still orienting myself (it\u0027s been a bit since I poked at swiftclient much), and realizing it doesn\u0027t handle short reads _at all_ like how I expected -- which is a little weird, since I could have sworn I\u0027d tested that sort of thing years ago...\n\nOh! I bet I had the server send itself a `SIGTERM` or something before, whereas the SLO case closes out the socket properly.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b530b478d4bef9c6b7ec571e1c80812625d03317","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b8bbecaa_938226c6","updated":"2024-01-31 15:12:16.000000000","message":"This is starting to look good. The change to ClientException may be in response to my own suggestion but I\u0027m not convinced about the way it worked out.\n\nI pushed some follow ups that may be useful for squashing or ideas.","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cb3bc7591a9cb9e772711cae3d9e8fa511918ffa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7b829186_251b234c","updated":"2024-02-16 10:36:56.000000000","message":"@Yan thanks! I think the test_download_clientexception test (or versions of that test) needs a little more work to expand coverage.\n\nI think I now see where the CaseInsensitiveDict change is significant. At least, I see how not having it breaks tests.","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"72e0ed600b5fc9c914b75738670449142e3a450c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c91b3631_18cbba7a","updated":"2024-02-28 16:34:57.000000000","message":"LGTM\n\nI\u0027ll defer merging this for @Tim to have another look if he wants","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4be2470baa5d7b48833d3bad12a96c953fcfcb39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"005a13d1_4716c967","updated":"2024-02-28 16:36:22.000000000","message":"recheck","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bfed03ee3ee6f0cebdb43066ebf0839e240b5d7b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d21b4ad4_80b20859","updated":"2024-03-01 11:07:58.000000000","message":"Patch has gone backwards w.r.t. patchset 6 😂 But I\u0027m happy with the places where it went forwards!","commit_id":"c06b3985bf5ff3c690a3586e03d488bde9e3288c"}],"swiftclient/client.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":738,"context_line":"def resp_header_dict(resp):"},{"line_number":739,"context_line":"    resp_headers \u003d CaseInsensitiveDict()"},{"line_number":740,"context_line":"    for header, value in resp.getheaders():"},{"line_number":741,"context_line":"        header \u003d parse_header_string(header).lower()"},{"line_number":742,"context_line":"        resp_headers[header] \u003d parse_header_string(value)"},{"line_number":743,"context_line":"    return resp_headers"},{"line_number":744,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"507c9dd0_c3915ff1","line":741,"range":{"start_line":741,"start_character":44,"end_line":741,"end_character":52},"updated":"2024-01-20 00:18:10.000000000","message":"nit: Shouldn\u0027t need to use `lower()` any more.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"9421059c6ffec409c1061d9482358ee52485a47d","unresolved":false,"context_lines":[{"line_number":738,"context_line":"def resp_header_dict(resp):"},{"line_number":739,"context_line":"    resp_headers \u003d CaseInsensitiveDict()"},{"line_number":740,"context_line":"    for header, value in resp.getheaders():"},{"line_number":741,"context_line":"        header \u003d parse_header_string(header).lower()"},{"line_number":742,"context_line":"        resp_headers[header] \u003d parse_header_string(value)"},{"line_number":743,"context_line":"    return resp_headers"},{"line_number":744,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"36688f82_e30d068e","line":741,"range":{"start_line":741,"start_character":44,"end_line":741,"end_character":52},"in_reply_to":"507c9dd0_c3915ff1","updated":"2024-01-22 16:35:19.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b530b478d4bef9c6b7ec571e1c80812625d03317","unresolved":true,"context_lines":[{"line_number":736,"context_line":""},{"line_number":737,"context_line":""},{"line_number":738,"context_line":"def resp_header_dict(resp):"},{"line_number":739,"context_line":"    resp_headers \u003d CaseInsensitiveDict()"},{"line_number":740,"context_line":"    for header, value in resp.getheaders():"},{"line_number":741,"context_line":"        header \u003d parse_header_string(header)"},{"line_number":742,"context_line":"        resp_headers[header] \u003d parse_header_string(value)"}],"source_content_type":"text/x-python","patch_set":4,"id":"45c438a9_e965c7c1","line":739,"updated":"2024-01-31 15:12:16.000000000","message":"the only test that fails if I revert this change is test_response_headers which is asserting that the dict can be accessed with title-case keys.\n\nIs there anywhere in the code that relies on this change?","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"8ae0c26d16a2a96ba45803d4a50485dbe1b07199","unresolved":false,"context_lines":[{"line_number":736,"context_line":""},{"line_number":737,"context_line":""},{"line_number":738,"context_line":"def resp_header_dict(resp):"},{"line_number":739,"context_line":"    resp_headers \u003d CaseInsensitiveDict()"},{"line_number":740,"context_line":"    for header, value in resp.getheaders():"},{"line_number":741,"context_line":"        header \u003d parse_header_string(header)"},{"line_number":742,"context_line":"        resp_headers[header] \u003d parse_header_string(value)"}],"source_content_type":"text/x-python","patch_set":4,"id":"95a54111_49020724","line":739,"in_reply_to":"45c438a9_e965c7c1","updated":"2024-02-15 20:22:32.000000000","message":"The test in test_response_headers was added for this change. The actual functionality of adding transaction id in python-swiftclient errors relies on this change, although test_download_clientexception couldn\u0027t capture this change as the mock in unit tests was at higher layer such as Connection or connection _retry.","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cb3bc7591a9cb9e772711cae3d9e8fa511918ffa","unresolved":true,"context_lines":[{"line_number":290,"context_line":"        try:"},{"line_number":291,"context_line":"            buf \u003d self.resp.read(length)"},{"line_number":292,"context_line":"            self.bytes_read +\u003d len(buf)"},{"line_number":293,"context_line":"        except (socket.error, urllib_http_error, RequestException):"},{"line_number":294,"context_line":"            if self.conn.attempts \u003e self.conn.retries:"},{"line_number":295,"context_line":"                raise"},{"line_number":296,"context_line":"        if (not buf and self.bytes_read \u003c self.expected_length and"}],"source_content_type":"text/x-python","patch_set":5,"id":"7e815312_73893b5c","line":293,"updated":"2024-02-16 10:36:56.000000000","message":"This feels like a different bugfix vs missing transaction ids? It may belong in this patch, but it\u0027s not mentioned in the commit message. Is it worthy of a separate commit?\n\nAlso, I don\u0027t think this change is covered by unit tests - we need to parameterise test_download_clientexception for each exception type","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"72e0ed600b5fc9c914b75738670449142e3a450c","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        try:"},{"line_number":291,"context_line":"            buf \u003d self.resp.read(length)"},{"line_number":292,"context_line":"            self.bytes_read +\u003d len(buf)"},{"line_number":293,"context_line":"        except (socket.error, urllib_http_error, RequestException):"},{"line_number":294,"context_line":"            if self.conn.attempts \u003e self.conn.retries:"},{"line_number":295,"context_line":"                raise"},{"line_number":296,"context_line":"        if (not buf and self.bytes_read \u003c self.expected_length and"}],"source_content_type":"text/x-python","patch_set":5,"id":"e0558ba2_bb16980e","line":293,"in_reply_to":"7e815312_73893b5c","updated":"2024-02-28 16:34:57.000000000","message":"```\ndiff --git a/test/unit/test_shell.py b/test/unit/test_shell.py\nindex 384b967..6e18d70 100644\n--- a/test/unit/test_shell.py\n+++ b/test/unit/test_shell.py\n@@ -15,6 +15,7 @@\n\n import io\n import contextlib\n+import socket\n from genericpath import getmtime\n import getpass\n import hashlib\n@@ -28,6 +29,7 @@ from unittest import mock\n import textwrap\n from time import localtime, mktime, strftime, strptime\n from requests.exceptions import RequestException\n+from urllib3.exceptions import HTTPError\n\n import swiftclient\n from swiftclient.service import SwiftError\n@@ -752,9 +754,9 @@ class TestShell(unittest.TestCase):\n             swiftclient.shell.main(argv)\n             self.assertEqual(\u0027objcontent\u0027, output.out)\n\n-    def test_download_clientexception(self):\n+    def _do_test_download_clientexception(self, exc):\n         def fakeIter(arg):\n-            raise RequestException(\u0027test_exc\u0027)\n+            raise exc\n\n         retry_calls \u003d []\n\n@@ -797,6 +799,15 @@ class TestShell(unittest.TestCase):\n                           str(output.err))\n             self.assertIn(\u0027someTransId\u0027, str(output.err))\n\n+    def test_download_request_exception(self):\n+        self._do_test_download_clientexception(RequestException(\u0027text_exc\u0027))\n+\n+    def test_download_socket_error(self):\n+        self._do_test_download_clientexception(socket.error())\n+\n+    def test_download_socket_error(self):\n+        self._do_test_download_clientexception(HTTPError)\n+\n     @mock.patch(\u0027swiftclient.service.Connection\u0027)\n     def test_download_bad_content_length(self, connection):\n         objcontent \u003d io.BytesIO(b\u0027objcontent\u0027)\n~\n~\n```","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"1ba736a765e881a986c14193e29e7e4c33ec1941","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        try:"},{"line_number":291,"context_line":"            buf \u003d self.resp.read(length)"},{"line_number":292,"context_line":"            self.bytes_read +\u003d len(buf)"},{"line_number":293,"context_line":"        except (socket.error, urllib_http_error, RequestException):"},{"line_number":294,"context_line":"            if self.conn.attempts \u003e self.conn.retries:"},{"line_number":295,"context_line":"                raise"},{"line_number":296,"context_line":"        if (not buf and self.bytes_read \u003c self.expected_length and"}],"source_content_type":"text/x-python","patch_set":5,"id":"0920c1e5_38ff599a","line":293,"in_reply_to":"e0558ba2_bb16980e","updated":"2024-02-29 20:39:48.000000000","message":"Thanks @Alistair! the updates look really good","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"}],"swiftclient/exceptions.py":[{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"6f25e621297ae2a91d05ea014fb3cd2bc6a63b6c","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        self.transaction_id \u003d None"},{"line_number":39,"context_line":"        if self.http_response_headers:"},{"line_number":40,"context_line":"            for header in (\u0027X-Trans-Id\u0027, \u0027X-Openstack-Request-Id\u0027, \u0027x-trans-id\u0027):"},{"line_number":41,"context_line":"                if header in self.http_response_headers:"},{"line_number":42,"context_line":"                    self.transaction_id \u003d self.http_response_headers[header]"},{"line_number":43,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":1,"id":"94849baf_ef046193","line":40,"in_reply_to":"0e293251_ad281448","updated":"2023-12-22 14:55:45.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5715cbc7f49618bf157a5ef219eeb8e9c08c8682","unresolved":true,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        self.transaction_id \u003d None"},{"line_number":39,"context_line":"        if self.http_response_headers:"},{"line_number":40,"context_line":"            for header in (\u0027X-Trans-Id\u0027, \u0027X-Openstack-Request-Id\u0027, \u0027x-trans-id\u0027):"},{"line_number":41,"context_line":"                if header in self.http_response_headers:"},{"line_number":42,"context_line":"                    self.transaction_id \u003d self.http_response_headers[header]"},{"line_number":43,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":1,"id":"0e293251_ad281448","line":40,"in_reply_to":"a37ee286_81f39100","updated":"2023-12-20 21:02:43.000000000","message":"you can run a full pep8 check inside a SAIO  ``vtox -e pep8``\n\nor sometimes quicker: ``flake8 \u003cpath to files\u003e``\n\nhttps://docs.openstack.org/swift/latest/development_guidelines.html#testing-guidelines\n\n(note use vtox rather than tox inside a vSAIO)","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":38,"context_line":"        self.transaction_id \u003d None"},{"line_number":39,"context_line":"        if self.http_response_headers:"},{"line_number":40,"context_line":"            for header in (\u0027X-Trans-Id\u0027, \u0027X-Openstack-Request-Id\u0027, \u0027x-trans-id\u0027):"},{"line_number":41,"context_line":"                if header in self.http_response_headers:"},{"line_number":42,"context_line":"                    self.transaction_id \u003d self.http_response_headers[header]"},{"line_number":43,"context_line":"                    break"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b22997f9_ef68be4f","line":41,"updated":"2023-12-15 16:47:52.000000000","message":"meh, i might rewrod this as\n\n    for header, value in resp_headers.items():\n        if header.lower() in (\u0027x-trans-id\u0027, \u0027x-openstack-request-id\u0027):\n            self.trans_id \u003d value","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        self.transaction_id \u003d None"},{"line_number":39,"context_line":"        if self.http_response_headers:"},{"line_number":40,"context_line":"            for header in (\u0027X-Trans-Id\u0027, \u0027X-Openstack-Request-Id\u0027, \u0027x-trans-id\u0027):"},{"line_number":41,"context_line":"                if header in self.http_response_headers:"},{"line_number":42,"context_line":"                    self.transaction_id \u003d self.http_response_headers[header]"},{"line_number":43,"context_line":"                    break"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"c595fb98_c60b39f8","line":41,"in_reply_to":"b22997f9_ef68be4f","updated":"2023-12-15 20:58:21.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f8f6f1bca44939392ad8811b8e857175083adc90","unresolved":true,"context_lines":[{"line_number":34,"context_line":"        self.http_reason \u003d http_reason"},{"line_number":35,"context_line":"        self.http_device \u003d http_device"},{"line_number":36,"context_line":"        self.http_response_content \u003d http_response_content"},{"line_number":37,"context_line":"        self.http_response_headers \u003d None"},{"line_number":38,"context_line":"        self.http_response_headers \u003d http_response_headers"},{"line_number":39,"context_line":"        if isinstance(http_response_headers, Mapping):"},{"line_number":40,"context_line":"            self.http_response_headers \u003d CaseInsensitiveDict(http_response_headers)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b1070b66_73f44190","line":37,"updated":"2024-01-10 15:44:38.000000000","message":"this line isn\u0027t necessary","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"e2ecd049601aeb6b14d7721bd93a943432c1e0c2","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        self.http_reason \u003d http_reason"},{"line_number":35,"context_line":"        self.http_device \u003d http_device"},{"line_number":36,"context_line":"        self.http_response_content \u003d http_response_content"},{"line_number":37,"context_line":"        self.http_response_headers \u003d None"},{"line_number":38,"context_line":"        self.http_response_headers \u003d http_response_headers"},{"line_number":39,"context_line":"        if isinstance(http_response_headers, Mapping):"},{"line_number":40,"context_line":"            self.http_response_headers \u003d CaseInsensitiveDict(http_response_headers)"}],"source_content_type":"text/x-python","patch_set":2,"id":"15a1df34_55183288","line":37,"in_reply_to":"b1070b66_73f44190","updated":"2024-01-10 21:36:32.000000000","message":"Acknowledged","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f8f6f1bca44939392ad8811b8e857175083adc90","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        self.transaction_id \u003d None"},{"line_number":43,"context_line":"        if self.http_response_headers:"},{"line_number":44,"context_line":"            for header in (\u0027x-trans-id\u0027, \u0027x-openstack-request-id\u0027):"},{"line_number":45,"context_line":"                if header in self.http_response_headers:"},{"line_number":46,"context_line":"                    self.transaction_id \u003d self.http_response_headers[header]"},{"line_number":47,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":2,"id":"11ccd6bf_2c7a6068","line":44,"updated":"2024-01-10 15:44:38.000000000","message":"now it\u0027s case-insensitive, this change isn\u0027t strictly necessary? Was the case insensitive dict required - is there a test case that demonstrates that?","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"e2ecd049601aeb6b14d7721bd93a943432c1e0c2","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        self.transaction_id \u003d None"},{"line_number":43,"context_line":"        if self.http_response_headers:"},{"line_number":44,"context_line":"            for header in (\u0027x-trans-id\u0027, \u0027x-openstack-request-id\u0027):"},{"line_number":45,"context_line":"                if header in self.http_response_headers:"},{"line_number":46,"context_line":"                    self.transaction_id \u003d self.http_response_headers[header]"},{"line_number":47,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":2,"id":"f1000d2c_8c927602","line":44,"in_reply_to":"11ccd6bf_2c7a6068","updated":"2024-01-10 21:36:32.000000000","message":"Correct not strictly necessary. Although probably less confusing if it\u0027s case-insensitive. The case insensitive dict would be required as some headers seem to be lowercase. Good point on adding a test case for that.","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"f3560307757f95ac240eb976d98a6f8b67af5dc3","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        self.transaction_id \u003d None"},{"line_number":43,"context_line":"        if self.http_response_headers:"},{"line_number":44,"context_line":"            for header in (\u0027x-trans-id\u0027, \u0027x-openstack-request-id\u0027):"},{"line_number":45,"context_line":"                if header in self.http_response_headers:"},{"line_number":46,"context_line":"                    self.transaction_id \u003d self.http_response_headers[header]"},{"line_number":47,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":2,"id":"d1929cc7_b3574ee0","line":44,"in_reply_to":"f1000d2c_8c927602","updated":"2024-01-23 21:16:29.000000000","message":"Acknowledged","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b530b478d4bef9c6b7ec571e1c80812625d03317","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                    % self.http_response_content[:60]"},{"line_number":88,"context_line":"        c \u003d \u0027\u0027"},{"line_number":89,"context_line":"        if self.transaction_id:"},{"line_number":90,"context_line":"            c \u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.transaction_id"},{"line_number":91,"context_line":"        return b and \u0027%s: %s%s\u0027 % (a, b, c) or (a + c)"}],"source_content_type":"text/x-python","patch_set":4,"id":"84c0c654_4af82c23","line":90,"updated":"2024-01-31 15:12:16.000000000","message":"I think this may be my fault because of a previous comment about anomalous logging of txn_id 😞\n\nI\u0027m not keen on having a newline in the class.__str__ - I didn\u0027t realise the (txn: %s) was already part of the __str__, I had assumed it was being added by a logger format statement. Maybe we just need to accept the existing anomalous format, or accept the duplication if we later add \u0027\\nFailed Transaction ID: %s\u0027 when printing the error.","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"8ae0c26d16a2a96ba45803d4a50485dbe1b07199","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                    % self.http_response_content[:60]"},{"line_number":88,"context_line":"        c \u003d \u0027\u0027"},{"line_number":89,"context_line":"        if self.transaction_id:"},{"line_number":90,"context_line":"            c \u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.transaction_id"},{"line_number":91,"context_line":"        return b and \u0027%s: %s%s\u0027 % (a, b, c) or (a + c)"}],"source_content_type":"text/x-python","patch_set":4,"id":"6d5a990e_b0be5267","line":90,"in_reply_to":"84c0c654_4af82c23","updated":"2024-02-15 20:22:32.000000000","message":"Acknowledged","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"}],"swiftclient/service.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":30,"context_line":"from random import shuffle"},{"line_number":31,"context_line":"from time import time"},{"line_number":32,"context_line":"from threading import Thread"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from queue import Queue"},{"line_number":35,"context_line":"from queue import Empty as QueueEmpty"},{"line_number":36,"context_line":"from urllib.parse import quote"}],"source_content_type":"text/x-python","patch_set":1,"id":"db8857ad_daa7c4a1","line":33,"updated":"2023-12-15 16:47:52.000000000","message":"thse are all stdlib modules; I\u0027d prefer to keep them together","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from random import shuffle"},{"line_number":31,"context_line":"from time import time"},{"line_number":32,"context_line":"from threading import Thread"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from queue import Queue"},{"line_number":35,"context_line":"from queue import Empty as QueueEmpty"},{"line_number":36,"context_line":"from urllib.parse import quote"}],"source_content_type":"text/x-python","patch_set":1,"id":"0d2148a4_1ffe0577","line":33,"in_reply_to":"db8857ad_daa7c4a1","updated":"2023-12-15 20:58:21.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":76,"context_line":"        self.segment \u003d segment"},{"line_number":77,"context_line":"        self.exception \u003d exc"},{"line_number":78,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":79,"context_line":"            self.value +\u003d \u0027 (%s)\u0027 % self.exception.transaction_id"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __str__(self):"},{"line_number":82,"context_line":"        value \u003d repr(self.value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"23c9ddbb_4812e1fe","line":79,"updated":"2023-12-15 16:47:52.000000000","message":"i think this seems ok;\n\n    +\u003d \u0027\\nFailed Transaction Id: %s\u0027 % tnx_id\n    \n... might be another good option.  I don\u0027t think I have a strong opinon, it\u0027s all human readable formatting.","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        self.segment \u003d segment"},{"line_number":77,"context_line":"        self.exception \u003d exc"},{"line_number":78,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":79,"context_line":"            self.value +\u003d \u0027 (%s)\u0027 % self.exception.transaction_id"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __str__(self):"},{"line_number":82,"context_line":"        value \u003d repr(self.value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b490e2b3_1a2c8164","line":79,"in_reply_to":"23c9ddbb_4812e1fe","updated":"2023-12-15 20:58:21.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":1180,"context_line":"            for o_down in interruptable_as_completed(o_downs):"},{"line_number":1181,"context_line":"                yield o_down.result()"},{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"    def _download_object_job(self, conn, container, obj, options):"},{"line_number":1184,"context_line":"        out_file \u003d options[\u0027out_file\u0027]"},{"line_number":1185,"context_line":"        results_dict \u003d {}"},{"line_number":1186,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"28d3fa6d_78308562","line":1183,"updated":"2023-12-15 16:47:52.000000000","message":"we start way up here!","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":false,"context_lines":[{"line_number":1180,"context_line":"            for o_down in interruptable_as_completed(o_downs):"},{"line_number":1181,"context_line":"                yield o_down.result()"},{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"    def _download_object_job(self, conn, container, obj, options):"},{"line_number":1184,"context_line":"        out_file \u003d options[\u0027out_file\u0027]"},{"line_number":1185,"context_line":"        results_dict \u003d {}"},{"line_number":1186,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f45de519_1d9887d6","line":1183,"in_reply_to":"28d3fa6d_78308562","updated":"2023-12-15 20:58:21.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":1310,"context_line":"                                pseudodir \u003d True"},{"line_number":1311,"context_line":""},{"line_number":1312,"context_line":"                try:"},{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"                  for chunk in obj_body:"},{"line_number":1315,"context_line":"                    if fp is not None:"},{"line_number":1316,"context_line":"                        fp.write(chunk)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d6013926_9088fce9","line":1313,"updated":"2023-12-15 16:47:52.000000000","message":"I think a newline after try (and before finally for that matter) is potentially un-idiomatic.\n\n    try:\n        for chunk in obj_body:\n            fp.write(chunk)\n    except ...:\n        ...\n    finally:\n        close()\n\n^ perfectly reasonable to keep it tight and highlight the scoping.\n\nbut maybe this module adopted a more \"spread out\" convention and you\u0027re just following what\u0027s there.","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":false,"context_lines":[{"line_number":1310,"context_line":"                                pseudodir \u003d True"},{"line_number":1311,"context_line":""},{"line_number":1312,"context_line":"                try:"},{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"                  for chunk in obj_body:"},{"line_number":1315,"context_line":"                    if fp is not None:"},{"line_number":1316,"context_line":"                        fp.write(chunk)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c21a788c_e492342e","line":1313,"in_reply_to":"d6013926_9088fce9","updated":"2023-12-15 20:58:21.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"                except Exception as err:"},{"line_number":1319,"context_line":"                  if isinstance(err, ClientException):"},{"line_number":1320,"context_line":"                    raise"},{"line_number":1321,"context_line":"                  raise ClientException(str(err), http_response_headers\u003dheaders)"},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3b7d503f_6c02df6f","line":1320,"updated":"2023-12-15 16:47:52.000000000","message":"i think in python you might want to spell this:\n\n    try:\n        ...\n    except ClientException:\n        raise\n    except Exception:\n        raise ClientException(...)","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":true,"context_lines":[{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"                except Exception as err:"},{"line_number":1319,"context_line":"                  if isinstance(err, ClientException):"},{"line_number":1320,"context_line":"                    raise"},{"line_number":1321,"context_line":"                  raise ClientException(str(err), http_response_headers\u003dheaders)"},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":1,"id":"58c30ae9_9fe6f2e1","line":1320,"in_reply_to":"3b7d503f_6c02df6f","updated":"2023-12-15 20:58:21.000000000","message":"that\u0027s a good point","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"ff369b65ee798a0ade3f31b9ada635c06942bd55","unresolved":false,"context_lines":[{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"                except Exception as err:"},{"line_number":1319,"context_line":"                  if isinstance(err, ClientException):"},{"line_number":1320,"context_line":"                    raise"},{"line_number":1321,"context_line":"                  raise ClientException(str(err), http_response_headers\u003dheaders)"},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":1,"id":"70f4a4ac_5a4e8246","line":1320,"in_reply_to":"58c30ae9_9fe6f2e1","updated":"2023-12-15 20:58:35.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":1354,"context_line":"                \u0027response_dict\u0027: results_dict"},{"line_number":1355,"context_line":"            }"},{"line_number":1356,"context_line":"            return res"},{"line_number":1357,"context_line":""},{"line_number":1358,"context_line":"        except Exception as err:"},{"line_number":1359,"context_line":"            traceback, err_time \u003d report_traceback()"},{"line_number":1360,"context_line":"            logger.exception(err)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2e402abb_cf50e739","line":1357,"updated":"2023-12-15 16:47:52.000000000","message":"looks like this module be loving some new lines.  looks weird to me!","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":false,"context_lines":[{"line_number":1354,"context_line":"                \u0027response_dict\u0027: results_dict"},{"line_number":1355,"context_line":"            }"},{"line_number":1356,"context_line":"            return res"},{"line_number":1357,"context_line":""},{"line_number":1358,"context_line":"        except Exception as err:"},{"line_number":1359,"context_line":"            traceback, err_time \u003d report_traceback()"},{"line_number":1360,"context_line":"            logger.exception(err)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8417b9f6_9268184c","line":1357,"in_reply_to":"2e402abb_cf50e739","updated":"2023-12-15 20:58:21.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"97453658e210563ff8fd94c9c764ded4bb3f5efc","unresolved":true,"context_lines":[{"line_number":1371,"context_line":"                \u0027pseudodir\u0027: pseudodir,"},{"line_number":1372,"context_line":"                \u0027attempts\u0027: conn.attempts"},{"line_number":1373,"context_line":"            }"},{"line_number":1374,"context_line":"            return res"},{"line_number":1375,"context_line":""},{"line_number":1376,"context_line":"    def _submit_page_downloads(self, container, page_generator, options):"},{"line_number":1377,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"b83c9d30_8377f087","line":1374,"updated":"2023-12-15 16:47:52.000000000","message":"whoa this is a LONG method\n\nmeh. well it\u0027s \"only\" 190 lines.","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b3a13367b86238762e568873167bd080ca0cb4e7","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"                \u0027pseudodir\u0027: pseudodir,"},{"line_number":1372,"context_line":"                \u0027attempts\u0027: conn.attempts"},{"line_number":1373,"context_line":"            }"},{"line_number":1374,"context_line":"            return res"},{"line_number":1375,"context_line":""},{"line_number":1376,"context_line":"    def _submit_page_downloads(self, container, page_generator, options):"},{"line_number":1377,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"2176991c_f6d5f355","line":1374,"in_reply_to":"b83c9d30_8377f087","updated":"2023-12-15 20:58:21.000000000","message":"Acknowledged","commit_id":"816f9da4d39880373fbb5ad5083aacf629551796"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f8f6f1bca44939392ad8811b8e857175083adc90","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        self.segment \u003d segment"},{"line_number":76,"context_line":"        self.exception \u003d exc"},{"line_number":77,"context_line":"        self.value_trans_id \u003d str(value)"},{"line_number":78,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":79,"context_line":"            if (self.exception.transaction_id):"},{"line_number":80,"context_line":"                self.value_trans_id +\u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"4b93058c_204c16f5","line":78,"updated":"2024-01-10 15:44:38.000000000","message":"try/except tends to be the python way:\n\n```\n        try:\n            self.value_trans_id \u003d str(value) + \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id\n        except AttributeError:\n            self.value_trans_id \u003d str(value)\n```","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"e2ecd049601aeb6b14d7721bd93a943432c1e0c2","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        self.segment \u003d segment"},{"line_number":76,"context_line":"        self.exception \u003d exc"},{"line_number":77,"context_line":"        self.value_trans_id \u003d str(value)"},{"line_number":78,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":79,"context_line":"            if (self.exception.transaction_id):"},{"line_number":80,"context_line":"                self.value_trans_id +\u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"78c0a02c_52ba34d2","line":78,"in_reply_to":"4b93058c_204c16f5","updated":"2024-01-10 21:36:32.000000000","message":"Acknowledged","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f8f6f1bca44939392ad8811b8e857175083adc90","unresolved":true,"context_lines":[{"line_number":87,"context_line":"            value +\u003d \" object:%s\" % self.obj"},{"line_number":88,"context_line":"        if self.segment is not None:"},{"line_number":89,"context_line":"            value +\u003d \" segment:%s\" % self.segment"},{"line_number":90,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":91,"context_line":"            if (self.exception.transaction_id):"},{"line_number":92,"context_line":"                value +\u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id"},{"line_number":93,"context_line":"        return value"}],"source_content_type":"text/x-python","patch_set":2,"id":"eca18870_370e4437","line":90,"updated":"2024-01-10 15:44:38.000000000","message":"seems like this is repeating work done in the constructor - do we need value_trans_id at all? could the shell output str(SwiftError) and just append the transaction_id here in? similar to ``ClientException.__str__``","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"9421059c6ffec409c1061d9482358ee52485a47d","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            value +\u003d \" object:%s\" % self.obj"},{"line_number":88,"context_line":"        if self.segment is not None:"},{"line_number":89,"context_line":"            value +\u003d \" segment:%s\" % self.segment"},{"line_number":90,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":91,"context_line":"            if (self.exception.transaction_id):"},{"line_number":92,"context_line":"                value +\u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id"},{"line_number":93,"context_line":"        return value"}],"source_content_type":"text/x-python","patch_set":2,"id":"2bc94c9b_c1ed5aad","line":90,"in_reply_to":"083ab31d_871f7424","updated":"2024-01-22 16:35:19.000000000","message":"Acknowledged","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"488c644891e768fd6ea30fcea160325fe6ae0cd3","unresolved":true,"context_lines":[{"line_number":87,"context_line":"            value +\u003d \" object:%s\" % self.obj"},{"line_number":88,"context_line":"        if self.segment is not None:"},{"line_number":89,"context_line":"            value +\u003d \" segment:%s\" % self.segment"},{"line_number":90,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":91,"context_line":"            if (self.exception.transaction_id):"},{"line_number":92,"context_line":"                value +\u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id"},{"line_number":93,"context_line":"        return value"}],"source_content_type":"text/x-python","patch_set":2,"id":"083ab31d_871f7424","line":90,"in_reply_to":"14ea3d28_9cd2a52f","updated":"2024-01-22 11:26:44.000000000","message":"I like Tim\u0027s idea, but maybe push on it further and have the error handler in shell.py dig out ``SwiftError.exception.transaction_id``\n\nI\u0027m generally in favour of passing around data and formatting at the last moment.","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":87,"context_line":"            value +\u003d \" object:%s\" % self.obj"},{"line_number":88,"context_line":"        if self.segment is not None:"},{"line_number":89,"context_line":"            value +\u003d \" segment:%s\" % self.segment"},{"line_number":90,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":91,"context_line":"            if (self.exception.transaction_id):"},{"line_number":92,"context_line":"                value +\u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id"},{"line_number":93,"context_line":"        return value"}],"source_content_type":"text/x-python","patch_set":2,"id":"14ea3d28_9cd2a52f","line":90,"in_reply_to":"bead32c7_5781a44d","updated":"2024-01-20 00:18:10.000000000","message":"I\u0027d be partial toward having `SwiftError` just be a raise-able bag of data, and let shell.py handle the formatting, similar to how it does for `ClientException` -- then this would just need a\n```\nself.transaction_id \u003d getattr(exc, \u0027transaction_id\u0027, None)\n```\nup in `__init__`","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"fc6835488ed8c4dd9a268bde3cf859a185bc88b0","unresolved":true,"context_lines":[{"line_number":87,"context_line":"            value +\u003d \" object:%s\" % self.obj"},{"line_number":88,"context_line":"        if self.segment is not None:"},{"line_number":89,"context_line":"            value +\u003d \" segment:%s\" % self.segment"},{"line_number":90,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":91,"context_line":"            if (self.exception.transaction_id):"},{"line_number":92,"context_line":"                value +\u003d \u0027\\nFailed Transaction ID: %s\u0027 % self.exception.transaction_id"},{"line_number":93,"context_line":"        return value"}],"source_content_type":"text/x-python","patch_set":2,"id":"bead32c7_5781a44d","line":90,"in_reply_to":"eca18870_370e4437","updated":"2024-01-10 22:36:25.000000000","message":"If we simply append transaction id to value, would not need value_trans_id, and would have enough info in shell.py error output. Although that would break existing test cases in test_service.py e.g. test_swifterror_creation, where it apparently expects value could be non-string such as int. Not sure if we need to maintain such semantics expectation for value in existing test cases?","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"488c644891e768fd6ea30fcea160325fe6ae0cd3","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":96,"context_line":"            if (self.exception.transaction_id):"},{"line_number":97,"context_line":"                value +\u003d (\u0027\\nFailed Transaction ID: %s\u0027"},{"line_number":98,"context_line":"                          % self.exception.transaction_id)"},{"line_number":99,"context_line":"        return value"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def __repr__(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"32f2b40c_346c1a1e","line":98,"updated":"2024-01-22 11:26:44.000000000","message":"is this used? I reverted this change and the only test to fail is the one that tests the error constructor:\n\n```\ndiff --git a/swiftclient/service.py b/swiftclient/service.py\nindex 770e604..08eda03 100644\n--- a/swiftclient/service.py\n+++ b/swiftclient/service.py\n@@ -92,10 +92,10 @@ class SwiftError(Exception):\n             value +\u003d \" object:%s\" % self.obj\n         if self.segment is not None:\n             value +\u003d \" segment:%s\" % self.segment\n-        if hasattr(self.exception, \u0027transaction_id\u0027):\n-            if (self.exception.transaction_id):\n-                value +\u003d (\u0027\\nFailed Transaction ID: %s\u0027\n-                          % self.exception.transaction_id)\n+        # if hasattr(self.exception, \u0027transaction_id\u0027):\n+        #     if (self.exception.transaction_id):\n+        #         value +\u003d (\u0027\\nFailed Transaction ID: %s\u0027\n+        #                   % self.exception.transaction_id)\n         return value\n\n     def __repr__(self):\n     ```","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"2043ee6c9467d4487405edbbfe062f5ea0499fef","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":96,"context_line":"            if (self.exception.transaction_id):"},{"line_number":97,"context_line":"                value +\u003d (\u0027\\nFailed Transaction ID: %s\u0027"},{"line_number":98,"context_line":"                          % self.exception.transaction_id)"},{"line_number":99,"context_line":"        return value"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def __repr__(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"81f5c117_c5dfaedc","line":98,"in_reply_to":"090662d8_98e6198a","updated":"2024-01-22 21:35:22.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        if hasattr(self.exception, \u0027transaction_id\u0027):"},{"line_number":96,"context_line":"            if (self.exception.transaction_id):"},{"line_number":97,"context_line":"                value +\u003d (\u0027\\nFailed Transaction ID: %s\u0027"},{"line_number":98,"context_line":"                          % self.exception.transaction_id)"},{"line_number":99,"context_line":"        return value"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def __repr__(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"090662d8_98e6198a","line":98,"in_reply_to":"32f2b40c_346c1a1e","updated":"2024-01-22 20:36:27.000000000","message":"This should not be required. It seems logical to add transaction id to both SwiftError\u0027s \"value\" string and str() output, to be consistent with ClientException str() output. But not strictly required. If we follow your and Tim\u0027s comment above to move SwiftError formatting to shell.py, this should then be removed.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                self._content_length \u003d int(headers.get(\u0027content-length\u0027))"},{"line_number":474,"context_line":"            except ValueError:"},{"line_number":475,"context_line":"                raise SwiftError(\u0027content-length header must be an integer\u0027)"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"    def __iter__(self):"},{"line_number":478,"context_line":"        for chunk in self._body:"}],"source_content_type":"text/x-python","patch_set":3,"id":"2f4f0d40_3870f85e","line":475,"updated":"2024-01-20 00:18:10.000000000","message":"Hm. Another place that could probably use a transaction ID... I guess the precedent in `_SwiftReader` would recommend something like\n```\nraise SwiftError(\u0027content-length header must be an integer (txn: {})\u0027.format(\n                 self._txn_id or \u0027unknown\u0027))\n```","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":false,"context_lines":[{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                self._content_length \u003d int(headers.get(\u0027content-length\u0027))"},{"line_number":474,"context_line":"            except ValueError:"},{"line_number":475,"context_line":"                raise SwiftError(\u0027content-length header must be an integer\u0027)"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"    def __iter__(self):"},{"line_number":478,"context_line":"        for chunk in self._body:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ebe04d3_852029f9","line":475,"in_reply_to":"2f4f0d40_3870f85e","updated":"2024-01-22 20:36:27.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":1325,"context_line":"                            fp.write(chunk)"},{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"35d1b461_3b7872fe","line":1328,"range":{"start_line":1328,"start_character":23,"end_line":1328,"end_character":32},"updated":"2024-01-20 00:18:10.000000000","message":"What kind of errors are we expecting to catch here? This feels rather broad; it\u0027ll mask things like typos down in client:\n```\nERROR:swiftclient.service:name \u0027bf\u0027 is not defined (txn: tx15154dfea5af453d956ff-0065aaf767)\nTraceback (most recent call last):\n  File \"/vagrant/python-swiftclient/swiftclient/service.py\", line 1323, in _download_object_job\n    for chunk in obj_body:\n  File \"/vagrant/python-swiftclient/swiftclient/service.py\", line 478, in __iter__\n    for chunk in self._body:\n  File \"/vagrant/python-swiftclient/swiftclient/client.py\", line 245, in __next__\n    return self.next()\n  File \"/vagrant/python-swiftclient/swiftclient/client.py\", line 239, in next\n    buf \u003d self.read(self.chunk_size)\n  File \"/vagrant/python-swiftclient/swiftclient/client.py\", line 324, in read\n    return bf\nNameError: name \u0027bf\u0027 is not defined\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n  File \"/vagrant/python-swiftclient/swiftclient/service.py\", line 1329, in _download_object_job\n    raise ClientException(\nswiftclient.exceptions.ClientException: name \u0027bf\u0027 is not defined (txn: tx15154dfea5af453d956ff-0065aaf767)\n```\nIf it\u0027s the `socket.error, RequestException` we re-raise when we run out of retries, we ought to just catch those. And I guess some new urllib3 errors we aren\u0027t currently catching and retrying but should?","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":true,"context_lines":[{"line_number":1325,"context_line":"                            fp.write(chunk)"},{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fdd52cfd_1868b114","line":1328,"range":{"start_line":1328,"start_character":23,"end_line":1328,"end_character":32},"in_reply_to":"35d1b461_3b7872fe","updated":"2024-01-22 20:36:27.000000000","message":"urllib3.exceptions.ProtocolError returns e.g. \u0027Connection broken: IncompleteRead ...\u0027 for this. Could probably catch this and socket.error, RequestException.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b97969052d0baa8b2c5dc4af90254ac1b2239e48","unresolved":false,"context_lines":[{"line_number":1325,"context_line":"                            fp.write(chunk)"},{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7714d10b_93ebb749","line":1328,"range":{"start_line":1328,"start_character":23,"end_line":1328,"end_character":32},"in_reply_to":"fdd52cfd_1868b114","updated":"2024-02-29 20:56:18.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"790d732d_f51b596c","line":1329,"updated":"2024-01-20 00:18:10.000000000","message":"It\u0027s a little weird to me that we\u0027re raising `ClientException` from service.py, rather than client.py. I suppose there\u0027s precedent for it with some stuff around large objects... but I\u0027d argue those probably should have been `SwiftError`s from the get-go.\n\nWhy isn\u0027t this error transformation happening in `_RetryBody`/`_ObjectBody`?","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"8ae0c26d16a2a96ba45803d4a50485dbe1b07199","unresolved":true,"context_lines":[{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"8cb95f8e_f1e024e4","line":1329,"in_reply_to":"7124734d_95f97e0e","updated":"2024-02-15 20:22:32.000000000","message":"Actually the orignal code in client.py was not catching urllib3 error. Added that, although other code won\u0027t change, as service.py still needs to catch these errors for adding transaction id.\n\nThrowing clientexception and morphing to swifterror here seems a bit strange. Unless you and Tim feel that\u0027s necessary, maybe we could address the exception handling in another task?","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":true,"context_lines":[{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"e8e3d055_b74ded0f","line":1329,"in_reply_to":"790d732d_f51b596c","updated":"2024-01-22 20:36:27.000000000","message":"Good point. There\u0027s precedent for it. Could be a SwiftError. Thought that SwiftError was for errors returned from Swift server, ClientException was for errors from HTTP connection or client code including service.py. Could we confirm that ClientException is intended for errors from client.py only? If throwing this error from _ObjectBody, there is no context for the response headers.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cb3bc7591a9cb9e772711cae3d9e8fa511918ffa","unresolved":true,"context_lines":[{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"fd7dbba2_47da05a0","line":1329,"in_reply_to":"8cb95f8e_f1e024e4","updated":"2024-02-16 10:36:56.000000000","message":"I\u0027m happy to limit the scope of this patch","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b530b478d4bef9c6b7ec571e1c80812625d03317","unresolved":true,"context_lines":[{"line_number":1326,"context_line":"                except ClientException:"},{"line_number":1327,"context_line":"                    raise"},{"line_number":1328,"context_line":"                except Exception as err:"},{"line_number":1329,"context_line":"                    raise ClientException("},{"line_number":1330,"context_line":"                        str(err), http_response_headers\u003dheaders)"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"                finish_time \u003d time()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7124734d_95f97e0e","line":1329,"in_reply_to":"e8e3d055_b74ded0f","updated":"2024-01-31 15:12:16.000000000","message":"It does seem that client.py will raise RequestException, socket_error rather than morphing them to ClientException, in more than one place.\n\nAlso looks like there are places where service.py catches ClientExcpetion and morphs it to SwiftError, so we could do that here.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b530b478d4bef9c6b7ec571e1c80812625d03317","unresolved":true,"context_lines":[{"line_number":466,"context_line":"                raise SwiftError("},{"line_number":467,"context_line":"                    \u0027content-length header must be an integer\u0027"},{"line_number":468,"context_line":"                    \u0027\\nFailed Transaction ID: {}\u0027"},{"line_number":469,"context_line":"                    .format(self._txn_id or \u0027unknown\u0027))"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def __iter__(self):"},{"line_number":472,"context_line":"        for chunk in self._body:"}],"source_content_type":"text/x-python","patch_set":4,"id":"593aaa83_acaaff02","line":469,"updated":"2024-01-31 15:12:16.000000000","message":"It\u0027s unfortunate to have this \"special case\" where the \"Failed Transaction ID...\" needs to be embedded in the \u0027value\u0027. It also means that the \"Failed Transaction ID...\" i snot actually written to a new line because str(SwiftError) uses the repr of value, so the \\n gets escaped.\n\nI suggest making SwiftError accept an optional transaction_id kwarg that we can just pass in here, and then in shell do the same formatting as elsewhere when this error is caught. I also prefer that because we get to \"pass data, format it later\".\n\nsee https://review.opendev.org/c/openstack/python-swiftclient/+/907331 for ideas","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"8ae0c26d16a2a96ba45803d4a50485dbe1b07199","unresolved":false,"context_lines":[{"line_number":466,"context_line":"                raise SwiftError("},{"line_number":467,"context_line":"                    \u0027content-length header must be an integer\u0027"},{"line_number":468,"context_line":"                    \u0027\\nFailed Transaction ID: {}\u0027"},{"line_number":469,"context_line":"                    .format(self._txn_id or \u0027unknown\u0027))"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def __iter__(self):"},{"line_number":472,"context_line":"        for chunk in self._body:"}],"source_content_type":"text/x-python","patch_set":4,"id":"25147bca_116a5cc9","line":469,"in_reply_to":"593aaa83_acaaff02","updated":"2024-02-15 20:22:32.000000000","message":"good idea, squashed the changes","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4502207b2b292c4c6649aa596bca79da42bebb","unresolved":true,"context_lines":[{"line_number":1319,"context_line":"                    for chunk in obj_body:"},{"line_number":1320,"context_line":"                        if fp is not None:"},{"line_number":1321,"context_line":"                            fp.write(chunk)"},{"line_number":1322,"context_line":"                except ClientException:"},{"line_number":1323,"context_line":"                    raise"},{"line_number":1324,"context_line":"                except (socket_error,"},{"line_number":1325,"context_line":"                        urllib_http_error,"},{"line_number":1326,"context_line":"                        RequestException) as err:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fc0c3fdd_55b0c69d","line":1323,"range":{"start_line":1322,"start_character":16,"end_line":1323,"end_character":25},"updated":"2024-02-28 18:31:37.000000000","message":"`ClientException` doesn\u0027t inherit from any of the other ones we\u0027re catching, does it? Do we need this?","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"1ba736a765e881a986c14193e29e7e4c33ec1941","unresolved":false,"context_lines":[{"line_number":1319,"context_line":"                    for chunk in obj_body:"},{"line_number":1320,"context_line":"                        if fp is not None:"},{"line_number":1321,"context_line":"                            fp.write(chunk)"},{"line_number":1322,"context_line":"                except ClientException:"},{"line_number":1323,"context_line":"                    raise"},{"line_number":1324,"context_line":"                except (socket_error,"},{"line_number":1325,"context_line":"                        urllib_http_error,"},{"line_number":1326,"context_line":"                        RequestException) as err:"}],"source_content_type":"text/x-python","patch_set":6,"id":"c4e39bc9_24f12348","line":1323,"range":{"start_line":1322,"start_character":16,"end_line":1323,"end_character":25},"in_reply_to":"fc0c3fdd_55b0c69d","updated":"2024-02-29 20:39:48.000000000","message":"Previously was catching Exception, thus needed this. No longer needed since we are catching specific exceptions.","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"}],"swiftclient/shell.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f8f6f1bca44939392ad8811b8e857175083adc90","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                        output_manager.error(\u0027Error Deleting: {0}: {1}\u0027"},{"line_number":228,"context_line":"                                             .format(p, r[\u0027error\u0027]))"},{"line_number":229,"context_line":"        except SwiftError as err:"},{"line_number":230,"context_line":"            output_manager.error(err.value_trans_id)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"st_download_options \u003d \u0027\u0027\u0027[--all] [--marker \u003cmarker\u003e] [--prefix \u003cprefix\u003e]"}],"source_content_type":"text/x-python","patch_set":2,"id":"d7e386df_39e8de2b","line":230,"updated":"2024-01-10 15:44:38.000000000","message":"what happens if we make this str(err)? Do we get more or less info in the output?","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"488c644891e768fd6ea30fcea160325fe6ae0cd3","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                        output_manager.error(\u0027Error Deleting: {0}: {1}\u0027"},{"line_number":228,"context_line":"                                             .format(p, r[\u0027error\u0027]))"},{"line_number":229,"context_line":"        except SwiftError as err:"},{"line_number":230,"context_line":"            output_manager.error(err.value_trans_id)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"st_download_options \u003d \u0027\u0027\u0027[--all] [--marker \u003cmarker\u003e] [--prefix \u003cprefix\u003e]"}],"source_content_type":"text/x-python","patch_set":2,"id":"fc521778_7e7d7e75","line":230,"in_reply_to":"7e021ea3_cdd7c837","updated":"2024-01-22 11:26:44.000000000","message":"Done","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"fc6835488ed8c4dd9a268bde3cf859a185bc88b0","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                        output_manager.error(\u0027Error Deleting: {0}: {1}\u0027"},{"line_number":228,"context_line":"                                             .format(p, r[\u0027error\u0027]))"},{"line_number":229,"context_line":"        except SwiftError as err:"},{"line_number":230,"context_line":"            output_manager.error(err.value_trans_id)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"st_download_options \u003d \u0027\u0027\u0027[--all] [--marker \u003cmarker\u003e] [--prefix \u003cprefix\u003e]"}],"source_content_type":"text/x-python","patch_set":2,"id":"7e021ea3_cdd7c837","line":230,"in_reply_to":"d7e386df_39e8de2b","updated":"2024-01-10 22:36:25.000000000","message":"Using str(err) would have more info, including container, object etc. Might not be desired for error output though.","commit_id":"d91add678e50f0b33161389e7a7ffaf20a42ef26"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c662e29603b98dec976edbc4d460768318e88b6","unresolved":true,"context_lines":[{"line_number":2080,"context_line":"                    InsecureRequestWarning)"},{"line_number":2081,"context_line":"        try:"},{"line_number":2082,"context_line":"            globals()[\u0027st_%s\u0027 % args[0]](parser, argv[1:], output)"},{"line_number":2083,"context_line":"        except ClientException as err:"},{"line_number":2084,"context_line":"            trans_id \u003d err.transaction_id"},{"line_number":2085,"context_line":"            err.transaction_id \u003d None  # clear it so we aren\u0027t overly noisy"},{"line_number":2086,"context_line":"            output.error(str(err))"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a0572ee_3cd14a88","line":2083,"updated":"2024-01-20 00:18:10.000000000","message":"I can\u0027t help but wonder if we should move all the `SwiftError` handling down here.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"9421059c6ffec409c1061d9482358ee52485a47d","unresolved":false,"context_lines":[{"line_number":2080,"context_line":"                    InsecureRequestWarning)"},{"line_number":2081,"context_line":"        try:"},{"line_number":2082,"context_line":"            globals()[\u0027st_%s\u0027 % args[0]](parser, argv[1:], output)"},{"line_number":2083,"context_line":"        except ClientException as err:"},{"line_number":2084,"context_line":"            trans_id \u003d err.transaction_id"},{"line_number":2085,"context_line":"            err.transaction_id \u003d None  # clear it so we aren\u0027t overly noisy"},{"line_number":2086,"context_line":"            output.error(str(err))"}],"source_content_type":"text/x-python","patch_set":3,"id":"53fbada8_722c2a9e","line":2083,"in_reply_to":"34ea5557_f3348ace","updated":"2024-01-22 16:35:19.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"488c644891e768fd6ea30fcea160325fe6ae0cd3","unresolved":true,"context_lines":[{"line_number":2080,"context_line":"                    InsecureRequestWarning)"},{"line_number":2081,"context_line":"        try:"},{"line_number":2082,"context_line":"            globals()[\u0027st_%s\u0027 % args[0]](parser, argv[1:], output)"},{"line_number":2083,"context_line":"        except ClientException as err:"},{"line_number":2084,"context_line":"            trans_id \u003d err.transaction_id"},{"line_number":2085,"context_line":"            err.transaction_id \u003d None  # clear it so we aren\u0027t overly noisy"},{"line_number":2086,"context_line":"            output.error(str(err))"}],"source_content_type":"text/x-python","patch_set":3,"id":"34ea5557_f3348ace","line":2083,"in_reply_to":"5a0572ee_3cd14a88","updated":"2024-01-22 11:26:44.000000000","message":"+1, either that or consider a helper function to print the error message, which gets called here and every place we have ``output_manager.error(e.error_value_str)``.\n\nNoting that a SwiftError has the underlying ClientException, the only(?) difference between handling a SwiftError vs ClientException would be that we need to look for err.exception.transaction_id rather than err.transaction_id (and be prepared to catch AttributeErrors in case the exception or transaction_id are not available).\n\nIn general, having output formatting in one place makes for easier maintenance.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b530b478d4bef9c6b7ec571e1c80812625d03317","unresolved":true,"context_lines":[{"line_number":228,"context_line":"                                             .format(p, r[\u0027error\u0027]))"},{"line_number":229,"context_line":"        except SwiftError as err:"},{"line_number":230,"context_line":"            output_manager.error(\"{}\\nFailed Transaction ID: {}\".format("},{"line_number":231,"context_line":"                err.value, err.transaction_id or \u0027unknown\u0027))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"st_download_options \u003d \u0027\u0027\u0027[--all] [--marker \u003cmarker\u003e] [--prefix \u003cprefix\u003e]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bd3c6a68_16959b4a","line":231,"updated":"2024-01-31 15:12:16.000000000","message":"seems like a helper method might be useful to eliminate the repetition of this pattern\n\n```\ndef output_error_with_txn_id(output_manager, message, err):\n    # try to get transaction id from err and append Failed Transaction ID: \n```","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"8ae0c26d16a2a96ba45803d4a50485dbe1b07199","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                                             .format(p, r[\u0027error\u0027]))"},{"line_number":229,"context_line":"        except SwiftError as err:"},{"line_number":230,"context_line":"            output_manager.error(\"{}\\nFailed Transaction ID: {}\".format("},{"line_number":231,"context_line":"                err.value, err.transaction_id or \u0027unknown\u0027))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"st_download_options \u003d \u0027\u0027\u0027[--all] [--marker \u003cmarker\u003e] [--prefix \u003cprefix\u003e]"}],"source_content_type":"text/x-python","patch_set":4,"id":"4d0314e3_be428dc3","line":231,"in_reply_to":"bd3c6a68_16959b4a","updated":"2024-02-15 20:22:32.000000000","message":"Acknowledged","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4502207b2b292c4c6649aa596bca79da42bebb","unresolved":true,"context_lines":[{"line_number":52,"context_line":"            \u0027capabilities\u0027, \u0027info\u0027, \u0027tempurl\u0027, \u0027auth\u0027, \u0027bash_completion\u0027)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def output_error_with_txn_id(output_manager, err):"},{"line_number":56,"context_line":"    output_manager.error(\"{}\\nFailed Transaction ID: {}\".format("},{"line_number":57,"context_line":"        err.value, err.transaction_id or \u0027unknown\u0027))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"19780acf_1f307b37","line":55,"range":{"start_line":55,"start_character":4,"end_line":55,"end_character":28},"updated":"2024-02-28 18:31:37.000000000","message":"I wonder if this might look better as a method off `OutputManager` in multithreading.py","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"1ba736a765e881a986c14193e29e7e4c33ec1941","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            \u0027capabilities\u0027, \u0027info\u0027, \u0027tempurl\u0027, \u0027auth\u0027, \u0027bash_completion\u0027)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def output_error_with_txn_id(output_manager, err):"},{"line_number":56,"context_line":"    output_manager.error(\"{}\\nFailed Transaction ID: {}\".format("},{"line_number":57,"context_line":"        err.value, err.transaction_id or \u0027unknown\u0027))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"b2033fb8_6abd1837","line":55,"range":{"start_line":55,"start_character":4,"end_line":55,"end_character":28},"in_reply_to":"19780acf_1f307b37","updated":"2024-02-29 20:39:48.000000000","message":"Acknowledged","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4502207b2b292c4c6649aa596bca79da42bebb","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def output_error_with_txn_id(output_manager, err):"},{"line_number":56,"context_line":"    output_manager.error(\"{}\\nFailed Transaction ID: {}\".format("},{"line_number":57,"context_line":"        err.value, err.transaction_id or \u0027unknown\u0027))"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def immediate_exit(signum, frame):"}],"source_content_type":"text/x-python","patch_set":6,"id":"dd08688e_ff00511c","line":57,"range":{"start_line":57,"start_character":38,"end_line":57,"end_character":50},"updated":"2024-02-28 18:31:37.000000000","message":"I like having the guard, just in case -- have you ever seen it come up in practice?","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f008615d3df1c48f1a905db05696c4d6d91bebc6","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def output_error_with_txn_id(output_manager, err):"},{"line_number":56,"context_line":"    output_manager.error(\"{}\\nFailed Transaction ID: {}\".format("},{"line_number":57,"context_line":"        err.value, err.transaction_id or \u0027unknown\u0027))"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def immediate_exit(signum, frame):"}],"source_content_type":"text/x-python","patch_set":6,"id":"68a96c7b_8f4b73bc","line":57,"range":{"start_line":57,"start_character":38,"end_line":57,"end_character":50},"in_reply_to":"71fd20d5_737743cc","updated":"2024-03-01 17:15:35.000000000","message":"Acknowledged","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"1ba736a765e881a986c14193e29e7e4c33ec1941","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def output_error_with_txn_id(output_manager, err):"},{"line_number":56,"context_line":"    output_manager.error(\"{}\\nFailed Transaction ID: {}\".format("},{"line_number":57,"context_line":"        err.value, err.transaction_id or \u0027unknown\u0027))"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def immediate_exit(signum, frame):"}],"source_content_type":"text/x-python","patch_set":6,"id":"71fd20d5_737743cc","line":57,"range":{"start_line":57,"start_character":38,"end_line":57,"end_character":50},"in_reply_to":"dd08688e_ff00511c","updated":"2024-02-29 20:39:48.000000000","message":"It seems mostly in unit tests","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"}],"test/unit/test_service.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4502207b2b292c4c6649aa596bca79da42bebb","unresolved":true,"context_lines":[{"line_number":170,"context_line":"        # Check Contentlength raises error if it isn\u0027t an integer"},{"line_number":171,"context_line":"        with self.assertRaises(SwiftError) as cm:"},{"line_number":172,"context_line":"            self.sr(\u0027path\u0027, \u0027body\u0027, {\u0027content-length\u0027: \u0027notanint\u0027})"},{"line_number":173,"context_line":"        self.assertEqual(\"\u0027content-length header must be an integer\u0027\","},{"line_number":174,"context_line":"                         str(cm.exception))"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def test_iterator_usage(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1694b9b1_7e0bbe79","line":173,"range":{"start_line":173,"start_character":25,"end_line":173,"end_character":27},"updated":"2024-02-28 18:31:37.000000000","message":"Oh, interesting that it includes quotes inside the string...","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"1ba736a765e881a986c14193e29e7e4c33ec1941","unresolved":true,"context_lines":[{"line_number":170,"context_line":"        # Check Contentlength raises error if it isn\u0027t an integer"},{"line_number":171,"context_line":"        with self.assertRaises(SwiftError) as cm:"},{"line_number":172,"context_line":"            self.sr(\u0027path\u0027, \u0027body\u0027, {\u0027content-length\u0027: \u0027notanint\u0027})"},{"line_number":173,"context_line":"        self.assertEqual(\"\u0027content-length header must be an integer\u0027\","},{"line_number":174,"context_line":"                         str(cm.exception))"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def test_iterator_usage(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"d9b7f5f8_406b5f07","line":173,"range":{"start_line":173,"start_character":25,"end_line":173,"end_character":27},"in_reply_to":"1694b9b1_7e0bbe79","updated":"2024-02-29 20:39:48.000000000","message":"Seems because of SwiftError using repr in __str__","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"af4502207b2b292c4c6649aa596bca79da42bebb","unresolved":true,"context_lines":[{"line_number":686,"context_line":"        self.assertEqual(se.exception, test_exc)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"        self.assertEqual(\u0027someTransId\u0027, se.transaction_id)"},{"line_number":689,"context_line":"        self.assertIn(\u00275 container:con object:obj segment:seg\u0027, str(se))"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class TestServiceUtils(unittest.TestCase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"5b5304fa_81245003","line":689,"updated":"2024-02-28 18:31:37.000000000","message":"Maybe add a\n```\nself.assertNotIn(\u0027someTransId\u0027, str(se))\n```\n?","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"1ba736a765e881a986c14193e29e7e4c33ec1941","unresolved":false,"context_lines":[{"line_number":686,"context_line":"        self.assertEqual(se.exception, test_exc)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"        self.assertEqual(\u0027someTransId\u0027, se.transaction_id)"},{"line_number":689,"context_line":"        self.assertIn(\u00275 container:con object:obj segment:seg\u0027, str(se))"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":""},{"line_number":692,"context_line":"class TestServiceUtils(unittest.TestCase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"7db22210_1829d392","line":689,"in_reply_to":"5b5304fa_81245003","updated":"2024-02-29 20:39:48.000000000","message":"Acknowledged","commit_id":"90d99ae25b8b73ab8fcac0b14500f67d91074bcc"}],"test/unit/test_shell.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"488c644891e768fd6ea30fcea160325fe6ae0cd3","unresolved":true,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":779,"context_line":"                          \u0027test_exc (txn: someTransId)\u0027,"},{"line_number":780,"context_line":"                          str(output.err))"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":783,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dd52281c_848aab1e","line":780,"updated":"2024-01-22 11:26:44.000000000","message":"this seems anomalous (i.e. the trans id is in parentheses in the error string rather than appended as an extra line).\n\nMaybe something to do with the shell logging the error at line 486 in st_download rather than in the exception handlers at the end of that function???","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"f3560307757f95ac240eb976d98a6f8b67af5dc3","unresolved":false,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":779,"context_line":"                          \u0027test_exc (txn: someTransId)\u0027,"},{"line_number":780,"context_line":"                          str(output.err))"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":783,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"13235e21_841419f0","line":780,"in_reply_to":"1c2d4f24_07a979d1","updated":"2024-01-23 21:16:29.000000000","message":"Acknowledged","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"2043ee6c9467d4487405edbbfe062f5ea0499fef","unresolved":true,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":779,"context_line":"                          \u0027test_exc (txn: someTransId)\u0027,"},{"line_number":780,"context_line":"                          str(output.err))"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":783,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f3874535_79008683","line":780,"in_reply_to":"aa526176_ee9b4568","updated":"2024-01-22 21:35:22.000000000","message":"Thinking that we are going to keep the shell.py error handling as it is currently, for example handling of errors in the st_download loop. Only moving the formatting of SwiftError value, transaction id, etc. from init()/str() to shell.py. Is that right?","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3f74a8a70a7092c0bdc9c49f1f65a9aaf0f60c01","unresolved":true,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":779,"context_line":"                          \u0027test_exc (txn: someTransId)\u0027,"},{"line_number":780,"context_line":"                          str(output.err))"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":783,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"aa526176_ee9b4568","line":780,"in_reply_to":"dd52281c_848aab1e","updated":"2024-01-22 20:50:59.000000000","message":"But having the error handling in the download loop is kinda nice -- if I want to download a whole \"directory\" with hundreds of objects, I probably don\u0027t want to abort the whole thing because I ran out of retries on one of them. I\u0027d prefer to have the download continue, then run the command again with a `--skip-identical`. Which might be an argument _against_ moving all the SwiftError handling into `main`...","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"54181cd8ef6622ad9413bffded801bc0fea4ba97","unresolved":true,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":779,"context_line":"                          \u0027test_exc (txn: someTransId)\u0027,"},{"line_number":780,"context_line":"                          str(output.err))"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":783,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"efe20159_8ffc47b2","line":780,"in_reply_to":"dd52281c_848aab1e","updated":"2024-01-22 20:36:27.000000000","message":"Right the existing output formats for errors seem a bit insconsistent. ClientException format prints transaction id in parentheses, while shell.py main() function outputs transaction id as extra line. To limit the scope of changes, not attempting to change those. But could be considered if desired.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"101b8cddc008ac3763ac54ff62c03c63347c767f","unresolved":true,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":779,"context_line":"                          \u0027test_exc (txn: someTransId)\u0027,"},{"line_number":780,"context_line":"                          str(output.err))"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":783,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1c2d4f24_07a979d1","line":780,"in_reply_to":"f3874535_79008683","updated":"2024-01-23 15:58:57.000000000","message":"I didn\u0027t mean to suggest that we move the download error handling (agree with Tim\u0027s reasons) but just that it would be *nice-to-have* the format consistent.","commit_id":"338d03302195bace9d53f4527f96a126469415fc"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b530b478d4bef9c6b7ec571e1c80812625d03317","unresolved":true,"context_lines":[{"line_number":779,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":780,"context_line":"                          \u0027test_exc\u0027"},{"line_number":781,"context_line":"                          \u0027\\nFailed Transaction ID: someTransId\u0027,"},{"line_number":782,"context_line":"                          str(output.err))"},{"line_number":783,"context_line":""},{"line_number":784,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":785,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3b5500fc_c1f1f81b","line":782,"updated":"2024-01-31 15:12:16.000000000","message":"I tried to get the mocking lower in the stack to be able to trace through more of the client.Connection code in a debugger: see https://review.opendev.org/c/openstack/python-swiftclient/+/907332","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"8ae0c26d16a2a96ba45803d4a50485dbe1b07199","unresolved":false,"context_lines":[{"line_number":779,"context_line":"            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027"},{"line_number":780,"context_line":"                          \u0027test_exc\u0027"},{"line_number":781,"context_line":"                          \u0027\\nFailed Transaction ID: someTransId\u0027,"},{"line_number":782,"context_line":"                          str(output.err))"},{"line_number":783,"context_line":""},{"line_number":784,"context_line":"    @mock.patch(\u0027swiftclient.service.shuffle\u0027)"},{"line_number":785,"context_line":"    @mock.patch(\u0027swiftclient.service.Connection\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"c106fac5_dcfde9be","line":782,"in_reply_to":"3b5500fc_c1f1f81b","updated":"2024-02-15 20:22:32.000000000","message":"good idea, needs a fake iter for retry\u003d\u003d0","commit_id":"5c9d32623b4214d3f23bcbd8b6e09206146cae4f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cb3bc7591a9cb9e772711cae3d9e8fa511918ffa","unresolved":true,"context_lines":[{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def test_download_clientexception(self):"},{"line_number":756,"context_line":"        def fakeIter(arg):"},{"line_number":757,"context_line":"            raise RequestException(\u0027test_exc\u0027)"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"        retry_calls \u003d []"},{"line_number":760,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"c36f48c6_adf89bf0","line":757,"updated":"2024-02-16 10:36:56.000000000","message":"the func isn\u0027t needed if ``body.__iter__`` is given a side_effect to raise the exception","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"72e0ed600b5fc9c914b75738670449142e3a450c","unresolved":false,"context_lines":[{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def test_download_clientexception(self):"},{"line_number":756,"context_line":"        def fakeIter(arg):"},{"line_number":757,"context_line":"            raise RequestException(\u0027test_exc\u0027)"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"        retry_calls \u003d []"},{"line_number":760,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"94dd2024_053b48a6","line":757,"in_reply_to":"c36f48c6_adf89bf0","updated":"2024-02-28 16:34:57.000000000","message":"Done","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cb3bc7591a9cb9e772711cae3d9e8fa511918ffa","unresolved":true,"context_lines":[{"line_number":763,"context_line":"            conn.attempts +\u003d 1"},{"line_number":764,"context_line":""},{"line_number":765,"context_line":"            body \u003d mock.MagicMock()"},{"line_number":766,"context_line":"            body.resp.read.side_effect \u003d RequestException(\u0027test_exc\u0027)"},{"line_number":767,"context_line":"            body.__iter__ \u003d fakeIter"},{"line_number":768,"context_line":"            return (CaseInsensitiveDict({"},{"line_number":769,"context_line":"                \u0027content-type\u0027: \u0027text/plain\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3434c584_b98b321e","line":766,"updated":"2024-02-16 10:36:56.000000000","message":"what about socket.error, urllib_http_error ?\n\nI suggest a helper _do_test_download_clientexception(self, exception) and a test method for each exception type","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"72e0ed600b5fc9c914b75738670449142e3a450c","unresolved":false,"context_lines":[{"line_number":763,"context_line":"            conn.attempts +\u003d 1"},{"line_number":764,"context_line":""},{"line_number":765,"context_line":"            body \u003d mock.MagicMock()"},{"line_number":766,"context_line":"            body.resp.read.side_effect \u003d RequestException(\u0027test_exc\u0027)"},{"line_number":767,"context_line":"            body.__iter__ \u003d fakeIter"},{"line_number":768,"context_line":"            return (CaseInsensitiveDict({"},{"line_number":769,"context_line":"                \u0027content-type\u0027: \u0027text/plain\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"67db4469_fa109007","line":766,"in_reply_to":"3434c584_b98b321e","updated":"2024-02-28 16:34:57.000000000","message":"Done","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cb3bc7591a9cb9e772711cae3d9e8fa511918ffa","unresolved":true,"context_lines":[{"line_number":764,"context_line":""},{"line_number":765,"context_line":"            body \u003d mock.MagicMock()"},{"line_number":766,"context_line":"            body.resp.read.side_effect \u003d RequestException(\u0027test_exc\u0027)"},{"line_number":767,"context_line":"            body.__iter__ \u003d fakeIter"},{"line_number":768,"context_line":"            return (CaseInsensitiveDict({"},{"line_number":769,"context_line":"                \u0027content-type\u0027: \u0027text/plain\u0027,"},{"line_number":770,"context_line":"                \u0027etag\u0027: \u00272cbbfe139a744d6abbe695e17f3c1991\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"652ff1e9_19c60ebc","line":767,"updated":"2024-02-16 10:36:56.000000000","message":"ah, that\u0027s what I was missing for retries\u003d0! 👍\n\nBut, we don\u0027t need this fakeIter for retries\u003d\u003d1 (because a _RetryBody gets constructed). I think the test can be simplified, or we can add another test case for retries\u003d0:\n\n```\n    def test_download_clientexception_retries_0(self):\n        retry_calls \u003d []\n\n        def _fake_retry(conn, *args, **kwargs):\n            retry_calls.append((args, kwargs))\n            conn.attempts +\u003d 1\n\n            body \u003d mock.MagicMock()\n            body.__iter__.side_effect \u003d RequestException(\u0027test_exc\u0027)\n            return (CaseInsensitiveDict({\n                \u0027content-type\u0027: \u0027text/plain\u0027,\n                \u0027etag\u0027: \u00272cbbfe139a744d6abbe695e17f3c1991\u0027,\n                \u0027x-trans-id\u0027: \u0027someTransId\u0027}),\n                body)\n\n        argv \u003d [\"\", \"download\", \"container\", \"object\", \"--retries\", \"0\"]\n        with CaptureOutput() as output:\n            with mock.patch(BUILTIN_OPEN) as mock_open:\n                with mock.patch(\"swiftclient.service.Connection._retry\",\n                                _fake_retry):\n                    with self.assertRaises(SystemExit):\n                        swiftclient.shell.main(argv)\n                    mock_open.assert_called_with(\u0027object\u0027, \u0027wb\u0027, 65536)\n            self.assertEqual([\n                ((None, swiftclient.client.get_object, \u0027container\u0027, \u0027object\u0027),\n                 {\u0027headers\u0027: {},\n                  \u0027query_string\u0027: None,\n                  \u0027resp_chunk_size\u0027: 65536,\n                  \u0027response_dict\u0027: {}}),],\n                retry_calls)\n            self.assertIn(\u0027Error downloading object \\\u0027container/object\\\u0027: \u0027\n                          \u0027test_exc\u0027,\n                          str(output.err))\n            self.assertIn(\u0027someTransId\u0027, str(output.err))\n            ```","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"72e0ed600b5fc9c914b75738670449142e3a450c","unresolved":false,"context_lines":[{"line_number":764,"context_line":""},{"line_number":765,"context_line":"            body \u003d mock.MagicMock()"},{"line_number":766,"context_line":"            body.resp.read.side_effect \u003d RequestException(\u0027test_exc\u0027)"},{"line_number":767,"context_line":"            body.__iter__ \u003d fakeIter"},{"line_number":768,"context_line":"            return (CaseInsensitiveDict({"},{"line_number":769,"context_line":"                \u0027content-type\u0027: \u0027text/plain\u0027,"},{"line_number":770,"context_line":"                \u0027etag\u0027: \u00272cbbfe139a744d6abbe695e17f3c1991\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"795c911e_332563f6","line":767,"in_reply_to":"652ff1e9_19c60ebc","updated":"2024-02-28 16:34:57.000000000","message":"Done","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bfed03ee3ee6f0cebdb43066ebf0839e240b5d7b","unresolved":true,"context_lines":[{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def test_download_clientexception(self):"},{"line_number":756,"context_line":"        def fakeIter(arg):"},{"line_number":757,"context_line":"            raise RequestException(\u0027test_exc\u0027)"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"        retry_calls \u003d []"},{"line_number":760,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32f140_b7b647bc","line":757,"updated":"2024-03-01 11:07:58.000000000","message":"the changes in patchset 6 have been reverted - I guess you worked from your local branch after I had pushed patchset 6 to gerrit :/","commit_id":"c06b3985bf5ff3c690a3586e03d488bde9e3288c"},{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"51f650e0c80d0a35442fc1d22b1c357f509bd1cf","unresolved":false,"context_lines":[{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def test_download_clientexception(self):"},{"line_number":756,"context_line":"        def fakeIter(arg):"},{"line_number":757,"context_line":"            raise RequestException(\u0027test_exc\u0027)"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"        retry_calls \u003d []"},{"line_number":760,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"d215bc0d_57ad3209","line":757,"in_reply_to":"fa32f140_b7b647bc","updated":"2024-03-01 16:17:38.000000000","message":"Acknowledged","commit_id":"c06b3985bf5ff3c690a3586e03d488bde9e3288c"}],"test/unit/test_swiftclient.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cb3bc7591a9cb9e772711cae3d9e8fa511918ffa","unresolved":true,"context_lines":[{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":"        self.assertEqual(\u0027t\\xe9st\u0027, headers.get(\u0027X-UTF-8-HEADER\u0027, \u0027\u0027))"},{"line_number":1126,"context_line":"        self.assertEqual(\u0027%ff\u0027, headers.get(\u0027X-NON-UTF-8-HEADER\u0027, \u0027\u0027))"},{"line_number":1127,"context_line":"        self.assertEqual(\u0027%FF\u0027, headers.get(\u0027X-BINARY-HEADER\u0027, \u0027\u0027))"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":"    def test_chunk_size_read_method(self):"},{"line_number":1130,"context_line":"        conn \u003d c.Connection(\u0027http://auth.url/\u0027, \u0027some_user\u0027, \u0027some_key\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"86bfe703_9db89d07","line":1127,"updated":"2024-02-16 10:36:56.000000000","message":"ok, so this is verifying that the response headers dict is now case insensitive","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"72e0ed600b5fc9c914b75738670449142e3a450c","unresolved":false,"context_lines":[{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":"        self.assertEqual(\u0027t\\xe9st\u0027, headers.get(\u0027X-UTF-8-HEADER\u0027, \u0027\u0027))"},{"line_number":1126,"context_line":"        self.assertEqual(\u0027%ff\u0027, headers.get(\u0027X-NON-UTF-8-HEADER\u0027, \u0027\u0027))"},{"line_number":1127,"context_line":"        self.assertEqual(\u0027%FF\u0027, headers.get(\u0027X-BINARY-HEADER\u0027, \u0027\u0027))"},{"line_number":1128,"context_line":""},{"line_number":1129,"context_line":"    def test_chunk_size_read_method(self):"},{"line_number":1130,"context_line":"        conn \u003d c.Connection(\u0027http://auth.url/\u0027, \u0027some_user\u0027, \u0027some_key\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5cc661ba_b776d043","line":1127,"in_reply_to":"86bfe703_9db89d07","updated":"2024-02-28 16:34:57.000000000","message":"Acknowledged","commit_id":"59044162aac025538a5e6165fa705b629a7092c6"}]}