)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5e61eefa3e8f9bf24df1a93f2e177eebaa6062ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"47221d7b_01d51819","updated":"2021-10-15 17:23:47.000000000","message":"Hmm... I just realized this patch hasn\u0027t touched composite builders at all... which seem most likely to benefit from wider device IDs.","commit_id":"73c17515db7a08cc800106a4b54535512effac19"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"515359021dbefc8be11948284fa34e0a2ad2f37a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fd9ea5fc_db2a882b","updated":"2021-10-15 16:27:09.000000000","message":"I\u0027m pretty familiar with this change at this point and I think it\u0027s pretty solid\n\nHowever, I\u0027m not sure we should merge it until we\u0027re ready to merge the ring/builder vary byte-id changes?\n\nhere\u0027s some other stuff that might be helpful:\n\nhttps://review.opendev.org/c/openstack/swift/+/814196 Add ringbuilder CLI tests [NEW]        \nhttps://review.opendev.org/c/openstack/swift/+/814197 Test increase part power with v2 [NEW]        \n","commit_id":"73c17515db7a08cc800106a4b54535512effac19"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"50e0832263597e08595dd85e13c235ff0d0e0d79","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9406b411_488c60f8","updated":"2021-10-13 23:22:20.000000000","message":"Not a problem, just something to think about if  https://review.opendev.org/c/openstack/swift/+/811833 goes forward as it is.","commit_id":"73c17515db7a08cc800106a4b54535512effac19"}],"swift/cli/ringbuilder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":1311,"context_line":"            print(\"Defaulting to --format-version\u003d1. This ensures the ring\\n\""},{"line_number":1312,"context_line":"                  \"written will be readable by older versions of Swift.\\n\""},{"line_number":1313,"context_line":"                  \"In a future release, the default will change to\\n\""},{"line_number":1314,"context_line":"                  \"--format-version\u003d2\\n\")"},{"line_number":1315,"context_line":"            options.format_version \u003d DEFAULT_RING_FORMAT_VERSION"},{"line_number":1316,"context_line":"        else:"},{"line_number":1317,"context_line":"            # N.B. choices doesn\u0027t work with type\u003dint"}],"source_content_type":"text/x-python","patch_set":1,"id":"58105c5d_d4dd35fa","line":1314,"updated":"2021-09-13 18:22:45.000000000","message":"maybe by the time we get around to changing this we\u0027ll have refactored all this to a single source of truth - so we don\u0027t have changed the constant over in another file, and then the copy in two different places.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":1311,"context_line":"            print(\"Defaulting to --format-version\u003d1. This ensures the ring\\n\""},{"line_number":1312,"context_line":"                  \"written will be readable by older versions of Swift.\\n\""},{"line_number":1313,"context_line":"                  \"In a future release, the default will change to\\n\""},{"line_number":1314,"context_line":"                  \"--format-version\u003d2\\n\")"},{"line_number":1315,"context_line":"            options.format_version \u003d DEFAULT_RING_FORMAT_VERSION"},{"line_number":1316,"context_line":"        else:"},{"line_number":1317,"context_line":"            # N.B. choices doesn\u0027t work with type\u003dint"}],"source_content_type":"text/x-python","patch_set":1,"id":"d002e817_fda2fd81","line":1314,"in_reply_to":"58105c5d_d4dd35fa","updated":"2021-09-13 21:29:06.000000000","message":"By the time we change this, I expect we\u0027ll just drop the copy entirely. May even drop/ignore the option and *only* write v2 rings, similar to how we no longer provide a way to write pickled rings.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b19c825140dfe8910571bfab1902cfeabfcd0523","unresolved":true,"context_lines":[{"line_number":1301,"context_line":"    \u0027set_info\u0027 calls when no rebalance is needed but you want to send out the"},{"line_number":1302,"context_line":"    new device information."},{"line_number":1303,"context_line":"        \"\"\""},{"line_number":1304,"context_line":"        usage \u003d Commands.rebalance.__doc__.strip()"},{"line_number":1305,"context_line":"        parser \u003d optparse.OptionParser(usage)"},{"line_number":1306,"context_line":"        parser.add_option(\u0027--format-version\u0027,"},{"line_number":1307,"context_line":"                          choices\u003dFORMAT_CHOICES, default\u003dNone,"}],"source_content_type":"text/x-python","patch_set":5,"id":"86b71c7d_07b06683","line":1304,"updated":"2021-09-29 04:31:38.000000000","message":"should be: \n  usage \u003d Commands.write_ring.__doc__.strip()","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bcb3603deb86486287dc69da7c8e655a8fff8e2e","unresolved":false,"context_lines":[{"line_number":1301,"context_line":"    \u0027set_info\u0027 calls when no rebalance is needed but you want to send out the"},{"line_number":1302,"context_line":"    new device information."},{"line_number":1303,"context_line":"        \"\"\""},{"line_number":1304,"context_line":"        usage \u003d Commands.rebalance.__doc__.strip()"},{"line_number":1305,"context_line":"        parser \u003d optparse.OptionParser(usage)"},{"line_number":1306,"context_line":"        parser.add_option(\u0027--format-version\u0027,"},{"line_number":1307,"context_line":"                          choices\u003dFORMAT_CHOICES, default\u003dNone,"}],"source_content_type":"text/x-python","patch_set":5,"id":"d9b62aa0_06034a62","line":1304,"in_reply_to":"86b71c7d_07b06683","updated":"2021-10-12 19:16:16.000000000","message":"Done","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b19c825140dfe8910571bfab1902cfeabfcd0523","unresolved":true,"context_lines":[{"line_number":1316,"context_line":"        else:"},{"line_number":1317,"context_line":"            # N.B. choices doesn\u0027t work with type\u003dint"},{"line_number":1318,"context_line":"            options.format_version \u003d int(options.format_version)"},{"line_number":1319,"context_line":""},{"line_number":1320,"context_line":"        if not builder.devs:"},{"line_number":1321,"context_line":"            print(\u0027Unable to write empty ring.\u0027)"},{"line_number":1322,"context_line":"            exit(EXIT_ERROR)"}],"source_content_type":"text/x-python","patch_set":5,"id":"be2c1cbd_40b860af","line":1319,"updated":"2021-09-29 04:31:38.000000000","message":"Shame we need to have the same bit of code twice, but understand why we do. It\u0027s interesting that we create seperate OptionParsers for each command and not use a global with sub parsers. But feel that\u0027s out of the scope. Something we could put up as a low_hanging_fruit maybe.","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bcb3603deb86486287dc69da7c8e655a8fff8e2e","unresolved":false,"context_lines":[{"line_number":1316,"context_line":"        else:"},{"line_number":1317,"context_line":"            # N.B. choices doesn\u0027t work with type\u003dint"},{"line_number":1318,"context_line":"            options.format_version \u003d int(options.format_version)"},{"line_number":1319,"context_line":""},{"line_number":1320,"context_line":"        if not builder.devs:"},{"line_number":1321,"context_line":"            print(\u0027Unable to write empty ring.\u0027)"},{"line_number":1322,"context_line":"            exit(EXIT_ERROR)"}],"source_content_type":"text/x-python","patch_set":5,"id":"8223ddf2_436f43ac","line":1319,"in_reply_to":"be2c1cbd_40b860af","updated":"2021-10-12 19:16:16.000000000","message":"Ack","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"515359021dbefc8be11948284fa34e0a2ad2f37a","unresolved":true,"context_lines":[{"line_number":1053,"context_line":"            print(\"Defaulting to --format-version\u003d1. This ensures the ring\\n\""},{"line_number":1054,"context_line":"                  \"written will be readable by older versions of Swift.\\n\""},{"line_number":1055,"context_line":"                  \"In a future release, the default will change to\\n\""},{"line_number":1056,"context_line":"                  \"--format-version\u003d2\\n\")"},{"line_number":1057,"context_line":"            options.format_version \u003d DEFAULT_RING_FORMAT_VERSION"},{"line_number":1058,"context_line":"        else:"},{"line_number":1059,"context_line":"            # N.B. choices doesn\u0027t work with type\u003dint"}],"source_content_type":"text/x-python","patch_set":7,"id":"4fe40497_960ba4ac","line":1056,"updated":"2021-10-15 16:27:09.000000000","message":"there doesn\u0027t seem to be any assertions around this copy - which makes refactoring harder.","commit_id":"73c17515db7a08cc800106a4b54535512effac19"}],"swift/common/ring/ring.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":37,"context_line":"from swift.common.ring.utils import tiers_for_dev, BYTES_TO_TYPE_CODE"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"DEFAULT_RING_FORMAT_VERSION \u003d 1"},{"line_number":41,"context_line":"ALLOWED_RING_FORMAT_VERSIONS \u003d (1, 2)"},{"line_number":42,"context_line":"DEFAULT_RELOAD_TIME \u003d 15"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bd4d09b6_7e0e256b","line":40,"updated":"2021-09-13 18:22:45.000000000","message":"+1","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":82,"context_line":"        data \u003d self._fp.read(amt)"},{"line_number":83,"context_line":"        self._remaining -\u003d len(data)"},{"line_number":84,"context_line":"        if len(data) !\u003d amt:"},{"line_number":85,"context_line":"            raise ValueError(\u0027Bad read: expected %d bytes but got %d\u0027"},{"line_number":86,"context_line":"                             % (amt, len(data)))"},{"line_number":87,"context_line":"        return data"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b1a4eaf7_f99307b3","line":85,"updated":"2021-09-13 18:22:45.000000000","message":"is it never appropriate for _fp.read to return less than amt?  why do we think we need this check here, wouldn\u0027t we blow up with _remaining in close if they stopped reading","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":82,"context_line":"        data \u003d self._fp.read(amt)"},{"line_number":83,"context_line":"        self._remaining -\u003d len(data)"},{"line_number":84,"context_line":"        if len(data) !\u003d amt:"},{"line_number":85,"context_line":"            raise ValueError(\u0027Bad read: expected %d bytes but got %d\u0027"},{"line_number":86,"context_line":"                             % (amt, len(data)))"},{"line_number":87,"context_line":"        return data"},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5df00635_9d02e872","line":85,"in_reply_to":"b1a4eaf7_f99307b3","updated":"2021-09-13 21:29:06.000000000","message":"Originally I didn\u0027t have the close and even once I added it, I wasn\u0027t real good about making sure I called it. Could probably drop it now.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def close(self):"},{"line_number":90,"context_line":"        if self._remaining:"},{"line_number":91,"context_line":"            raise ValueError(\u0027Excess data in buffer: %d bytes remaining\u0027"},{"line_number":92,"context_line":"                             % self._remaining)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5c4a0e58_fa20ded8","line":91,"updated":"2021-09-13 18:22:45.000000000","message":"I don\u0027t see a \"buffer\" here","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def close(self):"},{"line_number":90,"context_line":"        if self._remaining:"},{"line_number":91,"context_line":"            raise ValueError(\u0027Excess data in buffer: %d bytes remaining\u0027"},{"line_number":92,"context_line":"                             % self._remaining)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"4068b2be_d1e4ee21","line":91,"in_reply_to":"5c4a0e58_fa20ded8","updated":"2021-09-13 21:29:06.000000000","message":"\"block\" better?","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":271,"context_line":"                if format_version \u003d\u003d 1:"},{"line_number":272,"context_line":"                    ring_data \u003d cls.deserialize_v1("},{"line_number":273,"context_line":"                        gz_file, metadata_only\u003dmetadata_only)"},{"line_number":274,"context_line":"                elif format_version \u003d\u003d 2:"},{"line_number":275,"context_line":"                    ring_data \u003d cls.deserialize_v2("},{"line_number":276,"context_line":"                        gz_file, metadata_only\u003dmetadata_only)"},{"line_number":277,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1be8b6a2_2421b567","line":274,"updated":"2021-09-13 18:22:45.000000000","message":"maybe let\u0027s always do the lookup map:\n\nself.ring_format[version][\u0027serialize\u0027]","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":294,"context_line":"    def serialize_v1(self, file_obj):"},{"line_number":295,"context_line":"        # Write out new-style serialization magic and version:"},{"line_number":296,"context_line":"        file_obj.write(struct.pack(\u0027!4sH\u0027, b\u0027R1NG\u0027, 1))"},{"line_number":297,"context_line":"        ring \u003d self.to_dict()"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":300,"context_line":"        # builder, otherwise just ignore it"}],"source_content_type":"text/x-python","patch_set":1,"id":"09ce1e50_b6a003f0","line":297,"updated":"2021-09-13 18:22:45.000000000","message":"so v1 ring will also get dev_id_bytes key; but will ignore it","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":294,"context_line":"    def serialize_v1(self, file_obj):"},{"line_number":295,"context_line":"        # Write out new-style serialization magic and version:"},{"line_number":296,"context_line":"        file_obj.write(struct.pack(\u0027!4sH\u0027, b\u0027R1NG\u0027, 1))"},{"line_number":297,"context_line":"        ring \u003d self.to_dict()"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        # Only include next_part_power if it is set in the"},{"line_number":300,"context_line":"        # builder, otherwise just ignore it"}],"source_content_type":"text/x-python","patch_set":1,"id":"1bf369dc_3c749c37","line":297,"in_reply_to":"09ce1e50_b6a003f0","updated":"2021-09-13 21:29:06.000000000","message":"What do we mean by \"v1 ring\"? The way I\u0027ve been thinking about it,\n\n* RingData has no version -- it\u0027s just a data structure that can change and evolve over time. I guess if I *had* to put a version on it, it\u0027s whatever the Swift version is that you\u0027re currently running.\n* *.ring.gz files have versions for different serialization formats. Not all formats are capable of storing all data that might be hanging off a RingData.\n\nSo yeah, the RingData.to_dict() result has a dev_id_bytes key, but no v1 ring.gz file will, since we don\u0027t copy it to _text.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":300,"context_line":"        # builder, otherwise just ignore it"},{"line_number":301,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":302,"context_line":"                 \u0027replica_count\u0027: len(ring[\u0027replica2part2dev_id\u0027]),"},{"line_number":303,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        if ring[\u0027version\u0027] is not None:"},{"line_number":306,"context_line":"            _text[\u0027version\u0027] \u003d ring[\u0027version\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3db3eb32_4497dc95","line":303,"updated":"2021-09-13 18:22:45.000000000","message":"v2 never gets a byteorder key, and always writes/reads the same way regardless of platform","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":300,"context_line":"        # builder, otherwise just ignore it"},{"line_number":301,"context_line":"        _text \u003d {\u0027devs\u0027: ring[\u0027devs\u0027], \u0027part_shift\u0027: ring[\u0027part_shift\u0027],"},{"line_number":302,"context_line":"                 \u0027replica_count\u0027: len(ring[\u0027replica2part2dev_id\u0027]),"},{"line_number":303,"context_line":"                 \u0027byteorder\u0027: sys.byteorder}"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        if ring[\u0027version\u0027] is not None:"},{"line_number":306,"context_line":"            _text[\u0027version\u0027] \u003d ring[\u0027version\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"e9395cda_2a070ea8","line":303,"in_reply_to":"3db3eb32_4497dc95","updated":"2021-09-13 21:29:06.000000000","message":"IMHO, it was a bug that we ever wrote it down in a platform-dependent manner -- but by the time we noticed it, it was too late to do anything other than write down which order we used.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":342,"context_line":"        json_text \u003d json.dumps(_text, sort_keys\u003dTrue,"},{"line_number":343,"context_line":"                               ensure_ascii\u003dTrue).encode(\u0027ascii\u0027)"},{"line_number":344,"context_line":"        json_len \u003d len(json_text)"},{"line_number":345,"context_line":"        file_obj.write(struct.pack(\u0027!Q\u0027, json_len))"},{"line_number":346,"context_line":"        file_obj.write(json_text)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        assignments \u003d sum(len(a) for a in ring[\u0027replica2part2dev_id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"a3f27539_84aebba2","line":345,"updated":"2021-09-13 18:22:45.000000000","message":"a Q is like 8 bytes, so our max len here is 2 ** 64 - which is huge - so that should be enough for all the metadata we want to write.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":342,"context_line":"        json_text \u003d json.dumps(_text, sort_keys\u003dTrue,"},{"line_number":343,"context_line":"                               ensure_ascii\u003dTrue).encode(\u0027ascii\u0027)"},{"line_number":344,"context_line":"        json_len \u003d len(json_text)"},{"line_number":345,"context_line":"        file_obj.write(struct.pack(\u0027!Q\u0027, json_len))"},{"line_number":346,"context_line":"        file_obj.write(json_text)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        assignments \u003d sum(len(a) for a in ring[\u0027replica2part2dev_id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"111a4036_6e6a281a","line":345,"in_reply_to":"a3f27539_84aebba2","updated":"2021-09-13 21:29:06.000000000","message":"From a comment I left on https://review.opendev.org/c/openstack/swift/+/803665 while thinking about a 32-bit length here:\n\n\u003e As for the possibility of \u003cJSON length\u003e overflowing 32 bits, it looks like each dev in my saio adds ~170 bytes to the JSON metadata -- let\u0027s round it up to 200 to accommodate IPv6 (and for rounder numbers). Leaving some space for non-devs metadata, let\u0027s say we\u0027ve got ~4B bytes to play with -- so we\u0027d expect to be able to handle ~20M devices. I think we\u0027ll need to rethink a lot of things before we get close to that limit.\n\u003e \n\u003e OTOH, I could see an argument that it\u0027d be nice to standardize on 64-bit lengths so we can continue parsing in the face of unrecognized types. Though presumably we\u0027d want to order data in the ring by type code, so encountering an unknown type would be an indication that you can just halt parsing since there\u0027s nothing left you understand.\n\nI went with the \"64-bit length everywhere\" idea because those extra four bytes are nothing compared to the size of even pretty tiny rings.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":346,"context_line":"        file_obj.write(json_text)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        assignments \u003d sum(len(a) for a in ring[\u0027replica2part2dev_id\u0027])"},{"line_number":349,"context_line":"        file_obj.write(struct.pack(\u0027!Q\u0027, assignments * ring[\u0027dev_id_bytes\u0027]))"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        for part2dev_id in ring[\u0027replica2part2dev_id\u0027]:"},{"line_number":352,"context_line":"            with network_order_array(part2dev_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d36b31a9_5c660fc7","line":349,"updated":"2021-09-13 18:22:45.000000000","message":"assignments is like the \"number of parts\" in row summed up, so you can skip the whole table, but not a single row","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":346,"context_line":"        file_obj.write(json_text)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        assignments \u003d sum(len(a) for a in ring[\u0027replica2part2dev_id\u0027])"},{"line_number":349,"context_line":"        file_obj.write(struct.pack(\u0027!Q\u0027, assignments * ring[\u0027dev_id_bytes\u0027]))"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        for part2dev_id in ring[\u0027replica2part2dev_id\u0027]:"},{"line_number":352,"context_line":"            with network_order_array(part2dev_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"84c597eb_bce08db4","line":349,"in_reply_to":"d36b31a9_5c660fc7","updated":"2021-09-13 21:29:06.000000000","message":"🤔 Got composite rings in mind, or something? Have an on-disk ring file that *only* knows about replica 1 and not 0 or 2?\n\nI feel like we\u0027d want to check for IndexErrors a lot more when doing lookups...","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":382,"context_line":"        {"},{"line_number":383,"context_line":"            1: self.serialize_v1,"},{"line_number":384,"context_line":"            2: self.serialize_v2,"},{"line_number":385,"context_line":"        }[format_version](gz_file)"},{"line_number":386,"context_line":"        gz_file.close()"},{"line_number":387,"context_line":"        tempf.flush()"},{"line_number":388,"context_line":"        os.fsync(tempf.fileno())"}],"source_content_type":"text/x-python","patch_set":1,"id":"3541f7c3_2fb19bdd","line":385,"updated":"2021-09-13 18:22:45.000000000","message":"Maybe let\u0027s always do if/elif chain?","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":382,"context_line":"        {"},{"line_number":383,"context_line":"            1: self.serialize_v1,"},{"line_number":384,"context_line":"            2: self.serialize_v2,"},{"line_number":385,"context_line":"        }[format_version](gz_file)"},{"line_number":386,"context_line":"        gz_file.close()"},{"line_number":387,"context_line":"        tempf.flush()"},{"line_number":388,"context_line":"        os.fsync(tempf.fileno())"}],"source_content_type":"text/x-python","patch_set":1,"id":"863dd225_576e023b","line":385,"in_reply_to":"3541f7c3_2fb19bdd","updated":"2021-09-13 21:29:06.000000000","message":"*shrug* Either way.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":395,"context_line":"                \u0027replica2part2dev_id\u0027: self._replica2part2dev_id,"},{"line_number":396,"context_line":"                \u0027part_shift\u0027: self._part_shift,"},{"line_number":397,"context_line":"                \u0027next_part_power\u0027: self.next_part_power,"},{"line_number":398,"context_line":"                \u0027dev_id_bytes\u0027: 2,"},{"line_number":399,"context_line":"                \u0027version\u0027: self.version}"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"0e82e119_904d79bb","line":398,"updated":"2021-09-13 18:22:45.000000000","message":"maybe this should be added in the serialize_v2 method","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":395,"context_line":"                \u0027replica2part2dev_id\u0027: self._replica2part2dev_id,"},{"line_number":396,"context_line":"                \u0027part_shift\u0027: self._part_shift,"},{"line_number":397,"context_line":"                \u0027next_part_power\u0027: self.next_part_power,"},{"line_number":398,"context_line":"                \u0027dev_id_bytes\u0027: 2,"},{"line_number":399,"context_line":"                \u0027version\u0027: self.version}"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"d7e7ca80_8fd9a5a1","line":398,"in_reply_to":"0e82e119_904d79bb","updated":"2021-09-13 21:29:06.000000000","message":"I mean, ultimately, it\u0027s gonna be a property of the RingData -- maybe I should bring that change from the next patchset into this one.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b19c825140dfe8910571bfab1902cfeabfcd0523","unresolved":true,"context_lines":[{"line_number":222,"context_line":"        dictionary just has the value `[]`."},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        :param file gz_file: An opened file-like object which has already"},{"line_number":225,"context_line":"                             consumed the 6 bytes of magic and version."},{"line_number":226,"context_line":"        :param bool metadata_only: If True, only load `devs` and `part_shift`"},{"line_number":227,"context_line":"        :returns: A dict containing `devs`, `part_shift`, and"},{"line_number":228,"context_line":"                  `replica2part2dev_id`"}],"source_content_type":"text/x-python","patch_set":5,"id":"2510e826_06fecae4","line":225,"updated":"2021-09-29 04:31:38.000000000","message":"I understand that we pull off the magic and then the version which are the first 6 bytes. But when contrasting serialize_vX and deserialize_vX they aren\u0027t really the opposite of each other. The serialize code does all the serialization where as deserialise does only deserializes after the 6 bytes.. just doesn\u0027t feel properly symmetric. \n\nKinda makes me want to have deserialize to it all and just seek(0) which passing the file in, or have a skip_version if you want to start at byte 6.\nI guess it\u0027s always been this way so can live with it as it is.. just doesn\u0027t feel symmetric.","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bcb3603deb86486287dc69da7c8e655a8fff8e2e","unresolved":true,"context_lines":[{"line_number":222,"context_line":"        dictionary just has the value `[]`."},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        :param file gz_file: An opened file-like object which has already"},{"line_number":225,"context_line":"                             consumed the 6 bytes of magic and version."},{"line_number":226,"context_line":"        :param bool metadata_only: If True, only load `devs` and `part_shift`"},{"line_number":227,"context_line":"        :returns: A dict containing `devs`, `part_shift`, and"},{"line_number":228,"context_line":"                  `replica2part2dev_id`"}],"source_content_type":"text/x-python","patch_set":5,"id":"66e48435_6b9e004b","line":225,"in_reply_to":"2510e826_06fecae4","updated":"2021-10-12 19:16:16.000000000","message":"Yeah, it\u0027s been bugging me in this series, too. Makes me wonder if the fix ought to be to have save() write out the magic and version...","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b19c825140dfe8910571bfab1902cfeabfcd0523","unresolved":true,"context_lines":[{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        json_len, \u003d struct.unpack(\u0027!Q\u0027, gz_file.read(8))"},{"line_number":232,"context_line":"        with contextlib.closing(FixedLengthReader(gz_file, json_len)) as data:"},{"line_number":233,"context_line":"            ring_dict \u003d json.load(data)"},{"line_number":234,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bf8220b0_958b5aad","line":232,"updated":"2021-09-29 04:31:38.000000000","message":"Seeing as we have control of the FixedLengthReader class we could just implement the __enter__ and __exit__ methods and build in closing ourself and make this simpler, but meh.","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bcb3603deb86486287dc69da7c8e655a8fff8e2e","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"        json_len, \u003d struct.unpack(\u0027!Q\u0027, gz_file.read(8))"},{"line_number":232,"context_line":"        with contextlib.closing(FixedLengthReader(gz_file, json_len)) as data:"},{"line_number":233,"context_line":"            ring_dict \u003d json.load(data)"},{"line_number":234,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"859c0f31_061fa6a3","line":232,"in_reply_to":"bf8220b0_958b5aad","updated":"2021-10-12 19:16:16.000000000","message":"Done","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b19c825140dfe8910571bfab1902cfeabfcd0523","unresolved":true,"context_lines":[{"line_number":233,"context_line":"            ring_dict \u003d json.load(data)"},{"line_number":234,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        partition_count \u003d 1 \u003c\u003c (32 - ring_dict[\u0027part_shift\u0027])"},{"line_number":237,"context_line":"        max_row_len \u003d ring_dict[\u0027dev_id_bytes\u0027] * partition_count"},{"line_number":238,"context_line":"        type_code \u003d BYTES_TO_TYPE_CODE[ring_dict[\u0027dev_id_bytes\u0027]]"},{"line_number":239,"context_line":"        if metadata_only:"},{"line_number":240,"context_line":"            return ring_dict"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1ccfd61b_4c1cc813","line":238,"range":{"start_line":236,"start_character":8,"end_line":238,"end_character":65},"updated":"2021-09-29 04:31:38.000000000","message":"These can be moved to below the \u0027if metadata_only:\u0027 line, as needless work if only returning metadata.","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bcb3603deb86486287dc69da7c8e655a8fff8e2e","unresolved":true,"context_lines":[{"line_number":233,"context_line":"            ring_dict \u003d json.load(data)"},{"line_number":234,"context_line":"        ring_dict[\u0027replica2part2dev_id\u0027] \u003d []"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        partition_count \u003d 1 \u003c\u003c (32 - ring_dict[\u0027part_shift\u0027])"},{"line_number":237,"context_line":"        max_row_len \u003d ring_dict[\u0027dev_id_bytes\u0027] * partition_count"},{"line_number":238,"context_line":"        type_code \u003d BYTES_TO_TYPE_CODE[ring_dict[\u0027dev_id_bytes\u0027]]"},{"line_number":239,"context_line":"        if metadata_only:"},{"line_number":240,"context_line":"            return ring_dict"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"2ccc9f18_3d16c9b4","line":238,"range":{"start_line":236,"start_character":8,"end_line":238,"end_character":65},"in_reply_to":"1ccfd61b_4c1cc813","updated":"2021-10-12 19:16:16.000000000","message":"*shrug* Way I figure it, we get some cheap validation this way:\n\n* part_shift is an integer in [0, 32]\n* dev_id_bytes is in BYTES_TO_TYPE_CODE\n\nI don\u0027t have any real strong feelings, though.","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b19c825140dfe8910571bfab1902cfeabfcd0523","unresolved":true,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param filename: File into which this instance should be serialized."},{"line_number":368,"context_line":"        :param mtime: time used to override mtime for gzip, default or None"},{"line_number":369,"context_line":"                      if the caller wants to include time"},{"line_number":370,"context_line":"        \"\"\""},{"line_number":371,"context_line":"        if format_version not in ALLOWED_RING_FORMAT_VERSIONS:"},{"line_number":372,"context_line":"            raise ValueError(\u0027format_version must be one of %r\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"e3eec818_db9a6582","line":369,"updated":"2021-09-29 04:31:38.000000000","message":"format_version in doc string","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bcb3603deb86486287dc69da7c8e655a8fff8e2e","unresolved":false,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param filename: File into which this instance should be serialized."},{"line_number":368,"context_line":"        :param mtime: time used to override mtime for gzip, default or None"},{"line_number":369,"context_line":"                      if the caller wants to include time"},{"line_number":370,"context_line":"        \"\"\""},{"line_number":371,"context_line":"        if format_version not in ALLOWED_RING_FORMAT_VERSIONS:"},{"line_number":372,"context_line":"            raise ValueError(\u0027format_version must be one of %r\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"c2635ee8_a183cc94","line":369,"in_reply_to":"e3eec818_db9a6582","updated":"2021-10-12 19:16:16.000000000","message":"Done","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"515359021dbefc8be11948284fa34e0a2ad2f37a","unresolved":true,"context_lines":[{"line_number":70,"context_line":"    return arr"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"class FixedLengthReader(object):"},{"line_number":74,"context_line":"    def __init__(self, fp, length):"},{"line_number":75,"context_line":"        self._fp \u003d fp"},{"line_number":76,"context_line":"        self._remaining \u003d length"}],"source_content_type":"text/x-python","patch_set":7,"id":"0ec563a8_a6ca8b76","line":73,"updated":"2021-10-15 16:27:09.000000000","message":"docstrings would be helpful here","commit_id":"73c17515db7a08cc800106a4b54535512effac19"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"50e0832263597e08595dd85e13c235ff0d0e0d79","unresolved":true,"context_lines":[{"line_number":249,"context_line":"        max_row_len \u003d ring_dict[\u0027dev_id_bytes\u0027] * partition_count"},{"line_number":250,"context_line":"        type_code \u003d BYTES_TO_TYPE_CODE[ring_dict[\u0027dev_id_bytes\u0027]]"},{"line_number":251,"context_line":"        if metadata_only:"},{"line_number":252,"context_line":"            return ring_dict"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"        data_len, \u003d struct.unpack(\u0027!Q\u0027, gz_file.read(8))"},{"line_number":255,"context_line":"        with FixedLengthReader(gz_file, data_len) as data:"}],"source_content_type":"text/x-python","patch_set":7,"id":"39ee3232_5ecce7e5","line":252,"updated":"2021-10-13 23:22:20.000000000","message":"If we take https://review.opendev.org/c/openstack/swift/+/811833 as it currenlty stands, we actually need to replica_count in the metdata. So we have 2 options. We can just add it to serialize_v2 as a float (just because that makes more sense) or we can just calculate it here, something like:\n\n  data_len, \u003d struct.unpack(\u0027!Q\u0027, gz_file.read(8))\n  ring_dict[\u0027replica_count\u0027] \u003d float(data_len // max_row_length + float(data_len % max_row_length) / max_row_length)\n  if metadata_only:\n      return ring_dict\n\n   with FixedLengthReader(gz_file, data_len) as data:\n       ...\n\n I guess option 1 is the easiest, it\u0027s only a single float so doesn\u0027t cost much.. but we could calulate the result and not take up any extra space (option 2).","commit_id":"73c17515db7a08cc800106a4b54535512effac19"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"515359021dbefc8be11948284fa34e0a2ad2f37a","unresolved":true,"context_lines":[{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        next_part_power \u003d ring.get(\u0027next_part_power\u0027)"},{"line_number":348,"context_line":"        if next_part_power is not None:"},{"line_number":349,"context_line":"            _text[\u0027next_part_power\u0027] \u003d next_part_power"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        json_text \u003d json.dumps(_text, sort_keys\u003dTrue,"},{"line_number":352,"context_line":"                               ensure_ascii\u003dTrue).encode(\u0027ascii\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"02329735_dffe9cbb","line":349,"updated":"2021-10-15 16:27:09.000000000","message":"untested","commit_id":"73c17515db7a08cc800106a4b54535512effac19"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"515359021dbefc8be11948284fa34e0a2ad2f37a","unresolved":true,"context_lines":[{"line_number":408,"context_line":"                \u0027replica2part2dev_id\u0027: self._replica2part2dev_id,"},{"line_number":409,"context_line":"                \u0027part_shift\u0027: self._part_shift,"},{"line_number":410,"context_line":"                \u0027next_part_power\u0027: self.next_part_power,"},{"line_number":411,"context_line":"                \u0027dev_id_bytes\u0027: 2,"},{"line_number":412,"context_line":"                \u0027version\u0027: self.version}"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"73192ec0_4bebdacd","line":411,"updated":"2021-10-15 16:27:09.000000000","message":"so even if we merge this w/o the follow-up this ensures that all v2 rings will always explicitly write down their dev_id_bytes - even if there\u0027s no way to make it anything other than 2 with this change","commit_id":"73c17515db7a08cc800106a4b54535512effac19"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5e61eefa3e8f9bf24df1a93f2e177eebaa6062ae","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                \u0027replica2part2dev_id\u0027: self._replica2part2dev_id,"},{"line_number":409,"context_line":"                \u0027part_shift\u0027: self._part_shift,"},{"line_number":410,"context_line":"                \u0027next_part_power\u0027: self.next_part_power,"},{"line_number":411,"context_line":"                \u0027dev_id_bytes\u0027: 2,"},{"line_number":412,"context_line":"                \u0027version\u0027: self.version}"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"49f80751_13f807a5","line":411,"in_reply_to":"73192ec0_4bebdacd","updated":"2021-10-15 17:23:47.000000000","message":"Yes -- and we\u0027re even able to *read* rings with dev_id_bytes \u003d 1 or 4 with this, as demonstrated in tests.","commit_id":"73c17515db7a08cc800106a4b54535512effac19"}],"swift/common/ring/utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":25,"context_line":"BYTES_TO_TYPE_CODE \u003d {"},{"line_number":26,"context_line":"    1: \u0027B\u0027,"},{"line_number":27,"context_line":"    2: \u0027H\u0027,"},{"line_number":28,"context_line":"    4: \u0027I\u0027,"},{"line_number":29,"context_line":"    # This just seems excessive, but may as well be complete"},{"line_number":30,"context_line":"    8: \u0027Q\u0027,"},{"line_number":31,"context_line":"}"}],"source_content_type":"text/x-python","patch_set":1,"id":"12ce913e_2e12c33e","line":28,"updated":"2021-09-13 18:22:45.000000000","message":"this is \"only\" 4 billion devices, is it \"possible\" to pack 3 byte integers?","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":25,"context_line":"BYTES_TO_TYPE_CODE \u003d {"},{"line_number":26,"context_line":"    1: \u0027B\u0027,"},{"line_number":27,"context_line":"    2: \u0027H\u0027,"},{"line_number":28,"context_line":"    4: \u0027I\u0027,"},{"line_number":29,"context_line":"    # This just seems excessive, but may as well be complete"},{"line_number":30,"context_line":"    8: \u0027Q\u0027,"},{"line_number":31,"context_line":"}"}],"source_content_type":"text/x-python","patch_set":1,"id":"22b1c366_1045acbb","line":28,"in_reply_to":"12ce913e_2e12c33e","updated":"2021-09-13 21:29:06.000000000","message":"Not really, not without doing it ourselves. I could imagine doing something fairly clever during serialization/deserialization to drop that leading b\u0027\\x00\u0027 from each dev, but I really don\u0027t have the stomach to roll my own array().\n\nLet\u0027s just let gzip do something smart instead.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1ddaf71e3e29166fc1736a6d4d5da8a4fe82e72d","unresolved":true,"context_lines":[{"line_number":25,"context_line":"BYTES_TO_TYPE_CODE \u003d {"},{"line_number":26,"context_line":"    1: \u0027B\u0027,"},{"line_number":27,"context_line":"    2: \u0027H\u0027,"},{"line_number":28,"context_line":"    4: \u0027I\u0027,"},{"line_number":29,"context_line":"    # This just seems excessive, but may as well be complete"},{"line_number":30,"context_line":"    8: \u0027Q\u0027,"},{"line_number":31,"context_line":"}"}],"source_content_type":"text/x-python","patch_set":1,"id":"bb238133_9cbe1d35","line":28,"in_reply_to":"22b1c366_1045acbb","updated":"2021-09-14 07:30:36.000000000","message":"you did mention in a chat that we could use the first bit for something like a sticky bit or we could use it for marking a device as enabled or not. That come up in an SRE meeting at work (I think from Clay), to be able to mark a device as disabled so it\u0027s still there but things (like proxies) know if it should go look elsewhere rather then relying entirely on error limited nodes.\n\nHowever, it\u0027ll be some extra code to maintain, but should just be something like:\n # map just the first bit: ie 1000\n enabled \u003d \u003cdev_id from ring\u003e | (1 \u003c\u003c (dev_length - 1))\n # All 1\u0027s then bitshift so it starts with a 0, ie 0111\n dev_bitmap \u003d (1 \u003c\u003c dev_length) -1 \u003e\u003e 1\n dev_id \u003d \u003cdev_if from ring\u003e \u0026 dev_bitmap\n\nNOTE: I might have over thought it a little :P","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"38296125b243fe721c1c8ed045740f289fac9b59","unresolved":true,"context_lines":[{"line_number":25,"context_line":"BYTES_TO_TYPE_CODE \u003d {"},{"line_number":26,"context_line":"    1: \u0027B\u0027,"},{"line_number":27,"context_line":"    2: \u0027H\u0027,"},{"line_number":28,"context_line":"    4: \u0027I\u0027,"},{"line_number":29,"context_line":"    # This just seems excessive, but may as well be complete"},{"line_number":30,"context_line":"    8: \u0027Q\u0027,"},{"line_number":31,"context_line":"}"}],"source_content_type":"text/x-python","patch_set":1,"id":"cbdab1d0_71ccd2d2","line":28,"in_reply_to":"bb238133_9cbe1d35","updated":"2021-09-14 08:04:44.000000000","message":"though wouldn\u0027t in both cases, sticky or enabled eaiser just to put something in the devices dicts.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5994f69022cb869c15d12d4cb07dd3d7112e9294","unresolved":true,"context_lines":[{"line_number":27,"context_line":"    2: \u0027H\u0027,"},{"line_number":28,"context_line":"    4: \u0027I\u0027,"},{"line_number":29,"context_line":"    # This just seems excessive, but may as well be complete"},{"line_number":30,"context_line":"    8: \u0027Q\u0027,"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fd262ef1_94cf6cdc","line":30,"updated":"2021-09-15 18:04:35.000000000","message":"py27 array.array() doesn\u0027t qupport \u0027Q\u0027 -- struct does, though!","commit_id":"f0063b625febb4b8fe47a89650a1e5b1dc074bb4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b19c825140dfe8910571bfab1902cfeabfcd0523","unresolved":true,"context_lines":[{"line_number":27,"context_line":"    2: \u0027H\u0027,"},{"line_number":28,"context_line":"    4: \u0027I\u0027,"},{"line_number":29,"context_line":"    # This just seems excessive; besides, array.array() only takes it on py33+"},{"line_number":30,"context_line":"    # 8: \u0027Q\u0027,"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba7ee7b9_f52a5db4","line":30,"updated":"2021-09-29 04:31:38.000000000","message":"I guess we have somewhere to grow when we go py3 only 😊","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bcb3603deb86486287dc69da7c8e655a8fff8e2e","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    2: \u0027H\u0027,"},{"line_number":28,"context_line":"    4: \u0027I\u0027,"},{"line_number":29,"context_line":"    # This just seems excessive; besides, array.array() only takes it on py33+"},{"line_number":30,"context_line":"    # 8: \u0027Q\u0027,"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"467df4bd_a317ad6e","line":30,"in_reply_to":"ba7ee7b9_f52a5db4","updated":"2021-10-12 19:16:16.000000000","message":"Ack","commit_id":"1797cd4cbb8c8883c09d30fc070c4b9dba21cfed"}],"test/unit/cli/test_ringbuilder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"515359021dbefc8be11948284fa34e0a2ad2f37a","unresolved":true,"context_lines":[{"line_number":2552,"context_line":"        exp_results \u003d {\u0027valid_exit_codes\u0027: [2]}"},{"line_number":2553,"context_line":"        out, err \u003d self.run_srb(\"write_ring\", exp_results\u003dexp_results)"},{"line_number":2554,"context_line":"        exp_out \u003d \u0027Unable to write empty ring.\\n\u0027"},{"line_number":2555,"context_line":"        self.assertEqual(exp_out, out[-len(exp_out):])"},{"line_number":2556,"context_line":""},{"line_number":2557,"context_line":"    def test_write_builder(self):"},{"line_number":2558,"context_line":"        # Test builder file already exists"}],"source_content_type":"text/x-python","patch_set":7,"id":"1b0bfbc7_9aefec4f","line":2555,"updated":"2021-10-15 16:27:09.000000000","message":"cause of the --format-version default warning I guess...","commit_id":"73c17515db7a08cc800106a4b54535512effac19"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5e61eefa3e8f9bf24df1a93f2e177eebaa6062ae","unresolved":false,"context_lines":[{"line_number":2552,"context_line":"        exp_results \u003d {\u0027valid_exit_codes\u0027: [2]}"},{"line_number":2553,"context_line":"        out, err \u003d self.run_srb(\"write_ring\", exp_results\u003dexp_results)"},{"line_number":2554,"context_line":"        exp_out \u003d \u0027Unable to write empty ring.\\n\u0027"},{"line_number":2555,"context_line":"        self.assertEqual(exp_out, out[-len(exp_out):])"},{"line_number":2556,"context_line":""},{"line_number":2557,"context_line":"    def test_write_builder(self):"},{"line_number":2558,"context_line":"        # Test builder file already exists"}],"source_content_type":"text/x-python","patch_set":7,"id":"2dad9895_fbb8d185","line":2555,"in_reply_to":"1b0bfbc7_9aefec4f","updated":"2021-10-15 17:23:47.000000000","message":"Yup.","commit_id":"73c17515db7a08cc800106a4b54535512effac19"}],"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":"6ad91364b58f8f9b8a556cbda4f5c124a1466a1a","unresolved":true,"context_lines":[{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"class TestRingV2(TestRing):"},{"line_number":1004,"context_line":"    FORMAT_VERSION \u003d 2"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"e6f1323b_275d21f5","line":1004,"updated":"2021-09-13 18:22:45.000000000","message":"oh, that\u0027s interesting...","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf7171504aeb4c52b09255a61b6e7e0ec705617","unresolved":true,"context_lines":[{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":""},{"line_number":1003,"context_line":"class TestRingV2(TestRing):"},{"line_number":1004,"context_line":"    FORMAT_VERSION \u003d 2"},{"line_number":1005,"context_line":""},{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bc12bd96_ce45821f","line":1004,"in_reply_to":"e6f1323b_275d21f5","updated":"2021-09-13 21:29:06.000000000","message":"Yeah, got me a good bit of coverage for free -- this should surely have some tests featuring rings with dev_id_bytes\u003d1 or 4 or 8, though.","commit_id":"5608be3e622b7b4655514a6c6b15b50063224b45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"515359021dbefc8be11948284fa34e0a2ad2f37a","unresolved":true,"context_lines":[{"line_number":1018,"context_line":""},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"class TestRingV2(TestRing):"},{"line_number":1021,"context_line":"    FORMAT_VERSION \u003d 2"},{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"    def test_1_byte_dev_ids(self):"},{"line_number":1024,"context_line":"        ring_file \u003d os.path.join(self.testdir, \u0027test.ring.gz\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"f3a20ab3_d422252a","line":1021,"updated":"2021-10-15 16:27:09.000000000","message":"that\u0027s cool!","commit_id":"73c17515db7a08cc800106a4b54535512effac19"}]}