)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19bdc0b4f27ab832cf771218d0ab4f8a8b10c5b0","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Clay Gerrard \u003cclay.gerrard@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2025-03-05 18:13:33 -0600"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"http_protocol: cleanup txn_id error logs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I0b0bd367f93d4a5529d022f251bba5b99b824a2d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"52f10f48_5542e6f2","line":7,"range":{"start_line":7,"start_character":15,"end_line":7,"end_character":22},"updated":"2025-03-06 10:07:37.000000000","message":"It took me a while to notice that there was actually a change to the logged message (no comma) rather than just a refactor. Maybe that\u0027s because I was distracted by the apparent (but not) duplication of txn_id in the message. Might be worth calling out the change in the commit message.","commit_id":"250b419dad56e597eaff05817f6545779c023e6f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b823238a9b5928a18cc42e5d839bec1e9c0d9386","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a3a15624_811cc426","updated":"2025-03-06 10:08:19.000000000","message":"PS we could land this immediately on master if rebased","commit_id":"250b419dad56e597eaff05817f6545779c023e6f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19bdc0b4f27ab832cf771218d0ab4f8a8b10c5b0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"354df28b_2ba7f311","updated":"2025-03-06 10:07:37.000000000","message":"Subtle but correct.\n\nWhat do you think about maybe switching an assertIn for an assertEqual in the tests?","commit_id":"250b419dad56e597eaff05817f6545779c023e6f"}],"swift/common/http_protocol.py":[{"author":{"_account_id":36606,"name":"Yan Xiao","display_name":"Yan","email":"yanxiao@nvidia.com","username":"yanxiao"},"change_message_id":"b58e4bd139deb669bdbea314486d650d6c967f2d","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                self.conn_state[2] \u003d wsgi.STATE_IDLE"},{"line_number":230,"context_line":"        return got"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    def _get_txn_id(self):"},{"line_number":233,"context_line":"        try:"},{"line_number":234,"context_line":"            # assume we have a LogAdapter"},{"line_number":235,"context_line":"            txn_id \u003d self.server.app.logger.txn_id  # just in case it was set"}],"source_content_type":"text/x-python","patch_set":1,"id":"add98179_b13ff57e","line":232,"updated":"2025-03-06 15:22:26.000000000","message":"+1 for refactoring this as method","commit_id":"250b419dad56e597eaff05817f6545779c023e6f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19bdc0b4f27ab832cf771218d0ab4f8a8b10c5b0","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        txn_id \u003d self._get_txn_id()"},{"line_number":272,"context_line":"        self.log_error(\"code %d, message %s (txn: %s)\","},{"line_number":273,"context_line":"                       code, message, txn_id)"},{"line_number":274,"context_line":"        self.send_response(code, message)"},{"line_number":275,"context_line":"        self.send_header(\u0027Connection\u0027, \u0027close\u0027)"},{"line_number":276,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"d1d849ad_3ebc647c","line":273,"updated":"2025-03-06 10:07:37.000000000","message":"ok, so now we always explicitly include txn_id in the message even when we\u0027ve set it in the server logger !?!\n\n...but this DOES work (i.e. does NOT duplicate txn_id) because over in logs.py we refuse to log the txn_id more than once per message https://github.com/openstack/swift/blob/b94d7960f7ac598760e19179c9ee48be2712ff87/swift/common/utils/logs.py#L385-L388\n\n...but that\u0027s really subtle and cost me some time seeing how this patch could possibly be correct!","commit_id":"250b419dad56e597eaff05817f6545779c023e6f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2d19c206eb75123f59115590dd30be0f290f17ec","unresolved":true,"context_lines":[{"line_number":239,"context_line":"        else:"},{"line_number":240,"context_line":"            if not txn_id:"},{"line_number":241,"context_line":"                # we have a LogAdapter, but it just doesn\u0027t have a txn_id yet"},{"line_number":242,"context_line":"                txn_id \u003d self.server.app.logger.txn_id \u003d generate_trans_id(\u0027\u0027)"},{"line_number":243,"context_line":"        return txn_id"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    def send_error(self, code, message\u003dNone, explain\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f33e9ee_dfb576b5","line":242,"range":{"start_line":242,"start_character":25,"end_line":242,"end_character":78},"updated":"2025-03-06 18:42:21.000000000","message":"We should have a more descriptive name than `_get_txn_id` -- this seems like a pretty notable side effect.","commit_id":"1961e21f2726cd1ccf200603cecf7194f8bbbb3a"}],"test/unit/common/test_http_protocol.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19bdc0b4f27ab832cf771218d0ab4f8a8b10c5b0","unresolved":true,"context_lines":[{"line_number":259,"context_line":"        info_lines \u003d self.logger.get_lines_for_level(\u0027info\u0027)"},{"line_number":260,"context_line":"        self.assertEqual("},{"line_number":261,"context_line":"            \"ERROR WSGI: code 400, message \""},{"line_number":262,"context_line":"            \"Bad request syntax (\u0027ONLY-METHOD\u0027), (txn: test-trans-id)\","},{"line_number":263,"context_line":"            info_lines[1])"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def test_bad_request_app_logging(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"53ecc0e4_5cdf1771","side":"PARENT","line":262,"range":{"start_line":262,"start_character":47,"end_line":262,"end_character":48},"updated":"2025-03-06 10:07:37.000000000","message":"+1 the comma was deviant from usual logging format","commit_id":"c9da7202cff70a2b3d8ad15e79dc83c76f00cb60"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"19bdc0b4f27ab832cf771218d0ab4f8a8b10c5b0","unresolved":true,"context_lines":[{"line_number":280,"context_line":"        self.assertIn(b\"X-Trans-Id: test-trans-id\", lines[6])"},{"line_number":281,"context_line":"        self.assertIn(b\"X-Openstack-Request-Id: test-trans-id\", lines[7])"},{"line_number":282,"context_line":"        self.assertEqual(1, len(app_logger.records.get(\u0027ERROR\u0027, [])))"},{"line_number":283,"context_line":"        self.assertIn("},{"line_number":284,"context_line":"            \"ERROR WSGI: code 400, message Bad request syntax (\u0027ONLY-METHOD\u0027) \""},{"line_number":285,"context_line":"            \"(txn: test-trans-id)\","},{"line_number":286,"context_line":"            app_logger.records.get(\u0027ERROR\u0027)[0])"}],"source_content_type":"text/x-python","patch_set":1,"id":"083bd44a_8be14b1d","line":283,"range":{"start_line":283,"start_character":13,"end_line":283,"end_character":21},"updated":"2025-03-06 10:07:37.000000000","message":"``assertIn`` is so unfortunate...because this assertion does NOT reassure us that the txn_id is only included *once* in the message\n\nturns out that an assertEqual could be used to prove there\u0027s only one txn_id:\n\n```\ndiff --git a/test/unit/common/test_http_protocol.py b/test/unit/common/test_http_protocol.py\nindex 3f075dcfb..a49b3f3dd 100644\n--- a/test/unit/common/test_http_protocol.py\n+++ b/test/unit/common/test_http_protocol.py\n@@ -280,9 +280,9 @@ class TestSwiftHttpProtocolSomeMore(ProtocolTest):\n         self.assertIn(b\"X-Trans-Id: test-trans-id\", lines[6])\n         self.assertIn(b\"X-Openstack-Request-Id: test-trans-id\", lines[7])\n         self.assertEqual(1, len(app_logger.records.get(\u0027ERROR\u0027, [])))\n-        self.assertIn(\n-            \"ERROR WSGI: code 400, message Bad request syntax (\u0027ONLY-METHOD\u0027) \"\n-            \"(txn: test-trans-id)\",\n+        self.assertEqual(\n+            \"test ERROR: ERROR WSGI: code 400, message Bad request syntax \"\n+            \"(\u0027ONLY-METHOD\u0027) (txn: test-trans-id)\",\n             app_logger.records.get(\u0027ERROR\u0027)[0])\n         # but we can at least assert that the logger txn_id was set\n         self.assertEqual(\u0027test-trans-id\u0027, app_logger.txn_id)\n```","commit_id":"250b419dad56e597eaff05817f6545779c023e6f"}]}