)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a2c6841726f383f35df1210ddd1d09175d53f322","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2d75f5e0_ef4f9677","updated":"2025-08-06 19:45:36.000000000","message":"There\u0027s a bunch of clean-up in this change:\n\n* indicate that _RingGzReader is private and abstract (i.e. only ever used as a parent class for RingReader)\n * indicate that _RingGzWriter is private and abstract (i.e. only ever used as a parent class for RingWriter)\n * don\u0027t conflate seek/tell with GzipFile (i.e. new interface methods named compressed_seek/compressed_tell)\n * Make checksumming required for all sections (i.e. old code will error on unsupported checksums until you upgrade it; which is better than ignoring errors)\n * Do not create a section for swift/index (probably mostly to side-step thhe \"all sections require checksums inception problem; but also totally fine - we can add add an index checksum to a new uncompressed zlib deflate data block just ahead of the uncompressed byte offset)\n \nDrive-by explicitly remove dev_id_bytes from deserialize_v1 - it was sort redundant but mostly just un-necessary.\n\nI\u0027m not +A b/c Tim indicated he may not like all of these changes?  I think they\u0027re all improvements.","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"00fc917ebf39259608180297764941299e4b4520","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7cf801fe_96938a8c","in_reply_to":"2d75f5e0_ef4f9677","updated":"2025-08-06 22:18:12.000000000","message":"\u003e old code will error on unsupported checksums until you upgrade it; which is better than ignoring errors\n\nUnless we want to change which data we\u0027re checksumming.\n\nIf we wanted to drop the leading length bytes from the checksum (so the checksum in the index matches the checksum if this got exploded out on a filesystem), one way to do it and not break the world would be to switch to a different checksum algorithm and drop quietly support for sha256. If we do that before there\u0027s a Swift release that *does* support sha256, we could still add it as an option later.\n\n\u003e probably mostly to side-step thhe \"all sections require checksums inception problem\n\nAlso because it made for less fighting with the type system when I tried running `mypy swift/common/ring/io.py --ignore-missing-imports` -- the optionality of the end offsets got messy. You\u0027re right that if we\u0027re going to have type hints, we ought to work toward doing some useful checking with them.","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b333a4f90041de229c57641de8d4d917e395b680","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0de56d56_f50b2df5","in_reply_to":"7cf801fe_96938a8c","updated":"2025-08-08 01:55:55.000000000","message":"Eh, could still pull some trick like this if we wanted. May as well get it the way you wanted it.","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"}],"swift/common/ring/ring.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a2c6841726f383f35df1210ddd1d09175d53f322","unresolved":true,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        ring_dict \u003d json.loads(reader.read_blob(\u0027!I\u0027))"},{"line_number":140,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":141,"context_line":"        ring_dict[\u0027dev_id_bytes\u0027] \u003d 2"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        if metadata_only:"},{"line_number":144,"context_line":"            return ring_dict"}],"source_content_type":"text/x-python","patch_set":1,"id":"4a63c886_ef1f975f","side":"PARENT","line":141,"updated":"2025-08-06 19:45:36.000000000","message":"oic, this was explicitly removed from `deserialize_v1`\n\nseems fine.","commit_id":"ae062f8b09aed2f7bad9581396607045ea217fa8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"00fc917ebf39259608180297764941299e4b4520","unresolved":false,"context_lines":[{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        ring_dict \u003d json.loads(reader.read_blob(\u0027!I\u0027))"},{"line_number":140,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":141,"context_line":"        ring_dict[\u0027dev_id_bytes\u0027] \u003d 2"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        if metadata_only:"},{"line_number":144,"context_line":"            return ring_dict"}],"source_content_type":"text/x-python","patch_set":1,"id":"70eb74e4_4f83c3a9","side":"PARENT","line":141,"in_reply_to":"4a63c886_ef1f975f","updated":"2025-08-06 22:18:12.000000000","message":"I thought you explicitly wanted it gone? 😜","commit_id":"ae062f8b09aed2f7bad9581396607045ea217fa8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a2c6841726f383f35df1210ddd1d09175d53f322","unresolved":true,"context_lines":[{"line_number":168,"context_line":"        :param bool metadata_only: If True, skip loading"},{"line_number":169,"context_line":"                                   ``replica2part2dev_id``"},{"line_number":170,"context_line":"        :param bool include_devices: If False and ``metadata_only`` is True,"},{"line_number":171,"context_line":"                                     skip loading ``devs``"},{"line_number":172,"context_line":"        :returns: A dict containing ``devs``, ``part_shift``,"},{"line_number":173,"context_line":"                  ``dev_id_bytes``, and ``replica2part2dev_id``"},{"line_number":174,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"8dff3c28_b4ac38b7","line":171,"updated":"2025-08-06 19:45:36.000000000","message":"did this get out of sync with the implementation","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b333a4f90041de229c57641de8d4d917e395b680","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        :param bool metadata_only: If True, skip loading"},{"line_number":169,"context_line":"                                   ``replica2part2dev_id``"},{"line_number":170,"context_line":"        :param bool include_devices: If False and ``metadata_only`` is True,"},{"line_number":171,"context_line":"                                     skip loading ``devs``"},{"line_number":172,"context_line":"        :returns: A dict containing ``devs``, ``part_shift``,"},{"line_number":173,"context_line":"                  ``dev_id_bytes``, and ``replica2part2dev_id``"},{"line_number":174,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"834a4632_74334fba","line":171,"in_reply_to":"0c0afe28_75357e84","updated":"2025-08-08 01:55:55.000000000","message":"Done","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"00fc917ebf39259608180297764941299e4b4520","unresolved":true,"context_lines":[{"line_number":168,"context_line":"        :param bool metadata_only: If True, skip loading"},{"line_number":169,"context_line":"                                   ``replica2part2dev_id``"},{"line_number":170,"context_line":"        :param bool include_devices: If False and ``metadata_only`` is True,"},{"line_number":171,"context_line":"                                     skip loading ``devs``"},{"line_number":172,"context_line":"        :returns: A dict containing ``devs``, ``part_shift``,"},{"line_number":173,"context_line":"                  ``dev_id_bytes``, and ``replica2part2dev_id``"},{"line_number":174,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"0c0afe28_75357e84","line":171,"in_reply_to":"8dff3c28_b4ac38b7","updated":"2025-08-06 22:18:12.000000000","message":"Good call, should fix.","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"}],"test/unit/common/ring/test_ring.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a2c6841726f383f35df1210ddd1d09175d53f322","unresolved":true,"context_lines":[{"line_number":294,"context_line":"        self.assertEqual(loaded_rd[\u0027part_shift\u0027], 30)"},{"line_number":295,"context_line":"        self.assertEqual(loaded_rd[\u0027replica_count\u0027], 2)"},{"line_number":296,"context_line":"        self.assertEqual(loaded_rd[\u0027dev_id_bytes\u0027], 2)"},{"line_number":297,"context_line":"        self.assertTrue(loaded_rd[\u0027replica2part2dev_id\u0027])"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def test_load(self):"},{"line_number":300,"context_line":"        rd \u003d ring.RingData("}],"source_content_type":"text/x-python","patch_set":1,"id":"fd1c0cec_4579b758","line":297,"updated":"2025-08-06 19:45:36.000000000","message":"heheh, yeah I get that having the assignments w/o devices is sort of nuts!\n\nI like the `include_assignments\u003dFalse` idea!?  And then `metadata_only\u003dTrue` is just an alias for `include_devices\u003dTrue, include_assignments\u003dFalse`???\n\nI think we could even justify that if you say:\n\n\n```\nmetadata_only\u003dTrue, include_devices\u003dTrue, include_assignments\u003dTrue\n```\n\nyou *actually* get\n\n```\ninclude_devices\u003dFalse, include_assignemtns\u003dFalse\n```","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"00fc917ebf39259608180297764941299e4b4520","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        self.assertEqual(loaded_rd[\u0027part_shift\u0027], 30)"},{"line_number":295,"context_line":"        self.assertEqual(loaded_rd[\u0027replica_count\u0027], 2)"},{"line_number":296,"context_line":"        self.assertEqual(loaded_rd[\u0027dev_id_bytes\u0027], 2)"},{"line_number":297,"context_line":"        self.assertTrue(loaded_rd[\u0027replica2part2dev_id\u0027])"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def test_load(self):"},{"line_number":300,"context_line":"        rd \u003d ring.RingData("}],"source_content_type":"text/x-python","patch_set":1,"id":"cedf61ca_bb9c1877","line":297,"in_reply_to":"fd1c0cec_4579b758","updated":"2025-08-06 22:18:12.000000000","message":"I feel like I\u0027d rather work toward some\n```\nring \u003d Ring.load_sections(ring_file, \u0027swift/ring/*\u0027)\nmeta_only_ring \u003d Ring.load_sections(ring_file, \u0027swift/ring/metadata\u0027)\nno_assignments_ring \u003d Ring.load_sections(ring_file, \u0027swift/ring/metadata\u0027, \u0027swift/ring/devices\u0027)\ndev_list \u003d Ring.load_sections(ring_file, \u0027swift/ring/devices\u0027).devs\n```\nsort of an interface, and deprecate the bool args to `Ring.load` and `deserialize_v*`.","commit_id":"83bcbcc347f119d2f29a832b4f6c670932f8cb04"}]}
