)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"262d993c39415e4b5db4f529fe1ea58dd23dd8d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6d02d1b0_de86074d","updated":"2026-06-15 21:09:23.000000000","message":"IMHO this change is correct and complete as-is; I would prefer to see some additional investments in the tests but I don\u0027t think they should block the bug fix.  comments in code are nearly universally bad so I\u0027ll just ignore that one.","commit_id":"47dbcf97230f6b1f8c7f3de9e7d28c940102a596"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"68ead19ca9975d86b5812d1bd8045cc335afdd33","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cb9d5269_f40f9351","in_reply_to":"6d02d1b0_de86074d","updated":"2026-06-16 07:37:24.000000000","message":"Ack - the comment is bad, and before we carry this for longer, here is the fix - removing the comment and renaming base to start_offset: https://review.opendev.org/c/openstack/swift/+/993513","commit_id":"47dbcf97230f6b1f8c7f3de9e7d28c940102a596"}],"swift/obj/diskfile.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"262d993c39415e4b5db4f529fe1ea58dd23dd8d9","unresolved":true,"context_lines":[{"line_number":2189,"context_line":"            # absolute file offset this read starts at (a range start, or 0)."},{"line_number":2190,"context_line":"            # _drop_cache wants a file offset, but dropped_cache counts from"},{"line_number":2191,"context_line":"            # here, so add the base -- otherwise a ranged read fadvises the"},{"line_number":2192,"context_line":"            # wrong region (the head of the file rather than what it read)."},{"line_number":2193,"context_line":"            base \u003d self._fp.tell()"},{"line_number":2194,"context_line":"            self._bytes_read \u003d 0"},{"line_number":2195,"context_line":"            self._started_at_0 \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"5b2e516e_a5f55cdf","line":2192,"updated":"2026-06-15 21:09:23.000000000","message":"IMHO this comment bloats the change - I\u0027m not even sure I understand everything this comment is saying - it\u0027s almost like reading \"we had a bug like this here one time\"","commit_id":"47dbcf97230f6b1f8c7f3de9e7d28c940102a596"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"68ead19ca9975d86b5812d1bd8045cc335afdd33","unresolved":false,"context_lines":[{"line_number":2189,"context_line":"            # absolute file offset this read starts at (a range start, or 0)."},{"line_number":2190,"context_line":"            # _drop_cache wants a file offset, but dropped_cache counts from"},{"line_number":2191,"context_line":"            # here, so add the base -- otherwise a ranged read fadvises the"},{"line_number":2192,"context_line":"            # wrong region (the head of the file rather than what it read)."},{"line_number":2193,"context_line":"            base \u003d self._fp.tell()"},{"line_number":2194,"context_line":"            self._bytes_read \u003d 0"},{"line_number":2195,"context_line":"            self._started_at_0 \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"d7ae979f_e25cbdf0","line":2192,"in_reply_to":"5b2e516e_a5f55cdf","updated":"2026-06-16 07:37:24.000000000","message":"Acknowledged","commit_id":"47dbcf97230f6b1f8c7f3de9e7d28c940102a596"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"262d993c39415e4b5db4f529fe1ea58dd23dd8d9","unresolved":true,"context_lines":[{"line_number":2191,"context_line":"            # here, so add the base -- otherwise a ranged read fadvises the"},{"line_number":2192,"context_line":"            # wrong region (the head of the file rather than what it read)."},{"line_number":2193,"context_line":"            base \u003d self._fp.tell()"},{"line_number":2194,"context_line":"            self._bytes_read \u003d 0"},{"line_number":2195,"context_line":"            self._started_at_0 \u003d False"},{"line_number":2196,"context_line":"            self._read_to_eof \u003d False"},{"line_number":2197,"context_line":"            self._init_checks()"}],"source_content_type":"text/x-python","patch_set":1,"id":"dc277a08_61f5b2b9","line":2194,"updated":"2026-06-15 21:09:23.000000000","message":"perhaps more obvious as `start_offset \u003d tell()` maybe `init_offset`?\n\nI always feel justified when the bots agree with me:\n\n```\n• I don’t think a 4-line comment is needed there. The code is pretty self-explanatory if the variable name carries the idea.\n\n  I’d use either no comment with a better name:\n\n  read_start \u003d self._fp.tell()\n\n  Then:\n\n  self._drop_cache(\n      self._fp.fileno(), read_start + dropped_cache,\n      self._bytes_read - dropped_cache)\n\n  If you want a comment, I’d make it one short line focused on why tell() matters:\n\n  # app_iter_range() may have already seeked into the file.\n  read_start \u003d self._fp.tell()\n\n  That avoids restating _drop_cache mechanics and keeps the important invariant visible: _bytes_read/dropped_cache are relative to the current read, while drop_buffer_cache needs absolute file offsets.\n```","commit_id":"47dbcf97230f6b1f8c7f3de9e7d28c940102a596"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"68ead19ca9975d86b5812d1bd8045cc335afdd33","unresolved":false,"context_lines":[{"line_number":2191,"context_line":"            # here, so add the base -- otherwise a ranged read fadvises the"},{"line_number":2192,"context_line":"            # wrong region (the head of the file rather than what it read)."},{"line_number":2193,"context_line":"            base \u003d self._fp.tell()"},{"line_number":2194,"context_line":"            self._bytes_read \u003d 0"},{"line_number":2195,"context_line":"            self._started_at_0 \u003d False"},{"line_number":2196,"context_line":"            self._read_to_eof \u003d False"},{"line_number":2197,"context_line":"            self._init_checks()"}],"source_content_type":"text/x-python","patch_set":1,"id":"bcf151e6_a9bcfbd1","line":2194,"in_reply_to":"dc277a08_61f5b2b9","updated":"2026-06-16 07:37:24.000000000","message":"Acknowledged","commit_id":"47dbcf97230f6b1f8c7f3de9e7d28c940102a596"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"262d993c39415e4b5db4f529fe1ea58dd23dd8d9","unresolved":true,"context_lines":[{"line_number":4338,"context_line":"        self.assertEqual(body, on_disk[500:1500])"},{"line_number":4339,"context_line":"        # drops fire (keep_cache off) and never target offset 0"},{"line_number":4340,"context_line":"        self.assertTrue(offsets)"},{"line_number":4341,"context_line":"        self.assertEqual(min(offsets), 500, \u0027offsets\u003d%r\u0027 % offsets)"},{"line_number":4342,"context_line":""},{"line_number":4343,"context_line":"    def test_disk_file_app_iter_range_w_none(self):"},{"line_number":4344,"context_line":"        df, df_data \u003d self._create_test_file(b\u00271234567890\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a4b72de1_4c7feea5","line":4341,"updated":"2026-06-15 21:09:23.000000000","message":"the `mock.patch(..., lambda: .append())` trick is *kinda* cute, but the assertions seems weak:\n\n```\n\u003e       self.assertEqual(min(offsets), 500, \u0027offsets\u003d%r\u0027 % offsets)\nE       AssertionError: 0 !\u003d 500 : offsets\u003d[0]\n```\n\nthere\u0027s a single 64K read of a 20K object starting at byte 500 instead of 0 - the bug almost doesn\u0027t present in this case.\n\n```\n\u003e       self.assertEqual(\n            [mock.call(fd, 500, len(on_disk) - 500)],\n            mock_drop_cache.call_args_list)\nE       AssertionError: [call(\u003cANY\u003e, 500, 19500)] !\u003d [call(12, 0, 19500)]\n```\n\nBetter I think to create an object larger the keep_cache_size, assert the whole-object reads do the aligned per cache_size drop and THEN do the ranged equivalent:\n\n993459: sq? Assert range cache drop calls directly | https://review.opendev.org/c/openstack/swift/+/993459","commit_id":"47dbcf97230f6b1f8c7f3de9e7d28c940102a596"}]}