)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c6768a3126ed9b229278a4b3dc495ceed6802011","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add object-server partial write prevention tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change adds some addtional unit tests to test that partial"},{"line_number":10,"context_line":"writes are handled correctly through the object-server."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The bug intially wanted to test for partial writes when the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1ae5cdf2_4ee6164c","line":9,"updated":"2014-06-23 12:42:38.000000000","message":"addtional -\u003e additional","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac4b54848b5a1b8d81b6847643ca15ecadf12cae","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add object-server partial write prevention tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change adds some addtional unit tests to test that partial"},{"line_number":10,"context_line":"writes are handled correctly through the object-server."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The bug intially wanted to test for partial writes when the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baada198_e2271310","line":9,"in_reply_to":"1ae5cdf2_4ee6164c","updated":"2014-07-15 09:25:15.000000000","message":"Done","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c6768a3126ed9b229278a4b3dc495ceed6802011","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The bug intially wanted to test for partial writes when the"},{"line_number":13,"context_line":"object server wrapped up os.write. However, now that the writing out"},{"line_number":14,"context_line":"to disk is happening in the diskfile oject, the oject-server tests"},{"line_number":15,"context_line":"now only have to deal with partial writes correcly when the written"},{"line_number":16,"context_line":"byte counts are passed back from the diskfile object indicate a"},{"line_number":17,"context_line":"partial write."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1ae5cdf2_aef5daee","line":14,"updated":"2014-06-23 12:42:38.000000000","message":"oject-server -\u003e object-server","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac4b54848b5a1b8d81b6847643ca15ecadf12cae","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The bug intially wanted to test for partial writes when the"},{"line_number":13,"context_line":"object server wrapped up os.write. However, now that the writing out"},{"line_number":14,"context_line":"to disk is happening in the diskfile oject, the oject-server tests"},{"line_number":15,"context_line":"now only have to deal with partial writes correcly when the written"},{"line_number":16,"context_line":"byte counts are passed back from the diskfile object indicate a"},{"line_number":17,"context_line":"partial write."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baada198_221e6bbb","line":14,"in_reply_to":"1ae5cdf2_aef5daee","updated":"2014-07-15 09:25:15.000000000","message":"Done","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"4f8a4c9ad2f87b3b191edcefacb7973741a8ee3e","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This change adds some additional unit tests to test that partial"},{"line_number":10,"context_line":"writes are handled correctly through the object-server."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The bug initially wanted to test for partial writes when the"},{"line_number":13,"context_line":"object server wrapped up os.write. However, now that the writing out"},{"line_number":14,"context_line":"to disk is happening in the diskfile object, the object-server tests"},{"line_number":15,"context_line":"now only have to deal with partial writes correctly when the written"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1abeadc6_1843f76c","line":12,"updated":"2014-08-20 20:39:13.000000000","message":"I feel like I\u0027m missing something, what does \"The bug\" refer to here?","commit_id":"fe2500bc785fb30ea1d81833f092026668aa2cb1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"856843952c367100473478c9247d69e83bcbc27a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This change adds some additional unit tests to test that partial"},{"line_number":10,"context_line":"writes are handled correctly through the object-server."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The bug initially wanted to test for partial writes when the"},{"line_number":13,"context_line":"object server wrapped up os.write. However, now that the writing out"},{"line_number":14,"context_line":"to disk is happening in the diskfile object, the object-server tests"},{"line_number":15,"context_line":"now only have to deal with partial writes correctly when the written"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa98f980_cf6084a7","line":12,"in_reply_to":"1abeadc6_1843f76c","updated":"2014-09-15 03:53:18.000000000","message":"The bug linked to this change (Closes-Bug: 1040883), I\u0027ll make it more clear in the next patch set.","commit_id":"fe2500bc785fb30ea1d81833f092026668aa2cb1"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c6768a3126ed9b229278a4b3dc495ceed6802011","unresolved":false,"context_lines":[{"line_number":805,"context_line":"        content_length_written \u003d 90"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        def mock_disk_file_writer_write(*args, **kargs):"},{"line_number":808,"context_line":"            return content_length_written"},{"line_number":809,"context_line":""},{"line_number":810,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":811,"context_line":"        with mock.patch(\"swift.obj.diskfile.DiskFileWriter.write\","}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_d3fe4f39","line":808,"updated":"2014-06-23 12:42:38.000000000","message":"Looks like this is never used, it doesn\u0027t matter what is returned - could be None and the test would still pass.\n\nDiskFileWrtiter.write has only two arguments: self and chunk - you could check if this is equal to the data sent to the object server below.\n\n def mock_disk_file_writer_write(self, chunk):\n     assert \u0027VERIFY\u0027 \u003d\u003d chunk\n\n(Using self.assertEquals doesn\u0027t work here because the used class is DiskFileWriter)","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac4b54848b5a1b8d81b6847643ca15ecadf12cae","unresolved":false,"context_lines":[{"line_number":805,"context_line":"        content_length_written \u003d 90"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        def mock_disk_file_writer_write(*args, **kargs):"},{"line_number":808,"context_line":"            return content_length_written"},{"line_number":809,"context_line":""},{"line_number":810,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":811,"context_line":"        with mock.patch(\"swift.obj.diskfile.DiskFileWriter.write\","}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_e27d531d","line":808,"in_reply_to":"1ae5cdf2_d3fe4f39","updated":"2014-07-15 09:25:15.000000000","message":"According to debugging this test with the q module the function is used, and the return bytes written, the number is less then the content and file size simulating a partial write.\n\nSee my results in my comment in the next test.","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac4b54848b5a1b8d81b6847643ca15ecadf12cae","unresolved":false,"context_lines":[{"line_number":815,"context_line":"                headers\u003d{\u0027X-Timestamp\u0027: timestamp,"},{"line_number":816,"context_line":"                         \u0027Content-Length\u0027: str(content_length),"},{"line_number":817,"context_line":"                         \u0027Content-Type\u0027: \u0027application/octet-stream\u0027})"},{"line_number":818,"context_line":"            req.body \u003d \u0027VERIFY\u0027"},{"line_number":819,"context_line":"            resp \u003d req.get_response(self.object_controller)"},{"line_number":820,"context_line":"            self.assertEquals(resp.status_int, 499)"},{"line_number":821,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_82bbb7c2","line":818,"updated":"2014-07-15 09:25:15.000000000","message":"But yes, the req.body is actaully incorrect, and I have now changed it to the write size. Seeing as it is used in the chucking up loop.","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c6768a3126ed9b229278a4b3dc495ceed6802011","unresolved":false,"context_lines":[{"line_number":825,"context_line":"        content_length \u003d network_chunk_size + 6"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        def mock_disk_file_writer_write(*args, **kargs):"},{"line_number":828,"context_line":"            bytes_written.next()"},{"line_number":829,"context_line":""},{"line_number":830,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":831,"context_line":"        with mock.patch(\"swift.obj.diskfile.DiskFileWriter.write\","}],"source_content_type":"text/x-python","patch_set":1,"id":"1ae5cdf2_b346237a","line":828,"updated":"2014-06-23 12:42:38.000000000","message":"Same as in test above","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac4b54848b5a1b8d81b6847643ca15ecadf12cae","unresolved":false,"context_lines":[{"line_number":825,"context_line":"        content_length \u003d network_chunk_size + 6"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        def mock_disk_file_writer_write(*args, **kargs):"},{"line_number":828,"context_line":"            bytes_written.next()"},{"line_number":829,"context_line":""},{"line_number":830,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":831,"context_line":"        with mock.patch(\"swift.obj.diskfile.DiskFileWriter.write\","}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_a2db5b4d","line":828,"in_reply_to":"1ae5cdf2_b346237a","updated":"2014-07-15 09:25:15.000000000","message":"Actually this test is specifically simulating a partial write happening on a later chuck (so a bounds check). When using the q module to debug this one (checking the length of the send in chunk):\n\n  mock_disk_file_writer_write: len(args[1])\u003d65536\n  mock_disk_file_writer_write: len(args[1])\u003d6\n\nYou can see it\u0027s being called twice, the first one is the netwrok_chunk_size and the second is what is left. However on the second run the mocked function returns a 5 (simulating a partial write). Which then is caught in the object server and the required error 499 is returned.","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac4b54848b5a1b8d81b6847643ca15ecadf12cae","unresolved":false,"context_lines":[{"line_number":835,"context_line":"                headers\u003d{\u0027X-Timestamp\u0027: timestamp,"},{"line_number":836,"context_line":"                         \u0027Content-Length\u0027: str(content_length),"},{"line_number":837,"context_line":"                         \u0027Content-Type\u0027: \u0027application/octet-stream\u0027})"},{"line_number":838,"context_line":"            req.body \u003d \u0027VERIFY\u0027"},{"line_number":839,"context_line":"            resp \u003d req.get_response(self.object_controller)"},{"line_number":840,"context_line":"            self.assertEquals(resp.status_int, 499)"},{"line_number":841,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_e2e8f3e3","line":838,"updated":"2014-07-15 09:25:15.000000000","message":"But you are write, the body is incorrect, I have now modified it to be the write length.","commit_id":"cd81ad273119a8dbfbbb2b44105e31c702e088dd"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"199b4701f516932e8dc8c824b485e80b3c6bcdc3","unresolved":false,"context_lines":[{"line_number":825,"context_line":"        content_length \u003d network_chunk_size + 6"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        def mock_disk_file_writer_write(*args, **kargs):"},{"line_number":828,"context_line":"            bytes_written.next()"},{"line_number":829,"context_line":""},{"line_number":830,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":831,"context_line":"        with mock.patch(\"swift.obj.diskfile.DiskFileWriter.write\","}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_4c9a9fc5","line":828,"updated":"2014-08-12 19:38:54.000000000","message":"I think this should be: return bytes_written.next()\n\nAlso, do you know why this line: https://github.com/openstack/swift/blob/master/swift/obj/server.py#L432\n\nis not:\n\nupload_size +\u003d writer.write(chunk)\n\nI must be missing something...","commit_id":"1ef8c5f8a565d6e634dd30368c66c95813ddc0f9"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7cccee4b1c3d3b9ac96b899317c6b5fdcffa34d0","unresolved":false,"context_lines":[{"line_number":825,"context_line":"        content_length \u003d network_chunk_size + 6"},{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        def mock_disk_file_writer_write(*args, **kargs):"},{"line_number":828,"context_line":"            bytes_written.next()"},{"line_number":829,"context_line":""},{"line_number":830,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":831,"context_line":"        with mock.patch(\"swift.obj.diskfile.DiskFileWriter.write\","}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_9e35abf8","line":828,"in_reply_to":"baada198_4c9a9fc5","updated":"2014-08-13 01:45:05.000000000","message":"Thanks Thiago.\n\nThe writer object is a DiskFileWriter which internally manages how much of the file it has written. So the result of writer.write(chuck) is the total amount of how much its written (_upload_size) not how much of the chuck was written.\nThis confused me when I first read it too :)","commit_id":"1ef8c5f8a565d6e634dd30368c66c95813ddc0f9"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"4f8a4c9ad2f87b3b191edcefacb7973741a8ee3e","unresolved":false,"context_lines":[{"line_number":800,"context_line":"        finally:"},{"line_number":801,"context_line":"            object_server.http_connect \u003d old_http_connect"},{"line_number":802,"context_line":""},{"line_number":803,"context_line":"    def test_PUT_partial_write(self):"},{"line_number":804,"context_line":"        content_length \u003d 100"},{"line_number":805,"context_line":"        content_length_written \u003d 90"},{"line_number":806,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1abeadc6_1b34d148","line":803,"updated":"2014-08-20 20:39:13.000000000","message":"Could use a little help understanding this more if you could... couple of questions (a) it looks like you are exercising the error handling in lines 435-436 in obj/server.py but from what I can tell that code was already covered by some other test, just looking at coverage w/o your changes I didn\u0027t go track it down and - is this test really needed? (b) on the next test added, what additional coverage does it provide beyond this first one?\n\nNot trying to be a hard ass, just trying to understand where/how this fits in with existing coverage.  thanks!","commit_id":"fe2500bc785fb30ea1d81833f092026668aa2cb1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"856843952c367100473478c9247d69e83bcbc27a","unresolved":false,"context_lines":[{"line_number":800,"context_line":"        finally:"},{"line_number":801,"context_line":"            object_server.http_connect \u003d old_http_connect"},{"line_number":802,"context_line":""},{"line_number":803,"context_line":"    def test_PUT_partial_write(self):"},{"line_number":804,"context_line":"        content_length \u003d 100"},{"line_number":805,"context_line":"        content_length_written \u003d 90"},{"line_number":806,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa98f980_b2e00727","line":803,"in_reply_to":"1abeadc6_1b34d148","updated":"2014-09-15 03:53:18.000000000","message":"Lol, nah it\u0027s fine Paul, thanks for reviewing. Here are my 2 cents which I added as my own review comment to this change. \n\nTL;DR: Even though it tests the same code (exception block), it simulates a different scenario, rather then testing a closed connection, it tests a failed write.\n\n\u003csnip\u003e\nI have had a look at the test_short_body test to see how it compares with the partial write tests I\u0027ve added in this change.\n\nThe test, tests the same section of code, but does it in a very different simulation. test_short_body sends in a body of a different size then specified via the content_length. This is simulating a lost connection or a corrupted connection.\nThe partial write tests are testing the opposite side of the coin, it sends in a correct sized body so from the client side everything is OK. But simulates the diskfile.write method failing to completely write out to disk. Maybe a 499 isn\u0027t the best thing to send the user in this case, but does show that swift handles this situation.\n\nIn effect, it is testing the same code, but from two different, but I think both important sides, we want to test for an incorrect body size, but also the diskfile writer failing to write. As such it is my opinion that all 3 tests are valid.\n\u003c/snip\u003e\n\nI wrote these tests because testing the writes isn\u0027t tested (as per the bug attached to the commit message). I wanted to close the bug so added the test.. if the bug isn\u0027t relevant anymore then I am happy to abandon the change and we can simply close the bug :)","commit_id":"fe2500bc785fb30ea1d81833f092026668aa2cb1"}]}