)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"59e747052026f975cea2e135e86599c5c01efb93","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c3f73230_3a837d1e","updated":"2025-10-10 19:05:49.000000000","message":"The code looks good to me, however any possibility to have a unit test for coverage purpose?","commit_id":"c4b8ab3236c459394d1c609070c0fbf61f5a8357"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"c759885e8a0d535898dc197a3b2c50402f7517b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"778bb2a6_013ac6f2","in_reply_to":"c3f73230_3a837d1e","updated":"2025-10-13 18:49:07.000000000","message":"Thank you Abhishek. Also was thinking about that.\nJust added the simple test which fails for the previous implementation with\ntesttools.matchers._impl.MismatchError: b\u0027abc\u0027 ！\u003d b\u0027ab\u0027\nbecause it tries to return more than requested","commit_id":"c4b8ab3236c459394d1c609070c0fbf61f5a8357"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"fd10ef48953723e445ee4350552bc7f4781eec2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b188102e_2ddc58ad","updated":"2025-10-14 06:18:37.000000000","message":"Looks good, thank you!!","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2e3c88b43ccdf9647f5fe4919656421b7b42d855","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7e64ff3b_ba9ae357","updated":"2025-11-06 12:16:33.000000000","message":"removing +2 so that -1 can be highlighted","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"28d423759466978ff7f637e998541b6bfa1575b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"56af374e_8acab0f8","updated":"2025-11-21 15:36:38.000000000","message":"Thanks for this change, I think it\u0027s much better this way. Just a few comments on the test coverage. Sorry for the delay in getting back to this.","commit_id":"75c822f642859a8884a13516010d3905dea49fcb"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"1623670d3b2bd95863aad8bce6e8a524a4681f37","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3fd85ca1_8b012045","updated":"2025-11-18 20:04:02.000000000","message":"sorry for the delay. was waiting for the lab to test this code just in case. it works","commit_id":"75c822f642859a8884a13516010d3905dea49fcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c219a45b79ebd474765bfddee95159fe83506e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"37f85135_6e505420","updated":"2025-12-02 14:59:56.000000000","message":"Thanks!","commit_id":"e01a5f8e71959d1a283c96be59e3cb5138e726b7"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"5ab29d272ea7a47e70a36a1320ae4a56606c7f7d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"53c1244e_672cb112","updated":"2025-11-25 22:15:35.000000000","message":"recheck\n\nseems like it was just paramico timeout on one of the tests","commit_id":"e01a5f8e71959d1a283c96be59e3cb5138e726b7"}],"glance/common/wsgi.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9805db981bd4491efa5c807eb07d3bfcb04eea19","unresolved":true,"context_lines":[{"line_number":916,"context_line":"            length \u003d None"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        data_to_return \u003d b\"\""},{"line_number":919,"context_line":"        bytes_needed \u003d length if length is not None else float(\u0027inf\u0027)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"        # 1. Try to fulfill the request from the internal buffer"},{"line_number":922,"context_line":"        if self._buffer:"}],"source_content_type":"text/x-python","patch_set":4,"id":"f0d380dd_45f9e282","line":919,"updated":"2025-11-04 17:33:34.000000000","message":"This will effectively allow us to read until we OOM right? I don\u0027t see how this improves that for the length\u003dNone case. Are you sure that we need to actually support length\u003dNone? Also, on a socket, `read()` will not block until EOF but until the next packet arrives. Surely we can/should emulate that behavior here and not build an OOM into the code right?","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"1623670d3b2bd95863aad8bce6e8a524a4681f37","unresolved":false,"context_lines":[{"line_number":916,"context_line":"            length \u003d None"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        data_to_return \u003d b\"\""},{"line_number":919,"context_line":"        bytes_needed \u003d length if length is not None else float(\u0027inf\u0027)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"        # 1. Try to fulfill the request from the internal buffer"},{"line_number":922,"context_line":"        if self._buffer:"}],"source_content_type":"text/x-python","patch_set":4,"id":"add0813e_7f639420","line":919,"in_reply_to":"823cc700_0edc27a0","updated":"2025-11-18 20:04:02.000000000","message":"Done","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"d9ae5bc2391f926aa2c7d1e1139c04097172acca","unresolved":true,"context_lines":[{"line_number":916,"context_line":"            length \u003d None"},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"        data_to_return \u003d b\"\""},{"line_number":919,"context_line":"        bytes_needed \u003d length if length is not None else float(\u0027inf\u0027)"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"        # 1. Try to fulfill the request from the internal buffer"},{"line_number":922,"context_line":"        if self._buffer:"}],"source_content_type":"text/x-python","patch_set":4,"id":"823cc700_0edc27a0","line":919,"in_reply_to":"f0d380dd_45f9e282","updated":"2025-11-07 22:55:42.000000000","message":"there is a test \u0027test_read_data_no_length\u0027, which covers length\u003dNone.\nBut I agree, we don\u0027t want OOM - so your approach with some sane maximum where we should stop may be much more reasonable than float(\u0027inf\u0027)","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9805db981bd4491efa5c807eb07d3bfcb04eea19","unresolved":true,"context_lines":[{"line_number":959,"context_line":""},{"line_number":960,"context_line":"                    # b. Cache the excess part"},{"line_number":961,"context_line":"                    excess_part \u003d data[bytes_needed:]"},{"line_number":962,"context_line":"                    self._buffer \u003d excess_part"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"                    # c. Stop reading"},{"line_number":965,"context_line":"                    bytes_needed \u003d 0  # Request fulfilled"}],"source_content_type":"text/x-python","patch_set":4,"id":"2c7c0afe_8c159504","line":962,"updated":"2025-11-04 17:33:34.000000000","message":"This all seems more complicated than necessary. IMHO, we should just be filling `self._buffer` with any reads we do, and then pulling data off the front of the buffer until we have satisfied the need. Something like the following:\n\n```\n# If no length is provided, choose some sane minimum default\nlength \u003d max(length, length or 1 * units.Mi)\n\nwhile length \u003c\u003d len(self._buffer):\n    data \u003d uwsgi.chunked_read()\n    if not data:\n        break\n\nchunk \u003d self._buffer[:length]\nself._buffer \u003d self._buffer[length:]\nreturn chunk\n```\n\nThe above will:\n1. Avoid doing any reads if we already have data in the buffer to satisfy the request\n2. Never return more than the amount of data requested\n3. Not ever agree to fill the buffer with more than 1MiB of data if unbounded (this default up for discussion).","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"d9ae5bc2391f926aa2c7d1e1139c04097172acca","unresolved":true,"context_lines":[{"line_number":959,"context_line":""},{"line_number":960,"context_line":"                    # b. Cache the excess part"},{"line_number":961,"context_line":"                    excess_part \u003d data[bytes_needed:]"},{"line_number":962,"context_line":"                    self._buffer \u003d excess_part"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"                    # c. Stop reading"},{"line_number":965,"context_line":"                    bytes_needed \u003d 0  # Request fulfilled"}],"source_content_type":"text/x-python","patch_set":4,"id":"8068958a_df7a8a42","line":962,"in_reply_to":"2c7c0afe_8c159504","updated":"2025-11-07 22:55:42.000000000","message":"Thank you for review Dan, I like it, 1 Mi looks more practical and there will be much less code, I\u0027ll test this approach and return back","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"1623670d3b2bd95863aad8bce6e8a524a4681f37","unresolved":false,"context_lines":[{"line_number":959,"context_line":""},{"line_number":960,"context_line":"                    # b. Cache the excess part"},{"line_number":961,"context_line":"                    excess_part \u003d data[bytes_needed:]"},{"line_number":962,"context_line":"                    self._buffer \u003d excess_part"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"                    # c. Stop reading"},{"line_number":965,"context_line":"                    bytes_needed \u003d 0  # Request fulfilled"}],"source_content_type":"text/x-python","patch_set":4,"id":"180f8f8d_dcd40a6c","line":962,"in_reply_to":"8068958a_df7a8a42","updated":"2025-11-18 20:04:02.000000000","message":"Done","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9805db981bd4491efa5c807eb07d3bfcb04eea19","unresolved":true,"context_lines":[{"line_number":962,"context_line":"                    self._buffer \u003d excess_part"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"                    # c. Stop reading"},{"line_number":965,"context_line":"                    bytes_needed \u003d 0  # Request fulfilled"},{"line_number":966,"context_line":"                    break"},{"line_number":967,"context_line":""},{"line_number":968,"context_line":"        return data_to_return"}],"source_content_type":"text/x-python","patch_set":4,"id":"a0f42290_381433fd","line":965,"updated":"2025-11-04 17:33:34.000000000","message":"This is dead code, please remove for clarity.","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"1623670d3b2bd95863aad8bce6e8a524a4681f37","unresolved":false,"context_lines":[{"line_number":962,"context_line":"                    self._buffer \u003d excess_part"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"                    # c. Stop reading"},{"line_number":965,"context_line":"                    bytes_needed \u003d 0  # Request fulfilled"},{"line_number":966,"context_line":"                    break"},{"line_number":967,"context_line":""},{"line_number":968,"context_line":"        return data_to_return"}],"source_content_type":"text/x-python","patch_set":4,"id":"45c4554f_d70e456a","line":965,"in_reply_to":"a0f42290_381433fd","updated":"2025-11-18 20:04:02.000000000","message":"Done","commit_id":"a88ffdc0705f483de7e62593409ecde1a4596765"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"28d423759466978ff7f637e998541b6bfa1575b7","unresolved":true,"context_lines":[{"line_number":919,"context_line":"        # If no length is provided, choose some sane minimum default"},{"line_number":920,"context_line":"        length \u003d length if length is not None else 1 * units.Mi"},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"        while len(self._buffer) \u003c length:"},{"line_number":923,"context_line":"            data \u003d uwsgi.chunked_read()"},{"line_number":924,"context_line":"            if not data:"},{"line_number":925,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":6,"id":"e980874b_584486c4","line":922,"updated":"2025-11-21 15:36:38.000000000","message":"Thanks for fixing my mind-o with the ordering here :)","commit_id":"75c822f642859a8884a13516010d3905dea49fcb"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"f0fdb954050afe2065f057a33d6bcf299817bb8a","unresolved":false,"context_lines":[{"line_number":919,"context_line":"        # If no length is provided, choose some sane minimum default"},{"line_number":920,"context_line":"        length \u003d length if length is not None else 1 * units.Mi"},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"        while len(self._buffer) \u003c length:"},{"line_number":923,"context_line":"            data \u003d uwsgi.chunked_read()"},{"line_number":924,"context_line":"            if not data:"},{"line_number":925,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":6,"id":"8bb5da2f_03315381","line":922,"in_reply_to":"e980874b_584486c4","updated":"2025-11-24 19:55:37.000000000","message":"Done","commit_id":"75c822f642859a8884a13516010d3905dea49fcb"}],"glance/tests/unit/common/test_wsgi.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"28d423759466978ff7f637e998541b6bfa1575b7","unresolved":true,"context_lines":[{"line_number":852,"context_line":"        self.assertEqual(out, b\u0027cd\u0027)"},{"line_number":853,"context_line":""},{"line_number":854,"context_line":"        out \u003d reader.read(length\u003d2)"},{"line_number":855,"context_line":"        self.assertEqual(out, b\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5d7cbb0b_9556bc35","line":855,"updated":"2025-11-21 15:36:38.000000000","message":"I think this is good because it tests that we fill the buffer as needed. However, I don\u0027t see a case where you read a smaller amount than the next `chunked_read()` and only get the smaller amount (one of the cases this is supposed to fix).\n\nAlso can you also test read of a negative number? That\u0027s in the commit message as being fixed from the original implementation, so a regression-protecting test would be good.\n\nCan you add a case where we fully satisfy a read from the existing buffer and make sure we didn\u0027t do another read? If the code were designed differently, a ton of 1-byte reads could eventually OOM us if we always did at lease one `chunked_read()` per call. We don\u0027t, which means we\u0027re good, but please write a test to assert that so such a case doesn\u0027t creep in later.","commit_id":"75c822f642859a8884a13516010d3905dea49fcb"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"f0fdb954050afe2065f057a33d6bcf299817bb8a","unresolved":true,"context_lines":[{"line_number":852,"context_line":"        self.assertEqual(out, b\u0027cd\u0027)"},{"line_number":853,"context_line":""},{"line_number":854,"context_line":"        out \u003d reader.read(length\u003d2)"},{"line_number":855,"context_line":"        self.assertEqual(out, b\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"919ad965_8ba53fc3","line":855,"in_reply_to":"5d7cbb0b_9556bc35","updated":"2025-11-24 19:55:37.000000000","message":"I agree, those cases should be covered. I\u0027m not sure yet what is the best way on how do the the last one, probably will need to somehow modify the mock for chunked_read() more to check number of reads... I think I\u0027ll figure that out, just need to find some time to re-focus on this","commit_id":"75c822f642859a8884a13516010d3905dea49fcb"},{"author":{"_account_id":21040,"name":"Alexey","email":"aodinokov@mirantis.com","username":"aodinokov"},"change_message_id":"2ca6d8245401de144a055ae2f9735f883270f9d4","unresolved":false,"context_lines":[{"line_number":852,"context_line":"        self.assertEqual(out, b\u0027cd\u0027)"},{"line_number":853,"context_line":""},{"line_number":854,"context_line":"        out \u003d reader.read(length\u003d2)"},{"line_number":855,"context_line":"        self.assertEqual(out, b\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5c93bebd_7ae4a807","line":855,"in_reply_to":"919ad965_8ba53fc3","updated":"2025-11-25 19:10:02.000000000","message":"Dan, I tried to cover item 1 with the sub-case 2\n\"buffer contains more - no extra read\"\n\nand item 3, by introducing counter that counts number of reads\nmaking sure that sub-case 3 \"buffer contains exactly what we need - no extra read\"\ndoesn\u0027t cause any additional reads.\n\nspeaking about item 2 - I noticed that we have \"test_read_data_negative_length\" which should cover negative numbers. I also use negative for the subcase\n\"eof case when request till eof\" which covers a bit different scenario.\n\nI think we should be good, but please let me know if I\u0027ve missed anything important.","commit_id":"75c822f642859a8884a13516010d3905dea49fcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c219a45b79ebd474765bfddee95159fe83506e9","unresolved":true,"context_lines":[{"line_number":848,"context_line":"            values_read_count +\u003d 1"},{"line_number":849,"context_line":"            return next(values)"},{"line_number":850,"context_line":""},{"line_number":851,"context_line":"        def values_read_count_get():"},{"line_number":852,"context_line":"            nonlocal values_read_count, values_read_count_prev"},{"line_number":853,"context_line":"            res \u003d values_read_count - values_read_count_prev"},{"line_number":854,"context_line":"            values_read_count_prev \u003d values_read_count"}],"source_content_type":"text/x-python","patch_set":8,"id":"d8ffc9b7_e219db65","line":851,"updated":"2025-12-02 14:59:56.000000000","message":"Normally we\u0027d use a `mock.reset()` and `assertCalled..()` here, but okay :)","commit_id":"e01a5f8e71959d1a283c96be59e3cb5138e726b7"}]}