)]}'
{"swift/obj/server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c5b632a5424b6bdb0661ac678400529a5661e8bc","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        try:"},{"line_number":433,"context_line":"            footer_body \u003d \u0027\u0027.join(iter(timeout_reader, \u0027\u0027))"},{"line_number":434,"context_line":"        except ValueError:"},{"line_number":435,"context_line":"            raise HTTPClientDisconnect()"},{"line_number":436,"context_line":"        except ChunkReadTimeout:"},{"line_number":437,"context_line":"            raise HTTPRequestTimeout()"},{"line_number":438,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_08cce168","line":435,"updated":"2015-09-16 04:28:41.000000000","message":"this one *almost* makes sense - except you might as well go ahead and do it in _make_timeout_reader right next to the call to read so we know for sure we\u0027re only wrapping code outside of swift in such a big exception catch.\n\nThis is probably about all we need, until we up-rev eventlet:\n\n    diff --git a/swift/obj/server.py b/swift/obj/server.py\n    index a213fd2..2f2ba81 100644\n    --- a/swift/obj/server.py\n    +++ b/swift/obj/server.py\n    @@ -394,7 +394,14 @@ class ObjectController(BaseStorageServer):\n         def _make_timeout_reader(self, file_like):\n             def timeout_reader():\n                 with ChunkReadTimeout(self.client_timeout):\n    -                return file_like.read(self.network_chunk_size)\n    +                try:\n    +                    return file_like.read(self.network_chunk_size)\n    +                except ValueError:\n    +                    # eventlet will raise ValueError for chunked transfers\n    +                    # when the connection is closed.\n    +                    # NOTE: when we next up-rev eventlent we can fix this to\n    +                    # use ChunkReadError\n    +                    raise HTTPClientDisconnect()\n             return timeout_reader","commit_id":"eb0a3bbdcf99f5a707bc9aed012fb36d7a46340d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c5b632a5424b6bdb0661ac678400529a5661e8bc","unresolved":false,"context_lines":[{"line_number":619,"context_line":"                                request.environ[\u0027wsgi.input\u0027],"},{"line_number":620,"context_line":"                                mime_boundary, self.network_chunk_size)"},{"line_number":621,"context_line":"                            _junk_hdrs, obj_input \u003d next(mime_documents_iter)"},{"line_number":622,"context_line":"                    except ValueError:"},{"line_number":623,"context_line":"                        return HTTPClientDisconnect(request\u003drequest)"},{"line_number":624,"context_line":"                    except ChunkReadTimeout:"},{"line_number":625,"context_line":"                        return HTTPRequestTimeout(request\u003drequest)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_c8aee984","line":622,"updated":"2015-09-16 04:28:41.000000000","message":"I can not responsibly get behind turning a ValueError out of next(mime_docuemtns_iter) into a ClientDisconnect.\n\nHave you looked at that code - it\u0027s totally going to raise a ValueError next time someone sneezes at it.","commit_id":"eb0a3bbdcf99f5a707bc9aed012fb36d7a46340d"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c5b632a5424b6bdb0661ac678400529a5661e8bc","unresolved":false,"context_lines":[{"line_number":5331,"context_line":"        headers \u003d HeaderKeyDict(resp.getheaders())"},{"line_number":5332,"context_line":"        self.assertEqual(headers[\u0027X-Obj-Multiphase-Commit\u0027], \u0027yes\u0027)"},{"line_number":5333,"context_line":""},{"line_number":5334,"context_line":"        # simulate client disconnect"},{"line_number":5335,"context_line":"        conn.send(\"\\r\\n\")"},{"line_number":5336,"context_line":"        resp \u003d conn.getexpect()"},{"line_number":5337,"context_line":"        self.assertEqual(resp.status, 499)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba15a1d1_48ac19a0","line":5334,"updated":"2015-09-16 04:28:41.000000000","message":"I think since we have a real socket we can do a real disconnect and then validate the log lines:\n\nhttps://gist.github.com/clayg/b6629ae6286c0e9c1d3d","commit_id":"eb0a3bbdcf99f5a707bc9aed012fb36d7a46340d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"9b66d9526360b8e14f21efe5c54ce06053b300c2","unresolved":false,"context_lines":[{"line_number":5489,"context_line":"            }"},{"line_number":5490,"context_line":""},{"line_number":5491,"context_line":"        # give the object server a few trampolines to recognize request"},{"line_number":5492,"context_line":"        # has finished, or socket has closed or whatever"},{"line_number":5493,"context_line":"        sleep(0.1)"},{"line_number":5494,"context_line":""},{"line_number":5495,"context_line":"    def test_multiphase_put_client_disconnect_right_before_commit(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_869fdd5e","line":5492,"updated":"2015-09-25 02:09:16.000000000","message":"\"# give the object server a few trampolines...\"\n\nThe loop was removed, so now were are only giving only one, so the comment needs to be updated.","commit_id":"696186c680540606a8cea6495b39e23085b6863f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fe647101f339b8461abb0c1ba3191eaf2c388726","unresolved":false,"context_lines":[{"line_number":5489,"context_line":"            }"},{"line_number":5490,"context_line":""},{"line_number":5491,"context_line":"        # give the object server a few trampolines to recognize request"},{"line_number":5492,"context_line":"        # has finished, or socket has closed or whatever"},{"line_number":5493,"context_line":"        sleep(0.1)"},{"line_number":5494,"context_line":""},{"line_number":5495,"context_line":"    def test_multiphase_put_client_disconnect_right_before_commit(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_2714af42","line":5492,"in_reply_to":"9a1a9d01_869fdd5e","updated":"2015-09-25 18:23:23.000000000","message":"well the idea is the same - just sleep for a bit to allow a few trampolines in the hub - the count seemed to with how many reads needed to happen - so giving it a fixed time seemed slightly better than a large N","commit_id":"696186c680540606a8cea6495b39e23085b6863f"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"cd132861a8398b774e0208a015e0ad6fac24b47b","unresolved":false,"context_lines":[{"line_number":5685,"context_line":"                footer_json,"},{"line_number":5686,"context_line":"            ))"},{"line_number":5687,"context_line":""},{"line_number":5688,"context_line":"            # but don\u0027t send final boundry or last chunk"},{"line_number":5689,"context_line":"            to_send \u003d \"%x\\r\\n%s\\r\\n\" % \\"},{"line_number":5690,"context_line":"                (len(footer_doc), footer_doc)"},{"line_number":5691,"context_line":"            conn.send(to_send)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_dc77d8d2","line":5688,"updated":"2015-09-25 12:15:30.000000000","message":"(Nits) boundary AND chunk?","commit_id":"696186c680540606a8cea6495b39e23085b6863f"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"018d7c12a3d58071c9a645c4d927abddeaaa52fb","unresolved":false,"context_lines":[{"line_number":5685,"context_line":"                footer_json,"},{"line_number":5686,"context_line":"            ))"},{"line_number":5687,"context_line":""},{"line_number":5688,"context_line":"            # but don\u0027t send final boundry or last chunk"},{"line_number":5689,"context_line":"            to_send \u003d \"%x\\r\\n%s\\r\\n\" % \\"},{"line_number":5690,"context_line":"                (len(footer_doc), footer_doc)"},{"line_number":5691,"context_line":"            conn.send(to_send)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_8ab79785","line":5688,"in_reply_to":"9a1a9d01_6720679c","updated":"2015-09-28 11:43:50.000000000","message":"Agreed.","commit_id":"696186c680540606a8cea6495b39e23085b6863f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fe647101f339b8461abb0c1ba3191eaf2c388726","unresolved":false,"context_lines":[{"line_number":5685,"context_line":"                footer_json,"},{"line_number":5686,"context_line":"            ))"},{"line_number":5687,"context_line":""},{"line_number":5688,"context_line":"            # but don\u0027t send final boundry or last chunk"},{"line_number":5689,"context_line":"            to_send \u003d \"%x\\r\\n%s\\r\\n\" % \\"},{"line_number":5690,"context_line":"                (len(footer_doc), footer_doc)"},{"line_number":5691,"context_line":"            conn.send(to_send)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_6720679c","line":5688,"in_reply_to":"9a1a9d01_dc77d8d2","updated":"2015-09-25 18:23:23.000000000","message":"i guess it should be \"nor\"","commit_id":"696186c680540606a8cea6495b39e23085b6863f"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"cd132861a8398b774e0208a015e0ad6fac24b47b","unresolved":false,"context_lines":[{"line_number":5907,"context_line":"        self.assertEqual(\"%s.durable\" % put_timestamp.internal,"},{"line_number":5908,"context_line":"                         os.path.basename(durable_file))"},{"line_number":5909,"context_line":"        # but no continer update"},{"line_number":5910,"context_line":"        self.assertFalse(context[\u0027mock_container_update\u0027].called)"},{"line_number":5911,"context_line":""},{"line_number":5912,"context_line":""},{"line_number":5913,"context_line":"@patch_policies"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_bf69b68d","line":5910,"updated":"2015-09-25 12:15:30.000000000","message":"Wait, is this acceptable?\n\nIn fact, I\u0027m not sure a bit but when I was trying to figure out the object-server behavior, I guess object-server must call container update when the durable file landed.\n\nImagine this state from proxy-server, it will be network timeout so proxy-server regard it as failed connection but actually durable file landed.\n\nIf there are no enough good responses from object-servers, proxy will return 503 and the state might be indeterminate. However, if object-server is able to notice the failed state, object-server should not make a commit for that, IMHO.\n\nOr just suppressing the error during draining might be enough because if .durable already landed, it obviously succeeded even if the chunk draining causes an error. Perhaps what we need is just logging a warn for that and returning 201 Created.","commit_id":"696186c680540606a8cea6495b39e23085b6863f"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"018d7c12a3d58071c9a645c4d927abddeaaa52fb","unresolved":false,"context_lines":[{"line_number":5907,"context_line":"        self.assertEqual(\"%s.durable\" % put_timestamp.internal,"},{"line_number":5908,"context_line":"                         os.path.basename(durable_file))"},{"line_number":5909,"context_line":"        # but no continer update"},{"line_number":5910,"context_line":"        self.assertFalse(context[\u0027mock_container_update\u0027].called)"},{"line_number":5911,"context_line":""},{"line_number":5912,"context_line":""},{"line_number":5913,"context_line":"@patch_policies"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_dd6ec4c8","line":5910,"in_reply_to":"9a1a9d01_6726276a","updated":"2015-09-28 11:43:50.000000000","message":"I guess, this one is not a regression so we should discuss/consider as another one. That is because we had a same chance to get this situation with ChunkReadTimeout during draining. Okay let\u0027s land this as it is and I\u0027ll fix it in another.","commit_id":"696186c680540606a8cea6495b39e23085b6863f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fe647101f339b8461abb0c1ba3191eaf2c388726","unresolved":false,"context_lines":[{"line_number":5907,"context_line":"        self.assertEqual(\"%s.durable\" % put_timestamp.internal,"},{"line_number":5908,"context_line":"                         os.path.basename(durable_file))"},{"line_number":5909,"context_line":"        # but no continer update"},{"line_number":5910,"context_line":"        self.assertFalse(context[\u0027mock_container_update\u0027].called)"},{"line_number":5911,"context_line":""},{"line_number":5912,"context_line":""},{"line_number":5913,"context_line":"@patch_policies"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a1a9d01_6726276a","line":5910,"in_reply_to":"9a1a9d01_bf69b68d","updated":"2015-09-25 18:23:23.000000000","message":"yeah this highlights a pretty crappy behavior i \u0027spose.  I thought about looking into it but I was to tired.\n\nIt\u0027s sort of a BS test since the test \"client\" is making a bogus request that the proxy would never send - but the idea is to ensure we have a good backwards behavior to allow for a rolling upgrade.  I think a better test would just send the entire commit doc, but close the socket before sending \u00270\\r\\n\\r\\n\u0027\n\nI\u0027d need to know the behavior on master w/o the ChunkReadError to understand the impact - if it pre-existed we should open a bug and triage it separately.  If it was a regression with this change we might consider doing something else?\n\nFWIW I think the better behavior would be to not write the .durable *nor* send the container update.  But we definitely should not do one with-out the other.  Might be as simple as moving the call to commit to after the drain.","commit_id":"696186c680540606a8cea6495b39e23085b6863f"}]}