)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"63eb442722a0541af888a744f379801d7a476abe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5d06471e_0184ba52","updated":"2023-04-27 04:44:19.000000000","message":"I have a question... The _reload() is called by periodic checks as well as from load(). You are removing normalization from _reload(), so it\u0027s not going to happen anymore when rings are auto-laoded. I feel like it should be fine, it\u0027s not like operators are going to use obsolete ring builders. But just in case, did you consider such cases? We\u0027re not regressing here, right? is there no chance for some automation to produce rings that only work because of the normalization?","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"61d23e9bf3f2d7646f924f50045a6fd7d7caf233","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b3b94963_b7bd9673","updated":"2023-04-27 15:19:15.000000000","message":"I think moving the region and replication_ip stuff into one function and calling it at the right times is a good idea!\n\nI\u0027m a little confused by the load classmethod tho - and it\u0027s a smell that we have to call normalize_devices from two different places","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"74ef46828b43315411c6909a1d1df2417e5556ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"90b55c92_d0b513d0","updated":"2023-04-28 15:21:34.000000000","message":"ok, well this fixes a bug and seems correct.  We can argue about making it cleaner in follow-ups.","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"}],"swift/common/ring/ring.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"61d23e9bf3f2d7646f924f50045a6fd7d7caf233","unresolved":true,"context_lines":[{"line_number":199,"context_line":"                format_version, \u003d struct.unpack(\u0027!H\u0027, gz_file.read(2))"},{"line_number":200,"context_line":"                if format_version \u003d\u003d 1:"},{"line_number":201,"context_line":"                    ring_data \u003d cls.deserialize_v1("},{"line_number":202,"context_line":"                        gz_file, metadata_only\u003dmetadata_only)"},{"line_number":203,"context_line":"                else:"},{"line_number":204,"context_line":"                    raise Exception(\u0027Unknown ring format version %d\u0027 %"},{"line_number":205,"context_line":"                                    format_version)"}],"source_content_type":"text/x-python","patch_set":1,"id":"77efeb46_9dd39cee","line":202,"updated":"2023-04-27 15:19:15.000000000","message":"yeah I don\u0027t think we call `__init__` under here either?!","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cf6771dde65215e302086692daa5618b4dfdff38","unresolved":true,"context_lines":[{"line_number":199,"context_line":"                format_version, \u003d struct.unpack(\u0027!H\u0027, gz_file.read(2))"},{"line_number":200,"context_line":"                if format_version \u003d\u003d 1:"},{"line_number":201,"context_line":"                    ring_data \u003d cls.deserialize_v1("},{"line_number":202,"context_line":"                        gz_file, metadata_only\u003dmetadata_only)"},{"line_number":203,"context_line":"                else:"},{"line_number":204,"context_line":"                    raise Exception(\u0027Unknown ring format version %d\u0027 %"},{"line_number":205,"context_line":"                                    format_version)"}],"source_content_type":"text/x-python","patch_set":1,"id":"4f5549cb_6ca3a8c3","line":202,"in_reply_to":"77efeb46_9dd39cee","updated":"2023-04-27 16:36:37.000000000","message":"`deserialize_v1` returns a dict, so `hasattr(ring_data, \u0027devs\u0027)` is false, so we call `RingData.__init__` below.","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"61d23e9bf3f2d7646f924f50045a6fd7d7caf233","unresolved":true,"context_lines":[{"line_number":215,"context_line":"            ring_data \u003d RingData(ring_data[\u0027replica2part2dev_id\u0027],"},{"line_number":216,"context_line":"                                 ring_data[\u0027devs\u0027], ring_data[\u0027part_shift\u0027],"},{"line_number":217,"context_line":"                                 ring_data.get(\u0027next_part_power\u0027),"},{"line_number":218,"context_line":"                                 ring_data.get(\u0027version\u0027))"},{"line_number":219,"context_line":"        for attr in (\u0027md5\u0027, \u0027size\u0027, \u0027raw_size\u0027):"},{"line_number":220,"context_line":"            setattr(ring_data, attr, getattr(gz_file, attr))"},{"line_number":221,"context_line":"        return ring_data"}],"source_content_type":"text/x-python","patch_set":1,"id":"24bcd5cf_d4cbc33e","line":218,"updated":"2023-04-27 15:19:15.000000000","message":"oh, i\u0027m a little surprised we don\u0027t always call `RingData.__init__`","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cf6771dde65215e302086692daa5618b4dfdff38","unresolved":true,"context_lines":[{"line_number":215,"context_line":"            ring_data \u003d RingData(ring_data[\u0027replica2part2dev_id\u0027],"},{"line_number":216,"context_line":"                                 ring_data[\u0027devs\u0027], ring_data[\u0027part_shift\u0027],"},{"line_number":217,"context_line":"                                 ring_data.get(\u0027next_part_power\u0027),"},{"line_number":218,"context_line":"                                 ring_data.get(\u0027version\u0027))"},{"line_number":219,"context_line":"        for attr in (\u0027md5\u0027, \u0027size\u0027, \u0027raw_size\u0027):"},{"line_number":220,"context_line":"            setattr(ring_data, attr, getattr(gz_file, attr))"},{"line_number":221,"context_line":"        return ring_data"}],"source_content_type":"text/x-python","patch_set":1,"id":"c0242f53_88965c2a","line":218,"in_reply_to":"24bcd5cf_d4cbc33e","updated":"2023-04-27 16:36:37.000000000","message":"When we\u0027re just unpickling a real `RingData` object (rather than some massaged result of `RingData.to_dict`), pickle pops that guy into existence without calling `__init__` -- see also https://github.com/openstack/swift/commit/fc6391ea5c623c4c94ef37cf7c322b621b24313d\n\nMaybe we should just drop support for such old rings, though?","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"74ef46828b43315411c6909a1d1df2417e5556ed","unresolved":true,"context_lines":[{"line_number":215,"context_line":"            ring_data \u003d RingData(ring_data[\u0027replica2part2dev_id\u0027],"},{"line_number":216,"context_line":"                                 ring_data[\u0027devs\u0027], ring_data[\u0027part_shift\u0027],"},{"line_number":217,"context_line":"                                 ring_data.get(\u0027next_part_power\u0027),"},{"line_number":218,"context_line":"                                 ring_data.get(\u0027version\u0027))"},{"line_number":219,"context_line":"        for attr in (\u0027md5\u0027, \u0027size\u0027, \u0027raw_size\u0027):"},{"line_number":220,"context_line":"            setattr(ring_data, attr, getattr(gz_file, attr))"},{"line_number":221,"context_line":"        return ring_data"}],"source_content_type":"text/x-python","patch_set":1,"id":"6c6203c7_16121cdb","line":218,"in_reply_to":"c0242f53_88965c2a","updated":"2023-04-28 15:21:34.000000000","message":"I think you\u0027re addressing why it would be *incorrect* to *not* call normalize when we don\u0027t go through `__init__`\n\nI think Pete and I were asking about \"how do we make it obvious normalization always happens\" and my thinking was 1) only do normalization in one place 2) make sure all ring loading obviously flows through that place.\n\nSo either move normalization on all ring formats into load, or push old-ring initialization through `__init__` - or we have to do it in two places and convicne ourselves that is sufficient and will be maintained correctly.","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"61d23e9bf3f2d7646f924f50045a6fd7d7caf233","unresolved":true,"context_lines":[{"line_number":309,"context_line":"    def _reload(self, force\u003dFalse):"},{"line_number":310,"context_line":"        self._rtime \u003d time() + self.reload_time"},{"line_number":311,"context_line":"        if force or self.has_changed():"},{"line_number":312,"context_line":"            ring_data \u003d RingData.load(self.serialized_path)"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"            try:"},{"line_number":315,"context_line":"                self._validation_hook(ring_data)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c900798f_1050b3ff","line":312,"updated":"2023-04-27 15:19:15.000000000","message":"I was expecting reload normalization would still happens (but earlier now) aytime the RingData changes","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"cf6771dde65215e302086692daa5618b4dfdff38","unresolved":true,"context_lines":[{"line_number":309,"context_line":"    def _reload(self, force\u003dFalse):"},{"line_number":310,"context_line":"        self._rtime \u003d time() + self.reload_time"},{"line_number":311,"context_line":"        if force or self.has_changed():"},{"line_number":312,"context_line":"            ring_data \u003d RingData.load(self.serialized_path)"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"            try:"},{"line_number":315,"context_line":"                self._validation_hook(ring_data)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f06059a_073fcadc","line":312,"in_reply_to":"c900798f_1050b3ff","updated":"2023-04-27 16:36:37.000000000","message":"Yes, the normalization all happens by the time we return from `RingData.load` now.","commit_id":"b8f0a0ed5cf9dc79ed29073c15c9996cb55e4efb"}]}