)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"f78d7ffa439a6aea95e54bb8e1bb379f41fc5ced","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     FatemaKhalid \u003cfatemakhalid96@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-09-18 03:01:30 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enabling direct_client users to overwrite X-Timestamp"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Requests that need an X-Timestamp shall pass it in"},{"line_number":10,"context_line":"the header of the request to overwrite it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_88d60b27","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":53},"updated":"2018-09-18 07:08:56.000000000","message":"It looks like this is not correct. X-Timestamp can be overwritten already but it could be ensured if direct_client user missing to add it.","commit_id":"05994d0b8796be40361af2d7cf79559fd01f5aaf"}],"swift/common/direct_client.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"f78d7ffa439a6aea95e54bb8e1bb379f41fc5ced","unresolved":false,"context_lines":[{"line_number":173,"context_line":""},{"line_number":174,"context_line":"def gen_headers(hdrs_in\u003dNone, add_ts\u003dFalse, add_user_agent\u003dTrue):"},{"line_number":175,"context_line":"    \"\"\""},{"line_number":176,"context_line":"    Get the headers ready for a request. All requests should have a User-Agent"},{"line_number":177,"context_line":"    string, but if one is passed in don\u0027t over-write it. Not all requests will"},{"line_number":178,"context_line":"    need an X-Timestamp, but if one is passed in do not over-write it."},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    :param headers: dict or None, base for HTTP headers"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_a8442749","line":177,"range":{"start_line":176,"start_character":41,"end_line":177,"end_character":57},"updated":"2018-09-18 07:08:56.000000000","message":"It looks like those docs lines are from the related bug report[1] but the docs is inconsistent with the change. Because this gen_headers over-write anyway if add_user_agent\u003dTrue so if we could employ whole things reported in the link, we should change the related behavior at once, IMO.\n\n1: https://bugs.launchpad.net/swift/+bug/1757250","commit_id":"05994d0b8796be40361af2d7cf79559fd01f5aaf"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"f78d7ffa439a6aea95e54bb8e1bb379f41fc5ced","unresolved":false,"context_lines":[{"line_number":178,"context_line":"    need an X-Timestamp, but if one is passed in do not over-write it."},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    :param headers: dict or None, base for HTTP headers"},{"line_number":181,"context_line":"    :param ensure_ts: boolean, should be True for any \"unsafe\" HTTP request"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    :returns: HeaderKeyDict based on headers and ready for the request"},{"line_number":184,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_82ffc900","line":181,"range":{"start_line":181,"start_character":10,"end_line":181,"end_character":21},"updated":"2018-09-18 07:08:56.000000000","message":"It\u0027s not `eunsure_ts` but `add_ts`, isn\u0027t it?","commit_id":"05994d0b8796be40361af2d7cf79559fd01f5aaf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"875bbb9ee222756973ca9769180d3dceaa58f223","unresolved":false,"context_lines":[{"line_number":183,"context_line":"    :returns: HeaderKeyDict based on headers and ready for the request"},{"line_number":184,"context_line":"    \"\"\""},{"line_number":185,"context_line":"    hdrs_out \u003d HeaderKeyDict(hdrs_in) if hdrs_in else HeaderKeyDict()"},{"line_number":186,"context_line":"    if add_ts and \u0027X-Timestamp\u0027 not in hdrs_out:"},{"line_number":187,"context_line":"        hdrs_out[\u0027X-Timestamp\u0027] \u003d Timestamp.now().internal"},{"line_number":188,"context_line":"    if add_user_agent:"},{"line_number":189,"context_line":"        hdrs_out[\u0027User-Agent\u0027] \u003d \u0027direct-client %s\u0027 % os.getpid()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_1e222655","line":186,"range":{"start_line":186,"start_character":7,"end_line":186,"end_character":13},"updated":"2018-09-18 17:30:49.000000000","message":"Looks like we can divide callers into three cases:\n\n1. add_ts is always False\n- X-Timestamp would only be present if the caller provided it\n\n2. add_ts is always True\n- X-Timestamp would always be set to the current time, regardless of whether the caller set it\n\n3. add_ts is set to (essentially) `\u0027X-Timestamp\u0027 not in hdrs_out`\n- X-Timestamp would always be present; iff the caller didn\u0027t provide it, it would be set to current time\n\nThe change as written only affects case 2 and makes it behave like case 3; this affects:\n\n- direct_delete_account\n- direct_post_object\n\nI wonder if we could get rid of the add_ts arg entirely, though, and move case 1 to behave like case 3, too. This would affect:\n\n- _get_direct_account_container\n- direct_head_container\n- direct_head_object\n- direct_get_object\n- direct_get_suffix_hashes\n\nThe only reason not to would be to allow callers explicitly *not* to send an X-Timestamp, but I don\u0027t think it\u0027s particularly *bad* if we send one. If we really want to support that use case, we could always change http_connect_raw over in bufferedhttp.py to do something like\n\n    if headers:\n        for header, value in headers.items():\n            if value is not None:\n                conn.putheader(header, str(value))\n\nand require that callers explicitly set X-Timestamp to None to not send it. (In fact, that None-checking should probably be done anyway...)","commit_id":"05994d0b8796be40361af2d7cf79559fd01f5aaf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0b896d547564d5e7eb1b777dd68f8dc495c5978d","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    return resp_headers, json.loads(resp.read().decode(\u0027ascii\u0027))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"def gen_headers(hdrs_in\u003dNone, add_ts\u003dTrue):"},{"line_number":181,"context_line":"    \"\"\""},{"line_number":182,"context_line":"    Get the headers ready for a request. All requests should have a User-Agent"},{"line_number":183,"context_line":"    string, but if one is passed in don\u0027t over-write it. Not all requests will"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_9fc0947e","line":180,"range":{"start_line":180,"start_character":30,"end_line":180,"end_character":41},"updated":"2018-09-28 20:47:01.000000000","message":"Since we\u0027ve changed the default anyway... are there any callers that *don\u0027t* trigger the \"add x-timestamp if not present\" behavior? I think everything does one of:\n\n- not providing any value for add_ts,\n- always passing add_ts\u003dTrue, or\n- passing add_ts\u003d(\u0027x-timestamp\u0027 not in lower_headers) or similar\n\nwhich all have the same effect now, yeah? Should we just get rid of the arg, and make our signature\n\n def gen_headers(hdrs_in\u003dNone):\n\n?","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cec9d764dcabe4ad94b676059da671a5768727ae","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    return resp_headers, json.loads(resp.read().decode(\u0027ascii\u0027))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"def gen_headers(hdrs_in\u003dNone, add_ts\u003dTrue):"},{"line_number":181,"context_line":"    \"\"\""},{"line_number":182,"context_line":"    Get the headers ready for a request. All requests should have a User-Agent"},{"line_number":183,"context_line":"    string, but if one is passed in don\u0027t over-write it. Not all requests will"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_f528c7ee","line":180,"range":{"start_line":180,"start_character":30,"end_line":180,"end_character":41},"in_reply_to":"3f79a3b5_5af44a55","updated":"2018-09-28 23:29:29.000000000","message":"Fortunately, I\u0027m pretty sure the only docs we have for direct_client are at https://docs.openstack.org/swift/latest/misc.html#module-swift.common.direct_client and that\u0027s generated from the docstrings in this file. (And you can see your live edits by clicking through the openstack-tox-docs link that Zuul drops, then searching for \"Direct Client\" -- you can see patchset 5\u0027s generated docs at http://logs.openstack.org/61/603261/5/check/openstack-tox-docs/dc1b00b/html/misc.html#swift.common.direct_client.gen_headers for example.)\n\nWhich is to say, I\u0027m not too worried about the documentation bits. I guess we could add a couple lines like\n\n\u003e If no ``X-Timestamp`` was provided, the current time will be filled in. If no ``User-Agent`` was provided, it will be filled in as ``direct-client \u003ccurrent process ID\u003e``.","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"},{"author":{"_account_id":28028,"name":"Fatema Khalid Sherif","email":"fatemakhalid96@gmail.com","username":"FatemaKhalid"},"change_message_id":"aac93f02e417d9bd95d7236ec5c07fa5598c20de","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    return resp_headers, json.loads(resp.read().decode(\u0027ascii\u0027))"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"def gen_headers(hdrs_in\u003dNone, add_ts\u003dTrue):"},{"line_number":181,"context_line":"    \"\"\""},{"line_number":182,"context_line":"    Get the headers ready for a request. All requests should have a User-Agent"},{"line_number":183,"context_line":"    string, but if one is passed in don\u0027t over-write it. Not all requests will"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_5af44a55","line":180,"range":{"start_line":180,"start_character":30,"end_line":180,"end_character":41},"in_reply_to":"3f79a3b5_9fc0947e","updated":"2018-09-28 22:52:47.000000000","message":"I think if any callers have, it should have failed. I guess any other cases need more experience with the code base.\n\nMy main concern is if this change in the defaults reflected in the documentation correctly.","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"}],"test/unit/common/test_direct_client.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6acc9cd690e030d85efce1b01a980cc7d05dff40","unresolved":false,"context_lines":[{"line_number":616,"context_line":""},{"line_number":617,"context_line":"        self.assertEqual(conn.req_headers[\u0027user-agent\u0027], self.user_agent)"},{"line_number":618,"context_line":"        self.assertEqual(\u0027bar\u0027, conn.req_headers.get(\u0027x-foo\u0027))"},{"line_number":619,"context_line":"        self.assertEqual(\u0027x-timestamp\u0027, conn.req_headers)"},{"line_number":620,"context_line":"        self.assertEqual(headers, resp)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    def test_direct_head_object_error(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_3cad3c32","line":619,"range":{"start_line":619,"start_character":13,"end_line":619,"end_character":24},"updated":"2018-09-24 18:09:00.000000000","message":"I think we want self.assertIn(...)","commit_id":"c562164658952ef55647c087b81bf63eafa28a78"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0b896d547564d5e7eb1b777dd68f8dc495c5978d","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        headers \u003d direct_client.gen_headers(hdrs_in\u003d{\u0027x-timestamp\u0027: \u002715\u0027})"},{"line_number":147,"context_line":"        self.assertEqual(headers[\u0027x-timestamp\u0027], \u002715\u0027)"},{"line_number":148,"context_line":"        self.assertEqual(2, len(headers))"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        headers \u003d direct_client.gen_headers(hdrs_in\u003d{\u0027foo-bar\u0027: \u002763\u0027})"},{"line_number":151,"context_line":"        self.assertEqual(headers[\u0027user-agent\u0027], stub_user_agent)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_1f87a463","line":148,"updated":"2018-09-28 20:47:01.000000000","message":"It\u0027s a little weird for us to assert that there are two headers but only look at the value of one of them... should we add a\n\n self.assertEqual(headers[\u0027user-agent\u0027], stub_user_agent)\n\n?","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"},{"author":{"_account_id":28028,"name":"Fatema Khalid Sherif","email":"fatemakhalid96@gmail.com","username":"FatemaKhalid"},"change_message_id":"aac93f02e417d9bd95d7236ec5c07fa5598c20de","unresolved":false,"context_lines":[{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        headers \u003d direct_client.gen_headers(hdrs_in\u003d{\u0027x-timestamp\u0027: \u002715\u0027})"},{"line_number":147,"context_line":"        self.assertEqual(headers[\u0027x-timestamp\u0027], \u002715\u0027)"},{"line_number":148,"context_line":"        self.assertEqual(2, len(headers))"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        headers \u003d direct_client.gen_headers(hdrs_in\u003d{\u0027foo-bar\u0027: \u002763\u0027})"},{"line_number":151,"context_line":"        self.assertEqual(headers[\u0027user-agent\u0027], stub_user_agent)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_9a83c2a9","line":148,"in_reply_to":"3f79a3b5_1f87a463","updated":"2018-09-28 22:52:47.000000000","message":"This needs to be fixed.","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0b896d547564d5e7eb1b777dd68f8dc495c5978d","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        headers \u003d direct_client.gen_headers(hdrs_in\u003d{\u0027foo-bar\u0027: \u002763\u0027})"},{"line_number":151,"context_line":"        self.assertEqual(headers[\u0027user-agent\u0027], stub_user_agent)"},{"line_number":152,"context_line":"        self.assertEqual(headers[\u0027foo-bar\u0027], \u002763\u0027)"},{"line_number":153,"context_line":"        self.assertTrue(now - 1 \u003c Timestamp(headers[\u0027x-timestamp\u0027]) \u003c now + 1)"},{"line_number":154,"context_line":"        self.assertEqual(3, len(headers))"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        hdrs_in \u003d {\u0027foo-bar\u0027: \u002755\u0027}"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_1f9ec4b7","line":153,"range":{"start_line":153,"start_character":24,"end_line":153,"end_character":77},"updated":"2018-09-28 20:47:01.000000000","message":"I know this has precedent up on line 141, but we should think about mocking out time.time (or even just Timestamp.now). I\u0027m fairly certain this is one of the flaky tests we\u0027ve seen in the gate :-/","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cec9d764dcabe4ad94b676059da671a5768727ae","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        headers \u003d direct_client.gen_headers(hdrs_in\u003d{\u0027foo-bar\u0027: \u002763\u0027})"},{"line_number":151,"context_line":"        self.assertEqual(headers[\u0027user-agent\u0027], stub_user_agent)"},{"line_number":152,"context_line":"        self.assertEqual(headers[\u0027foo-bar\u0027], \u002763\u0027)"},{"line_number":153,"context_line":"        self.assertTrue(now - 1 \u003c Timestamp(headers[\u0027x-timestamp\u0027]) \u003c now + 1)"},{"line_number":154,"context_line":"        self.assertEqual(3, len(headers))"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        hdrs_in \u003d {\u0027foo-bar\u0027: \u002755\u0027}"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_55465ba9","line":153,"range":{"start_line":153,"start_character":24,"end_line":153,"end_character":77},"in_reply_to":"3f79a3b5_156303b3","updated":"2018-09-28 23:29:29.000000000","message":"It\u0027s making sure that the timestamp header round-trips properly (I think). Mainly it checks that the timestamp has exactly 5 places after the decimal. I wouldn\u0027t worry about checking it more than once.","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"},{"author":{"_account_id":28028,"name":"Fatema Khalid Sherif","email":"fatemakhalid96@gmail.com","username":"FatemaKhalid"},"change_message_id":"aac93f02e417d9bd95d7236ec5c07fa5598c20de","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        headers \u003d direct_client.gen_headers(hdrs_in\u003d{\u0027foo-bar\u0027: \u002763\u0027})"},{"line_number":151,"context_line":"        self.assertEqual(headers[\u0027user-agent\u0027], stub_user_agent)"},{"line_number":152,"context_line":"        self.assertEqual(headers[\u0027foo-bar\u0027], \u002763\u0027)"},{"line_number":153,"context_line":"        self.assertTrue(now - 1 \u003c Timestamp(headers[\u0027x-timestamp\u0027]) \u003c now + 1)"},{"line_number":154,"context_line":"        self.assertEqual(3, len(headers))"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        hdrs_in \u003d {\u0027foo-bar\u0027: \u002755\u0027}"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_156303b3","line":153,"range":{"start_line":153,"start_character":24,"end_line":153,"end_character":77},"in_reply_to":"3f79a3b5_1f9ec4b7","updated":"2018-09-28 22:52:47.000000000","message":"I didn\u0027t fully understand the reason for the line:\n\"self.assertEqual(headers[\u0027x-timestamp\u0027],                   Timestamp(headers[\u0027x-timestamp\u0027]).internal)\" after the assertTrue check.","commit_id":"1c56238d889ea2f15d4aedc046e346f01c802c01"}]}