)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8706bbab37314b39f42af882bff417b0d65dc45a","unresolved":true,"context_lines":[{"line_number":14,"context_line":"long period of time. To improve this, this patch enable the"},{"line_number":15,"context_line":"configurable yield parameter \u0027cooperative_period\u0027 into object"},{"line_number":16,"context_line":"server controller write path."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Co-Authored-By: Clay Gerrard \u003cclay.gerrard@gmail.com\u003e"},{"line_number":19,"context_line":"Change-Id: I1c0aba9830433f093d024b4c39cd3a3b2f0d69f1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"41574ec5_528f03f9","line":17,"updated":"2024-11-29 13:58:47.000000000","message":"I think it would be worth adding\n\n```\nRelated-Change: I80b04bad0601b6cd6caef35498f89d4ba70a4fd4\n```","commit_id":"fdc5f8dd56cc28ccea6303bf2606a9f314ee9647"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"71bc6e83881127d72fa6dbb0c2d9f4084fbe239f","unresolved":false,"context_lines":[{"line_number":14,"context_line":"long period of time. To improve this, this patch enable the"},{"line_number":15,"context_line":"configurable yield parameter \u0027cooperative_period\u0027 into object"},{"line_number":16,"context_line":"server controller write path."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Co-Authored-By: Clay Gerrard \u003cclay.gerrard@gmail.com\u003e"},{"line_number":19,"context_line":"Change-Id: I1c0aba9830433f093d024b4c39cd3a3b2f0d69f1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf32e3c5_127567cb","line":17,"in_reply_to":"41574ec5_528f03f9","updated":"2024-11-29 13:59:48.000000000","message":"Done","commit_id":"fdc5f8dd56cc28ccea6303bf2606a9f314ee9647"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8706bbab37314b39f42af882bff417b0d65dc45a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"16e001ee_dab7c238","updated":"2024-11-29 13:58:47.000000000","message":"LGTM and is consistent with the change made in the GET path.\n\nI functionally tested this by making concurrent curl PUT requests direct to an object-server on a vsaio.\n\nThe first request uploads a 4GB object - this actually results in a 500 after \u003e10s because the disk fills up:\n\n```\nvagrant@vagrant:~$ time curl 127.0.0.3:6030/sdb3/238/AUTH_test/test/4GB  -T 4GB  -H \u0027Content-Type: plain/text\u0027 -H \u0027X-Timestamp: 1732882452\u0027 -is\nHTTP/1.1 100 Continue\n\nHTTP/1.1 500 Internal Error\nContent-Type: text/html; charset\u003dUTF-8\nContent-Length: 624\nDate: Fri, 29 Nov 2024 12:56:33 GMT\n\nTraceback (most recent call last):\n  File \"/vagrant/swift/swift/obj/server.py\", line 1363, in __call__\n    res \u003d getattr(self, req.method)(req)\n  File \"/vagrant/swift/swift/common/utils/__init__.py\", line 870, in _timing_stats\n    resp \u003d func(ctrl, *args, **kwargs)\n  File \"/vagrant/swift/swift/obj/server.py\", line 1057, in PUT\n    upload_size, etag \u003d self._stage_obj_data(\n  File \"/vagrant/swift/swift/obj/server.py\", line 922, in _stage_obj_data\n    writer.write(chunk)\n  File \"/vagrant/swift/swift/obj/diskfile.py\", line 1925, in write\n    written \u003d os.write(self._fd, chunk)\nOSError: [Errno 28] No space left on device\n\nreal\t0m14.455s\nuser\t0m0.072s\nsys\t0m1.245s\n```\n\nThe second request uploads a 0B object.\n\nWith ``cooperative_period \u003d 0`` this concurrent request is slow, but not completely blocked:\n\n```\ntime curl 127.0.0.3:6030/sdb3/238/AUTH_test/test/0B  -T 0B  -H \u0027Content-Type: plain/text\u0027 -is -H \u0027X-Timestamp: 1732882466\u0027\nHTTP/1.1 201 Created\nContent-Type: text/html; charset\u003dUTF-8\nEtag: \"d41d8cd98f00b204e9800998ecf8427e\"\nContent-Length: 0\nDate: Fri, 29 Nov 2024 12:53:48 GMT\n\n\nreal\t0m2.751s\nuser\t0m0.000s\nsys\t0m0.004s\n```\n\nWith ``cooperative_period \u003d 1`` it\u0027s a lot quicker:\n\n```\nvagrant@vagrant:~$ time curl 127.0.0.3:6030/sdb3/238/AUTH_test/test/0B  -T 0B  -H \u0027Content-Type: plain/text\u0027 -is -H \u0027X-Timestamp: 1732882467\u0027\nHTTP/1.1 201 Created\nContent-Type: text/html; charset\u003dUTF-8\nEtag: \"d41d8cd98f00b204e9800998ecf8427e\"\nContent-Length: 0\nDate: Fri, 29 Nov 2024 13:00:06 GMT\n\n\nreal\t0m0.022s\nuser\t0m0.004s\nsys\t0m0.000s\n```","commit_id":"fdc5f8dd56cc28ccea6303bf2606a9f314ee9647"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8706bbab37314b39f42af882bff417b0d65dc45a","unresolved":true,"context_lines":[{"line_number":4795,"context_line":"        obj_controller \u003d object_server.ObjectController("},{"line_number":4796,"context_line":"            conf, logger\u003dself.logger)"},{"line_number":4797,"context_line":""},{"line_number":4798,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":4799,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027},"},{"line_number":4800,"context_line":"                            headers\u003d{\u0027X-Timestamp\u0027: timestamp,"},{"line_number":4801,"context_line":"                                     \u0027Content-Type\u0027: \u0027application/x-test\u0027})"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a1245cb_b6b44e9c","line":4798,"updated":"2024-11-29 13:58:47.000000000","message":"nit/FYI: many tests use a timestamp iterator helper that is available here as ``self.ts`` so this could be written:\n\n```\ntimestamp \u003d next(self.ts).normal\n```\n\nIt makes little difference here but thought I\u0027d mention it for future reference. In general, it gets you a Timestamp object rather than just the string which ``normalize_timestamp`` returns, which can be more useful in some tests.","commit_id":"fdc5f8dd56cc28ccea6303bf2606a9f314ee9647"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8706bbab37314b39f42af882bff417b0d65dc45a","unresolved":true,"context_lines":[{"line_number":4792,"context_line":"        conf \u003d {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027false\u0027,"},{"line_number":4793,"context_line":"                \u0027container_update_timeout\u0027: 0.0,"},{"line_number":4794,"context_line":"                \u0027cooperative_period\u0027: \u00271\u0027}"},{"line_number":4795,"context_line":"        obj_controller \u003d object_server.ObjectController("},{"line_number":4796,"context_line":"            conf, logger\u003dself.logger)"},{"line_number":4797,"context_line":""},{"line_number":4798,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":4799,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027},"},{"line_number":4800,"context_line":"                            headers\u003d{\u0027X-Timestamp\u0027: timestamp,"},{"line_number":4801,"context_line":"                                     \u0027Content-Type\u0027: \u0027application/x-test\u0027})"},{"line_number":4802,"context_line":"        req.body \u003d b\u00277 bytes\u0027"},{"line_number":4803,"context_line":"        with mock.patch(\u0027swift.common.utils.sleep\u0027) as mock_sleep:"},{"line_number":4804,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4805,"context_line":"            self.assertEqual(resp.status_int, 201)"},{"line_number":4806,"context_line":"        self.assertEqual(1, mock_sleep.call_count)"},{"line_number":4807,"context_line":""},{"line_number":4808,"context_line":"        # Test DiskFile writer actually sleeps when writing chunks. And verify"}],"source_content_type":"text/x-python","patch_set":2,"id":"255698c6_4c0094ea","line":4805,"range":{"start_line":4795,"start_character":8,"end_line":4805,"end_character":50},"updated":"2024-11-29 13:58:47.000000000","message":"nit: these lines are repeated 5 times and could be a helper e.g.\n\n```\ndef do_call(conf):\n    \u003crepeated lines\u003e\n    return mock_sleep\n```\n\nI find it helpful to clarify what is changing between each scenario (i.e only the conf) and what is not changing, vs needing to review the repeated lines 5 times to establish that nothing about the request is changing.\n\nThe phrase \"tests should be DAMP\" is often used but that doesn\u0027t mean that tests cannot also be appropriately DRY as well. The setUp and tearDown pattern is an example. Here, adding a simple helper within the test method doesn\u0027t create any coupling with other tests and IMHO doesn\u0027t obscure any concepts.","commit_id":"fdc5f8dd56cc28ccea6303bf2606a9f314ee9647"}]}