)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a02cbc24df61616e826bde47b0348234445c2b87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d433e159_95b973ae","updated":"2022-03-23 00:53:35.000000000","message":"Sorry still havn\u0027t cleaned it up. I got rather sidetracted thinking about last part table again :P ","commit_id":"7b7d8ec635d9d6d9829a2abd45d3f2c1f4cf4d55"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bca1ab2f041f009f671609bcf1a9773914e57595","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"caef0321_2778eeeb","updated":"2022-05-04 20:42:56.000000000","message":"Bleh -- looks like I had some stale comments from like three patchsets ago.","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b32e8273_9d51b71a","updated":"2022-07-09 04:23:46.000000000","message":"OK, I\u0027ve got a rebase on the way -- it still has some failing tests -- but I\u0027m not sure I actually like this better. WDYT about flipping the order and putting this *after* the patch to add last primaries? I feel like this is likely going to need more work.","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7870eda5ce3664d860ef38b88ce99e98ae789396","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1798fd83_92c23723","in_reply_to":"b32e8273_9d51b71a","updated":"2022-07-11 00:04:56.000000000","message":"Yeah not a bad idea, I\u0027ll rework it and put it before this. This stuff I think really shines when we want to add new things _and_ are using the ring for more then just the ring, ie, ringbuilder as well. But yeah no doubt there can be improvements to it.","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"}],"swift/common/ring/ring.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bca1ab2f041f009f671609bcf1a9773914e57595","unresolved":true,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        # pull metadata out of the file"},{"line_number":143,"context_line":"        with reader.open_section(\u0027swift/metadata\u0027) as r:"},{"line_number":144,"context_line":"            ring_dict \u003d json.load(r)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":147,"context_line":"        ring_dict[\u0027devs\u0027] \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"b30d9a41_96775a39","line":144,"updated":"2022-05-04 20:42:56.000000000","message":"Could even do\n\n ring_dict \u003d json.loads(reader.read_section(\u0027swift/metadata\u0027))\n\nThe way Python does json.load() means there isn\u0027t any advantage to opening the section and working with a file-like: https://github.com/python/cpython/blob/v3.10.0/Lib/json/__init__.py#L293\n\nI was also debating about adding a RingReader.read_json() helper, but that\u0027s more for the symmetry with RingWriter.write_json() than any actual ergonomics.","commit_id":"7b7d8ec635d9d6d9829a2abd45d3f2c1f4cf4d55"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bca1ab2f041f009f671609bcf1a9773914e57595","unresolved":true,"context_lines":[{"line_number":147,"context_line":"        ring_dict[\u0027devs\u0027] \u003d []"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        if metadata_only:"},{"line_number":150,"context_line":"            # Note empty devs and replica2part2dev table"},{"line_number":151,"context_line":"            return ring_dict"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        # Now grab the devs"}],"source_content_type":"text/x-python","patch_set":1,"id":"6b41e1d3_594cfebc","line":150,"updated":"2022-05-04 20:42:56.000000000","message":"IDK -- might want devs, too... at the very least, we ought to call it out in the docstring like we do for replica2part2dev_id.","commit_id":"7b7d8ec635d9d6d9829a2abd45d3f2c1f4cf4d55"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bca1ab2f041f009f671609bcf1a9773914e57595","unresolved":true,"context_lines":[{"line_number":237,"context_line":"        metadata \u003d {\u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":238,"context_line":"                    \u0027dev_id_bytes\u0027: ring[\u0027dev_id_bytes\u0027]}"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        if ring[\u0027version\u0027] is not None:"},{"line_number":241,"context_line":"            metadata[\u0027version\u0027] \u003d ring[\u0027version\u0027]"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"        next_part_power \u003d ring.get(\u0027next_part_power\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7fdc67e8_8c9ed3c0","line":240,"updated":"2022-05-04 20:42:56.000000000","message":"I need to refresh my memory of when ring[\u0027version\u0027] can be None :-/","commit_id":"7b7d8ec635d9d6d9829a2abd45d3f2c1f4cf4d55"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f475284815a807aef79be707df8d30c93c328981","unresolved":true,"context_lines":[{"line_number":237,"context_line":"        metadata \u003d {\u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":238,"context_line":"                    \u0027dev_id_bytes\u0027: ring[\u0027dev_id_bytes\u0027]}"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        if ring[\u0027version\u0027] is not None:"},{"line_number":241,"context_line":"            metadata[\u0027version\u0027] \u003d ring[\u0027version\u0027]"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"        next_part_power \u003d ring.get(\u0027next_part_power\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a833bcd_ea534f62","line":240,"in_reply_to":"7fdc67e8_8c9ed3c0","updated":"2022-05-08 23:46:23.000000000","message":"Backwards compat. We used to not have a ring version. But seeing as we\u0027re now in a v2 world. Maybe it\u0027s safe to assume version always exists?","commit_id":"7b7d8ec635d9d6d9829a2abd45d3f2c1f4cf4d55"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bca1ab2f041f009f671609bcf1a9773914e57595","unresolved":true,"context_lines":[{"line_number":259,"context_line":"            for i, part2dev_id in enumerate(ring[\u0027replica2part2dev_id\u0027]):"},{"line_number":260,"context_line":"                with network_order_array(part2dev_id):"},{"line_number":261,"context_line":"                    if i \u003d\u003d 0:"},{"line_number":262,"context_line":"                        writer.write(struct.pack(\u0027!Q\u0027, length))"},{"line_number":263,"context_line":"                    if six.PY2:"},{"line_number":264,"context_line":"                        # Can\u0027t just use tofile() because a GzipFile apparently"},{"line_number":265,"context_line":"                        # doesn\u0027t count as an \u0027open file\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"59784e1c_3be0163a","line":262,"updated":"2022-05-04 20:42:56.000000000","message":"Eh, I\u0027d move this up and out of the loop -- can drop the enumerate and fewer branches in the loop.","commit_id":"7b7d8ec635d9d6d9829a2abd45d3f2c1f4cf4d55"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f475284815a807aef79be707df8d30c93c328981","unresolved":true,"context_lines":[{"line_number":259,"context_line":"            for i, part2dev_id in enumerate(ring[\u0027replica2part2dev_id\u0027]):"},{"line_number":260,"context_line":"                with network_order_array(part2dev_id):"},{"line_number":261,"context_line":"                    if i \u003d\u003d 0:"},{"line_number":262,"context_line":"                        writer.write(struct.pack(\u0027!Q\u0027, length))"},{"line_number":263,"context_line":"                    if six.PY2:"},{"line_number":264,"context_line":"                        # Can\u0027t just use tofile() because a GzipFile apparently"},{"line_number":265,"context_line":"                        # doesn\u0027t count as an \u0027open file\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"c0eae09a_380e793e","line":262,"in_reply_to":"59784e1c_3be0163a","updated":"2022-05-08 23:46:23.000000000","message":"oh yeah true","commit_id":"7b7d8ec635d9d6d9829a2abd45d3f2c1f4cf4d55"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":311,"context_line":"                        # doesn\u0027t count as an \u0027open file\u0027"},{"line_number":312,"context_line":"                        writer.write(part2dev_id.tostring())"},{"line_number":313,"context_line":"                    else:"},{"line_number":314,"context_line":"                        part2dev_id.tofile(writer)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"    def save(self, filename, mtime\u003d1300507380.0,"},{"line_number":317,"context_line":"             format_version\u003dDEFAULT_RING_FORMAT_VERSION):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5735816f_852ddcef","side":"PARENT","line":314,"updated":"2022-07-09 04:23:46.000000000","message":"IDK that smearing these 35 lines out as 100 lines over in util.py is better... and deserialize_v2 doesn\u0027t seems to have gotten dramatically shorter :-/","commit_id":"b3da7c22b17209960b62ef15476651a9754a5cda"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":180,"context_line":"        return ring_dict"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    @classmethod"},{"line_number":183,"context_line":"    def deserialize_v2(cls, reader, metadata_only\u003dFalse):"},{"line_number":184,"context_line":"        \"\"\""},{"line_number":185,"context_line":"        Deserialize a v2 ring file into a dictionary with `devs`, `part_shift`,"},{"line_number":186,"context_line":"        and `replica2part2dev_id` keys."}],"source_content_type":"text/x-python","patch_set":4,"id":"4bf13495_d684da34","line":183,"updated":"2022-07-09 04:23:46.000000000","message":"IDK, I think I liked the include_devices arg...","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        :param file reader: An opened RingReader object."},{"line_number":193,"context_line":"        :param bool metadata_only: If True, only load `devs` and `part_shift`"},{"line_number":194,"context_line":"        :returns: A dict containing `devs`, `part_shift`, and"},{"line_number":195,"context_line":"                  `replica2part2dev_id`"},{"line_number":196,"context_line":"        \"\"\""},{"line_number":197,"context_line":"        ring_data \u003d RingData([], [], None)"}],"source_content_type":"text/x-python","patch_set":4,"id":"d8b6d484_1ee54dc0","line":194,"updated":"2022-07-09 04:23:46.000000000","message":"No longer accurate: we\u0027re returning a RingData, which means TestRingExtensibility needs updating.\n\nThough, it\u0027s also a little weird to me that we\u0027re returning a RingData -- deserialize_v1() returns a dict. v0 is a weird special case, in that it started out having pickle.loads() return a RingData, but then we realized there were upgrade problems with that, so it switched to returning a dict.\n\nI think I liked the old boundaries a little more, where deserialize_vX would return a dict, and load was responsible for getting it transfered to a RingData instance.","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"68e3e7c501a35aeee78b460e407ff42f914af8a3","unresolved":true,"context_lines":[{"line_number":95,"context_line":"            return calc_replica_count(self._replica2part2dev_id)"},{"line_number":96,"context_line":"        else:"},{"line_number":97,"context_line":"            # For rings loaded with metadata_only\u003dTrue"},{"line_number":98,"context_line":"            return self._replica_count"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    @property"},{"line_number":101,"context_line":"    def dev_id_bytes(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3ee0034a_3c544c59","line":98,"updated":"2022-07-11 17:11:15.000000000","message":"I should probably pull this (and the similar bit for dev_id_bytes) into the parent patch...","commit_id":"c4385bc5b7fc18aebb942d033ae05056d7c11fda"}],"swift/common/ring/utils.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":790,"context_line":"    def serialize(self, writer):"},{"line_number":791,"context_line":"        raise NotImplementedError()"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"    def deserialize(self, reader):"},{"line_number":794,"context_line":"        raise NotImplementedError()"},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"c01a6f28_bd96a9a4","line":793,"updated":"2022-07-09 04:23:46.000000000","message":"All of the implementations are very much specific to v2 rings... I wonder if we\u0027ll wish we\u0027d done these as (de)serialize_v2 methods.","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":816,"context_line":""},{"line_number":817,"context_line":"        # First let\u0027s write the metadata, as this is the first we pull out"},{"line_number":818,"context_line":"        # in deserialize."},{"line_number":819,"context_line":"        metadata \u003d {\u0027_part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":820,"context_line":"                    \u0027dev_id_bytes\u0027: ring[\u0027dev_id_bytes\u0027]}"},{"line_number":821,"context_line":"        with writer.section(self.index_name):"},{"line_number":822,"context_line":"            writer.write_json(metadata)"}],"source_content_type":"text/x-python","patch_set":4,"id":"992f4a26_0e94cdcd","line":819,"updated":"2022-07-09 04:23:46.000000000","message":"IDK -- the leading underscore is a little fishy -- I guess it\u0027s for the sake of the setattr() below?","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":817,"context_line":"        # First let\u0027s write the metadata, as this is the first we pull out"},{"line_number":818,"context_line":"        # in deserialize."},{"line_number":819,"context_line":"        metadata \u003d {\u0027_part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":820,"context_line":"                    \u0027dev_id_bytes\u0027: ring[\u0027dev_id_bytes\u0027]}"},{"line_number":821,"context_line":"        with writer.section(self.index_name):"},{"line_number":822,"context_line":"            writer.write_json(metadata)"},{"line_number":823,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0bbc412b_bc43ad71","line":820,"updated":"2022-07-09 04:23:46.000000000","message":"There\u0027s at least a couple more metadata that need to come along, too:\n\n* version\n* next_part_power","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bca1ab2f041f009f671609bcf1a9773914e57595","unresolved":true,"context_lines":[{"line_number":826,"context_line":"            ring_dict \u003d json.load(r)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"        for k, v in ring_dict.items():"},{"line_number":829,"context_line":"            setattr(self.ring, k, v)"},{"line_number":830,"context_line":""},{"line_number":831,"context_line":""},{"line_number":832,"context_line":"@RingSerializeBase.register()"}],"source_content_type":"text/x-python","patch_set":4,"id":"4eead81a_aa401a14","line":829,"updated":"2022-05-04 20:42:56.000000000","message":"Looks like tests trip a lot of\n\n AttributeError: can\u0027t set attribute\n\nhere...","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f475284815a807aef79be707df8d30c93c328981","unresolved":true,"context_lines":[{"line_number":826,"context_line":"            ring_dict \u003d json.load(r)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"        for k, v in ring_dict.items():"},{"line_number":829,"context_line":"            setattr(self.ring, k, v)"},{"line_number":830,"context_line":""},{"line_number":831,"context_line":""},{"line_number":832,"context_line":"@RingSerializeBase.register()"}],"source_content_type":"text/x-python","patch_set":4,"id":"bc05ade8_a5affed5","line":829,"in_reply_to":"4eead81a_aa401a14","updated":"2022-05-08 23:46:23.000000000","message":"Yeah, I kinda feel wierd just setattr on the ring/builder object. But seemsed to add less scaffolding in the objects when useing the serialization classes. the to_dict keys chould now match values in the ring/builder object, so I hope their there.. but they might not be renamed in the multitude of test ring/builder classes we use :P","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":826,"context_line":"            ring_dict \u003d json.load(r)"},{"line_number":827,"context_line":""},{"line_number":828,"context_line":"        for k, v in ring_dict.items():"},{"line_number":829,"context_line":"            setattr(self.ring, k, v)"},{"line_number":830,"context_line":""},{"line_number":831,"context_line":""},{"line_number":832,"context_line":"@RingSerializeBase.register()"}],"source_content_type":"text/x-python","patch_set":4,"id":"baab6d0c_26712103","line":829,"in_reply_to":"bc05ade8_a5affed5","updated":"2022-07-09 04:23:46.000000000","message":"Looks like it had to do with the parent patch\u0027s change to have dev_id_bytes be a read-only property -- and just replacing this with\n\n        self.ring._part_shift \u003d ring_dict[\u0027_part_shift\u0027]\n        self.ring.set_dev_id_bytes(ring_dict[\u0027dev_id_bytes\u0027])\n\nwon\u0027t cut it, since the dev_id_bytes won\u0027t be set properly for RingSerializeReplica2Part2Dev later...","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"208d03d08dd12b2e863f0bc0782280e9a4374cee","unresolved":true,"context_lines":[{"line_number":842,"context_line":"        with reader.open_section(self.index_name) as r:"},{"line_number":843,"context_line":"            devs \u003d json.load(r)"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        setattr(self.ring, self.ring_attr, devs)"},{"line_number":846,"context_line":""},{"line_number":847,"context_line":""},{"line_number":848,"context_line":"@RingSerializeBase.register()"}],"source_content_type":"text/x-python","patch_set":4,"id":"edf98603_70d8eae5","line":845,"updated":"2022-07-09 04:23:46.000000000","message":"This seems to lose the\n\n        for dev in self.devs:\n            if dev is not None:\n                dev.setdefault(\"region\", 1)\n\nfrom RingData\u0027s constructor, but I guess it should be OK since any pre-region ring data would be loaded via v0/v1 (so would have had their dev list passed to the constructor, so all devs would pick up region 1) before they could be serialized as v2... I think? Ugh, I think I need to look through the builder code again...","commit_id":"c67661d74f69ab25b679c83581b85b99249332cc"}]}
