)]}'
{"swift/common/middleware/slo.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"53ea3bff8d3982d8a6766edf649ab779483978cd","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                    req, self._response_headers),"},{"line_number":802,"context_line":"                conditional_response\u003dTrue)"},{"line_number":803,"context_line":"            resp.headers.update({"},{"line_number":804,"context_line":"                \u0027Etag\u0027: \u0027\"%s\"\u0027 % slo_etag,"},{"line_number":805,"context_line":"                \u0027Content-Length\u0027: slo_size,"},{"line_number":806,"context_line":"            })"},{"line_number":807,"context_line":"            return resp(req.environ, start_response)"},{"line_number":808,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_d90d8fe8","line":805,"range":{"start_line":804,"start_character":0,"end_line":805,"end_character":43},"updated":"2018-04-02 08:38:08.000000000","message":"it sounds like conditional_etag should be slo_etag?\nIf we updates these directly here, it causes a mis-match problem with response.__call__.\n\nI should go ahead the tests...\n\n\nEr... this means the response should response slo_etag and slo_size as correct answer, even s3api checks it with custom etag. Exactly, the behavior is same with older code but... perhaps, we should replace the etag with the conditional (i.e. the value pointed by x-backend-etag-is-at) one?","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"a5d3525bf0700b7ea938562497c4715a77315a93","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                    req, self._response_headers),"},{"line_number":802,"context_line":"                conditional_response\u003dTrue)"},{"line_number":803,"context_line":"            resp.headers.update({"},{"line_number":804,"context_line":"                \u0027Etag\u0027: \u0027\"%s\"\u0027 % slo_etag,"},{"line_number":805,"context_line":"                \u0027Content-Length\u0027: slo_size,"},{"line_number":806,"context_line":"            })"},{"line_number":807,"context_line":"            return resp(req.environ, start_response)"},{"line_number":808,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_1b6cf89a","line":805,"range":{"start_line":804,"start_character":0,"end_line":805,"end_character":43},"in_reply_to":"bf659307_c9a91a24","updated":"2018-04-06 06:25:57.000000000","message":"OK. the middleware which uses x-backend-etag-is-at should have the responsibility to return correct etag (and size) at the layer. And currently it works fine in those cases, you suggested.","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"43faf6f79bf266cf67230a1d78fc2b6638757cd9","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                    req, self._response_headers),"},{"line_number":802,"context_line":"                conditional_response\u003dTrue)"},{"line_number":803,"context_line":"            resp.headers.update({"},{"line_number":804,"context_line":"                \u0027Etag\u0027: \u0027\"%s\"\u0027 % slo_etag,"},{"line_number":805,"context_line":"                \u0027Content-Length\u0027: slo_size,"},{"line_number":806,"context_line":"            })"},{"line_number":807,"context_line":"            return resp(req.environ, start_response)"},{"line_number":808,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_c9a91a24","line":805,"range":{"start_line":804,"start_character":0,"end_line":805,"end_character":43},"in_reply_to":"bf659307_d90d8fe8","updated":"2018-04-03 20:42:08.000000000","message":"Yeah, SLO responds with its ETag, then s3api (for example) would swap it out for another. I\u0027m not sure about having the ETag in the response change flopping around like this as we work our way back through the pipeline -- I could see arguments both ways -- but this at least has some precedent in the object controller [1] and encryption [2].\n\n[1] https://github.com/openstack/swift/blob/2.17.0/swift/proxy/controllers/obj.py#L2438\n\n[2] https://github.com/openstack/swift/blob/2.17.0/swift/common/middleware/crypto/decrypter.py#L201-L207","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"53ea3bff8d3982d8a6766edf649ab779483978cd","unresolved":false,"context_lines":[{"line_number":931,"context_line":""},{"line_number":932,"context_line":"    def _manifest_head_response(self, req, response_headers):"},{"line_number":933,"context_line":"        conditional_etag \u003d resolve_etag_is_at_header(req, response_headers)"},{"line_number":934,"context_line":"        return HTTPOk(request\u003dreq, headers\u003dresponse_headers, body\u003d\u0027\u0027,"},{"line_number":935,"context_line":"                      conditional_etag\u003dconditional_etag,"},{"line_number":936,"context_line":"                      conditional_response\u003dTrue)"},{"line_number":937,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_c4712678","line":934,"range":{"start_line":934,"start_character":15,"end_line":934,"end_character":21},"updated":"2018-04-02 08:38:08.000000000","message":"Off topic:\n\nit looks weird because this is conditional response but the class is HTTPOk leading 200 OK HTTP response. Just swob.Response works fine in my local.","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"53ea3bff8d3982d8a6766edf649ab779483978cd","unresolved":false,"context_lines":[{"line_number":932,"context_line":"    def _manifest_head_response(self, req, response_headers):"},{"line_number":933,"context_line":"        conditional_etag \u003d resolve_etag_is_at_header(req, response_headers)"},{"line_number":934,"context_line":"        return HTTPOk(request\u003dreq, headers\u003dresponse_headers, body\u003d\u0027\u0027,"},{"line_number":935,"context_line":"                      conditional_etag\u003dconditional_etag,"},{"line_number":936,"context_line":"                      conditional_response\u003dTrue)"},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"    def _manifest_get_response(self, req, content_length, response_headers,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_24f34276","line":935,"updated":"2018-04-02 08:38:08.000000000","message":"OK. What this patch want to do is injecting conditional_etag in slo (the primary user will be obviously s3api). I\u0027m not sure any other custom middleware want to do so too but with this change, we will be able to decrease # of duplicated lines for each middleware to achieve the large object stuff.","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"43faf6f79bf266cf67230a1d78fc2b6638757cd9","unresolved":false,"context_lines":[{"line_number":932,"context_line":"    def _manifest_head_response(self, req, response_headers):"},{"line_number":933,"context_line":"        conditional_etag \u003d resolve_etag_is_at_header(req, response_headers)"},{"line_number":934,"context_line":"        return HTTPOk(request\u003dreq, headers\u003dresponse_headers, body\u003d\u0027\u0027,"},{"line_number":935,"context_line":"                      conditional_etag\u003dconditional_etag,"},{"line_number":936,"context_line":"                      conditional_response\u003dTrue)"},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"    def _manifest_get_response(self, req, content_length, response_headers,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_8c2a507c","line":935,"in_reply_to":"bf659307_24f34276","updated":"2018-04-03 20:42:08.000000000","message":"\u003e with this change, we will be able to decrease # of duplicated lines for each middleware to achieve the large object stuff\n\nIt\u0027s more than that -- *without* this change, If-Match/If-None-Match is broken for https://review.openstack.org/#/c/302475/ !\n\nWith an If-Match that should 200, the request makes its way down to the object server. It responds 200 on account of the X-Backend-Etag-Is-At; object controller will do something similar. But when we get *here*, we make a new Response object with conditional_response\u003dTrue and an ETag that doesn\u0027t match... so we send a 412 on down the pipeline and drop any body on the floor :-(","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"a5d3525bf0700b7ea938562497c4715a77315a93","unresolved":false,"context_lines":[{"line_number":932,"context_line":"    def _manifest_head_response(self, req, response_headers):"},{"line_number":933,"context_line":"        conditional_etag \u003d resolve_etag_is_at_header(req, response_headers)"},{"line_number":934,"context_line":"        return HTTPOk(request\u003dreq, headers\u003dresponse_headers, body\u003d\u0027\u0027,"},{"line_number":935,"context_line":"                      conditional_etag\u003dconditional_etag,"},{"line_number":936,"context_line":"                      conditional_response\u003dTrue)"},{"line_number":937,"context_line":""},{"line_number":938,"context_line":"    def _manifest_get_response(self, req, content_length, response_headers,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_bbe06cfc","line":935,"in_reply_to":"bf659307_8c2a507c","updated":"2018-04-06 06:25:57.000000000","message":"OIC. some middleware located in right of slo would set x-backend-etag-is-at, then conditional_response\u003dTrue below will cause re-check the if-match. Make sense.\n\nI\u0027d like to avoid such a duplicated conditional check but it seems to be in your refactor after this patch so we can have this patch as it is as *a fix*","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"}],"test/unit/common/middleware/test_slo.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"53ea3bff8d3982d8a6766edf649ab779483978cd","unresolved":false,"context_lines":[{"line_number":3634,"context_line":""},{"line_number":3635,"context_line":"        self.assertEqual(status, \u0027200 OK\u0027)"},{"line_number":3636,"context_line":"        self.assertIn((\u0027Content-Length\u0027, \u002750\u0027), headers)"},{"line_number":3637,"context_line":"        self.assertIn((\u0027Etag\u0027, \u0027\"%s\"\u0027 % self.slo_etag), headers)"},{"line_number":3638,"context_line":"        self.assertIn((\u0027X-Object-Sysmeta-Custom-Etag\u0027, \u0027a custom etag\u0027),"},{"line_number":3639,"context_line":"                      headers)"},{"line_number":3640,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_64061a23","line":3637,"range":{"start_line":3637,"start_character":24,"end_line":3637,"end_character":55},"updated":"2018-04-02 08:38:08.000000000","message":"so this might confuse the client, the client expects \u0027a custom etag\u0027 to be matched but the response will include slo_etag. Exactly the custom middleware would replace the etag with the if-match value tho.","commit_id":"42adbe561f8029d7d3481191925549066f6863f8"}]}