)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"3a846ce130c6f7bfa5b49cc835debd547d3daf0c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2da40b7b_33b02bb6","updated":"2023-11-13 05:43:47.000000000","message":"Push up a WIP to get early feedback on new test cases of container backend, will still need to add test cases for container server.","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0d5b7b44_6bd39f23","updated":"2023-11-13 18:03:29.000000000","message":"this makes sense, please consider a more discriptive check_ helper name:\n\nhttps://review.opendev.org/c/openstack/swift/+/900814","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c878cf4a418503815caac91845c39152fcf9f65d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"c70f1f02_5cbdc892","updated":"2023-12-01 09:30:22.000000000","message":"recheck\n\n503 in TestReconstructorRebuild.test_sync_expired_object","commit_id":"c279907202c995ed8c28904dbe7563844557c9d0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"14f6dfe265c4b76083a01a73154d5ee35545b075","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"4f3c33cf_26e00b09","updated":"2023-12-13 06:28:45.000000000","message":"Looking good. Just running the probe tests locally, so save my vote for when I get back from dinner or the morning.","commit_id":"c5ea203be8f91f270cb1b82404c43316d15360e8"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0f80323ff0a73066812e97bce8ccd6a7f45ffb54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"934e9b39_d073fc4b","updated":"2023-12-14 04:49:54.000000000","message":"This is really good guys.. and don\u0027t want to hold things up anymore. But also attempting to make an effort to use the -1 for what it means:\n\n   This patch needs further work before it can be merged\n   \nNot as a negative, in this case I just have a question inline that I\u0027d like answered/discussed before I\u0027m comfortable enough, its isn\u0027t a hard question so will probably change my vote without another patchset ;)\n\nNormally I just do a 0 vote for most questions. Maybe that\u0027s good enough for NIT questions.\nAlso I think it\u0027s nice to try and pull more of our conversations upstream or at least reference any discussions and resolutions that are Swift specific upstream to keep everything transparent.","commit_id":"c5ea203be8f91f270cb1b82404c43316d15360e8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e29b8a6dce3d9274207e97caeaad54661b2cdfb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"c36fb6fd_794a633d","updated":"2023-12-11 19:00:35.000000000","message":"recheck","commit_id":"c5ea203be8f91f270cb1b82404c43316d15360e8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"473790f9e386e19ea52e555b5a8e2182dc1a648d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"cc9f34e9_927d9718","updated":"2024-01-05 01:35:32.000000000","message":"thanks for the help!","commit_id":"035f62f58587b06aaa0d740c678715d4a2d6eae7"}],"swift/container/backend.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":1721,"context_line":"        :param includes: restricts the returned list to the shard range that"},{"line_number":1722,"context_line":"            includes the given value; if ``includes`` is specified then"},{"line_number":1723,"context_line":"            ``fill_gaps``, ``marker`` and ``end_marker`` are ignored."},{"line_number":1724,"context_line":"        :param reverse: reverse the result order."},{"line_number":1725,"context_line":"        :param states: if specified, restricts the returned list to namespaces"},{"line_number":1726,"context_line":"            that have the given state(s); can be a list of ints or a single"},{"line_number":1727,"context_line":"            int."}],"source_content_type":"text/x-python","patch_set":1,"id":"6e06a37e_d542d2a6","line":1724,"updated":"2023-11-13 18:03:29.000000000","message":"helpful!","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cdd639419c5db35249829f46e9110de066b55b1b","unresolved":false,"context_lines":[{"line_number":1721,"context_line":"        :param includes: restricts the returned list to the shard range that"},{"line_number":1722,"context_line":"            includes the given value; if ``includes`` is specified then"},{"line_number":1723,"context_line":"            ``fill_gaps``, ``marker`` and ``end_marker`` are ignored."},{"line_number":1724,"context_line":"        :param reverse: reverse the result order."},{"line_number":1725,"context_line":"        :param states: if specified, restricts the returned list to namespaces"},{"line_number":1726,"context_line":"            that have the given state(s); can be a list of ints or a single"},{"line_number":1727,"context_line":"            int."}],"source_content_type":"text/x-python","patch_set":1,"id":"2817c182_2883d47d","line":1724,"in_reply_to":"6e06a37e_d542d2a6","updated":"2023-11-14 22:25:53.000000000","message":"Ack","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34b7d76b87d8aedc80b052f55239be9c5d6070ab","unresolved":true,"context_lines":[{"line_number":1689,"context_line":"        if namespaces and namespaces[-1].upper \u003d\u003d Namespace.MAX:"},{"line_number":1690,"context_line":"            return None"},{"line_number":1691,"context_line":""},{"line_number":1692,"context_line":"        # If there is a gap in the last, insert a modified copy of own"},{"line_number":1693,"context_line":"        # shard range to fill any gap between the end of any found and the"},{"line_number":1694,"context_line":"        # upper bound of own shard range. Gaps enclosed within the found"},{"line_number":1695,"context_line":"        # shard ranges are not filled."}],"source_content_type":"text/x-python","patch_set":3,"id":"6f620deb_6a688be9","line":1692,"range":{"start_line":1692,"start_character":10,"end_line":1692,"end_character":40},"updated":"2023-11-15 13:04:06.000000000","message":"nit: I think this phrase is redundant - the rest of the sentence says the same","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"15a22df241fbbafb90e1b1abacb850f8f23ec6f5","unresolved":false,"context_lines":[{"line_number":1689,"context_line":"        if namespaces and namespaces[-1].upper \u003d\u003d Namespace.MAX:"},{"line_number":1690,"context_line":"            return None"},{"line_number":1691,"context_line":""},{"line_number":1692,"context_line":"        # If there is a gap in the last, insert a modified copy of own"},{"line_number":1693,"context_line":"        # shard range to fill any gap between the end of any found and the"},{"line_number":1694,"context_line":"        # upper bound of own shard range. Gaps enclosed within the found"},{"line_number":1695,"context_line":"        # shard ranges are not filled."}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4e5c93_84937046","line":1692,"range":{"start_line":1692,"start_character":10,"end_line":1692,"end_character":40},"in_reply_to":"6f620deb_6a688be9","updated":"2023-11-16 06:08:06.000000000","message":"Ack","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0f80323ff0a73066812e97bce8ccd6a7f45ffb54","unresolved":true,"context_lines":[{"line_number":2013,"context_line":""},{"line_number":2014,"context_line":"        shard_ranges \u003d ["},{"line_number":2015,"context_line":"            ShardRange(*row)"},{"line_number":2016,"context_line":"            for row in self._get_shard_range_rows("},{"line_number":2017,"context_line":"                marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":2018,"context_line":"                include_deleted\u003dinclude_deleted, states\u003dstates,"},{"line_number":2019,"context_line":"                include_own\u003dinclude_own, exclude_others\u003dexclude_others)]"}],"source_content_type":"text/x-python","patch_set":13,"id":"1ab519a0_9e101c22","line":2016,"range":{"start_line":2016,"start_character":28,"end_line":2016,"end_character":49},"updated":"2023-12-14 04:49:54.000000000","message":"get shard_range_rows is getting pretty similar to what\u0027s happening in get_namespaces now for the SQL side of things. And they are hitting the same table in the database. the main difference is the number of fields we\u0027re grabbing in the SELECT. I assume we don\u0027t reuse this helper because what, getting those extra fields is also expensive?\n\nIf it isn\u0027t expensive I wonder if we should reuse the same getting rows code (so long as that makes sense) but have a different entrypoint (get_namespaces) and only return the Namespace objects.\n\nI just at what point does it make sense does the DRY princible kick in? I dont mind some repeditiveness when it eases readability. I just wonder when we draw the line.\n\nI don\u0027t want to drag on the patch, this is looking good.. just thinking. Maybe it\u0027s ok as it is for now and we think about using the same get_row code if/when we support the extra api bits (include_deelted etc).\n\nThoughts?","commit_id":"c5ea203be8f91f270cb1b82404c43316d15360e8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"102689115dfd5bf3c727bbb0439068542d74be3f","unresolved":true,"context_lines":[{"line_number":2013,"context_line":""},{"line_number":2014,"context_line":"        shard_ranges \u003d ["},{"line_number":2015,"context_line":"            ShardRange(*row)"},{"line_number":2016,"context_line":"            for row in self._get_shard_range_rows("},{"line_number":2017,"context_line":"                marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":2018,"context_line":"                include_deleted\u003dinclude_deleted, states\u003dstates,"},{"line_number":2019,"context_line":"                include_own\u003dinclude_own, exclude_others\u003dexclude_others)]"}],"source_content_type":"text/x-python","patch_set":13,"id":"bacda769_8c209895","line":2016,"range":{"start_line":2016,"start_character":28,"end_line":2016,"end_character":49},"in_reply_to":"093b679f_de1d1c3f","updated":"2023-12-17 05:53:26.000000000","message":"I benchmarked below patch to reuse ``_get_shard_range_rows`` in ``get_namespaces``.\nhttps://review.opendev.org/c/openstack/swift/+/903814\n\nOn a container DB with ~11k shard ranges, with the new patch 903814, my test was able to fetch all updating namespaces at 9.81 op/sec, each updating namespace GET takes 102ms.\n\nWithout the new patch(the patch #13 of this MR 900740), my test was able to fetch all updating namespaces at 12.01 op/sec, each updating namespace GET takes 83 ms.\n\nSo the current patch and separate SQL query implementation gives about 22% performance improvement. Probably we should continue to use a dedicated SQL query implementation, also will be convenient if we need to optimize it further.","commit_id":"c5ea203be8f91f270cb1b82404c43316d15360e8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"65ab70b2b714186663534803cec280e39a092ccf","unresolved":true,"context_lines":[{"line_number":2013,"context_line":""},{"line_number":2014,"context_line":"        shard_ranges \u003d ["},{"line_number":2015,"context_line":"            ShardRange(*row)"},{"line_number":2016,"context_line":"            for row in self._get_shard_range_rows("},{"line_number":2017,"context_line":"                marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":2018,"context_line":"                include_deleted\u003dinclude_deleted, states\u003dstates,"},{"line_number":2019,"context_line":"                include_own\u003dinclude_own, exclude_others\u003dexclude_others)]"}],"source_content_type":"text/x-python","patch_set":13,"id":"093b679f_de1d1c3f","line":2016,"range":{"start_line":2016,"start_character":28,"end_line":2016,"end_character":49},"in_reply_to":"1ab519a0_9e101c22","updated":"2023-12-16 01:58:07.000000000","message":"from previous container performance optimizations, I came to the conclusion that in general the less data we pull out of sqlite, the better performance we got. that\u0027s why when I started to implement ``get_namespaces``, I decided to retrieve only those four needed fields out of sqlite.\n\nwe should benchmark the difference, will be interesting to find out. I will do so after fixing up my vasio setup.","commit_id":"c5ea203be8f91f270cb1b82404c43316d15360e8"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":true,"context_lines":[{"line_number":2013,"context_line":""},{"line_number":2014,"context_line":"        shard_ranges \u003d ["},{"line_number":2015,"context_line":"            ShardRange(*row)"},{"line_number":2016,"context_line":"            for row in self._get_shard_range_rows("},{"line_number":2017,"context_line":"                marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":2018,"context_line":"                include_deleted\u003dinclude_deleted, states\u003dstates,"},{"line_number":2019,"context_line":"                include_own\u003dinclude_own, exclude_others\u003dexclude_others)]"}],"source_content_type":"text/x-python","patch_set":13,"id":"da6e4f73_dd4ef7c5","line":2016,"range":{"start_line":2016,"start_character":28,"end_line":2016,"end_character":49},"in_reply_to":"bacda769_8c209895","updated":"2024-01-04 16:32:39.000000000","message":"@JIanjian thanks for the benchmarking","commit_id":"c5ea203be8f91f270cb1b82404c43316d15360e8"}],"swift/container/server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34b7d76b87d8aedc80b052f55239be9c5d6070ab","unresolved":true,"context_lines":[{"line_number":789,"context_line":"        # to talk to older version of container servers who don\u0027t support"},{"line_number":790,"context_line":"        # Namespace yet during upgrade."},{"line_number":791,"context_line":"        if shard_format \u003d\u003d \u0027namespace\u0027:"},{"line_number":792,"context_line":"            override_deleted \u003d False"},{"line_number":793,"context_line":"        else:"},{"line_number":794,"context_line":"            shard_format \u003d \u0027full\u0027"},{"line_number":795,"context_line":"            override_deleted \u003d info and config_true_value("}],"source_content_type":"text/x-python","patch_set":3,"id":"10585f4c_1a9a1e3e","line":792,"updated":"2023-11-15 13:04:06.000000000","message":"I don\u0027t recall why we explicitly set this to False.\n\n\u0027deleted\u0027 here refers to the container (not the shard ranges) so it is certainly possible to support this for namespace format, and the code already exists.\n\nBut there\u0027s not currently a use-case (AFAICT x-backend-override-deleted is sent by the sharder when fetching full format shard ranges), so should we actually return BadRequest if the x-backend-override-deleted is received with format\u003dnamespaces? Given that it can be supported \"by default\" I am actually inclined to allow it in case a use case arises.\n\nNote: nothing is free - we\u0027d need to test the support! But we need to test either way and maybe with the test helper methods the testing is almost free ??","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"15a22df241fbbafb90e1b1abacb850f8f23ec6f5","unresolved":true,"context_lines":[{"line_number":789,"context_line":"        # to talk to older version of container servers who don\u0027t support"},{"line_number":790,"context_line":"        # Namespace yet during upgrade."},{"line_number":791,"context_line":"        if shard_format \u003d\u003d \u0027namespace\u0027:"},{"line_number":792,"context_line":"            override_deleted \u003d False"},{"line_number":793,"context_line":"        else:"},{"line_number":794,"context_line":"            shard_format \u003d \u0027full\u0027"},{"line_number":795,"context_line":"            override_deleted \u003d info and config_true_value("}],"source_content_type":"text/x-python","patch_set":3,"id":"ce88fe1b_1310662c","line":792,"in_reply_to":"10585f4c_1a9a1e3e","updated":"2023-11-16 06:08:06.000000000","message":"Yes, \u0027deleted\u0027 here refers to the container (not the shard ranges). I remember we set ``override_deleted \u003d False`` for ``namespace``, because only sharder will use this header and sharder needs full shard ranges. \nSince there is no use case yet, I feel a little awkward to add support right now?","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":true,"context_lines":[{"line_number":789,"context_line":"        # to talk to older version of container servers who don\u0027t support"},{"line_number":790,"context_line":"        # Namespace yet during upgrade."},{"line_number":791,"context_line":"        if shard_format \u003d\u003d \u0027namespace\u0027:"},{"line_number":792,"context_line":"            override_deleted \u003d False"},{"line_number":793,"context_line":"        else:"},{"line_number":794,"context_line":"            shard_format \u003d \u0027full\u0027"},{"line_number":795,"context_line":"            override_deleted \u003d info and config_true_value("}],"source_content_type":"text/x-python","patch_set":3,"id":"7cfd3e0f_d2715890","line":792,"in_reply_to":"13a6e155_fe415d1a","updated":"2024-01-04 16:32:39.000000000","message":"I\u0027m also in two minds, but leaning towards supporting it because:\n\n- it\u0027s one less deviation from the GET API with \u0027full\u0027 format.\n- it avoids a potential future upgrade headache\n- it comes with no performance impact\n- it requires very little work now (compared to alternative work required to add a 400 response)\n\nTo emphasise the last point, we either support this option or we need to add a 400 response to this patch. So we either do work now to prevent override_deleted being used with namespaces, or we do work now and have the namespaces API be closer to the shard ranges API.","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"3cdf82ac151eec0720c23300aff214a321051ab0","unresolved":false,"context_lines":[{"line_number":789,"context_line":"        # to talk to older version of container servers who don\u0027t support"},{"line_number":790,"context_line":"        # Namespace yet during upgrade."},{"line_number":791,"context_line":"        if shard_format \u003d\u003d \u0027namespace\u0027:"},{"line_number":792,"context_line":"            override_deleted \u003d False"},{"line_number":793,"context_line":"        else:"},{"line_number":794,"context_line":"            shard_format \u003d \u0027full\u0027"},{"line_number":795,"context_line":"            override_deleted \u003d info and config_true_value("}],"source_content_type":"text/x-python","patch_set":3,"id":"75e93d44_91206d6f","line":792,"in_reply_to":"1e78843e_12affc4b","updated":"2024-01-05 01:49:02.000000000","message":"okay, your point of \"either support this option or we need to add a 400 response to this patch\" persuaded me, I am on board to support this option.","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"56617fd04bad15fca1308a243896d6ee5c19b22d","unresolved":true,"context_lines":[{"line_number":789,"context_line":"        # to talk to older version of container servers who don\u0027t support"},{"line_number":790,"context_line":"        # Namespace yet during upgrade."},{"line_number":791,"context_line":"        if shard_format \u003d\u003d \u0027namespace\u0027:"},{"line_number":792,"context_line":"            override_deleted \u003d False"},{"line_number":793,"context_line":"        else:"},{"line_number":794,"context_line":"            shard_format \u003d \u0027full\u0027"},{"line_number":795,"context_line":"            override_deleted \u003d info and config_true_value("}],"source_content_type":"text/x-python","patch_set":3,"id":"1e78843e_12affc4b","line":792,"in_reply_to":"7cfd3e0f_d2715890","updated":"2024-01-04 16:52:11.000000000","message":"see https://review.opendev.org/c/openstack/swift/+/904772","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"14f6dfe265c4b76083a01a73154d5ee35545b075","unresolved":true,"context_lines":[{"line_number":789,"context_line":"        # to talk to older version of container servers who don\u0027t support"},{"line_number":790,"context_line":"        # Namespace yet during upgrade."},{"line_number":791,"context_line":"        if shard_format \u003d\u003d \u0027namespace\u0027:"},{"line_number":792,"context_line":"            override_deleted \u003d False"},{"line_number":793,"context_line":"        else:"},{"line_number":794,"context_line":"            shard_format \u003d \u0027full\u0027"},{"line_number":795,"context_line":"            override_deleted \u003d info and config_true_value("}],"source_content_type":"text/x-python","patch_set":3,"id":"13a6e155_fe415d1a","line":792,"in_reply_to":"ce88fe1b_1310662c","updated":"2023-12-13 06:28:45.000000000","message":"We want to get shards out of the container even if its deleted when it\u0027s audited in the sharder.. and probably needs all the data, but maybe it doesn\u0027t or maybe we\u0027ll find a usecase where pulling less data but still being able to pull from delete containers would be useful.\n\nSo to be honest I don\u0027t mind if we enable this feature for namespaces too.. but also that isn\u0027t somethin this patch needs to do. We could just cross that bridge when we want it. What I do want is marker, end_marker and includes support for namespace and this patch is doing that. So let\u0027s just leave this as it is for now!","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"14f6dfe265c4b76083a01a73154d5ee35545b075","unresolved":true,"context_lines":[{"line_number":804,"context_line":"        # Note: there is no support of \u0027includes\u0027, \u0027marker\u0027, \u0027end_marker\u0027"},{"line_number":805,"context_line":"        # or \u0027reverse\u0027 for \u0027namespace\u0027 GET."},{"line_number":806,"context_line":"        if shard_format \u003d\u003d \"namespace\" and not ("},{"line_number":807,"context_line":"            includes or marker or end_marker or reverse"},{"line_number":808,"context_line":"        ):"},{"line_number":809,"context_line":"            override_deleted \u003d False"},{"line_number":810,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":13,"id":"0cca9529_81b8c469","side":"PARENT","line":807,"updated":"2023-12-13 06:28:45.000000000","message":"Yay! Goodbye sucker!","commit_id":"c22a7946d1211d1d1ad20ae23c0c5a7cb870c3cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":804,"context_line":"        # Note: there is no support of \u0027includes\u0027, \u0027marker\u0027, \u0027end_marker\u0027"},{"line_number":805,"context_line":"        # or \u0027reverse\u0027 for \u0027namespace\u0027 GET."},{"line_number":806,"context_line":"        if shard_format \u003d\u003d \"namespace\" and not ("},{"line_number":807,"context_line":"            includes or marker or end_marker or reverse"},{"line_number":808,"context_line":"        ):"},{"line_number":809,"context_line":"            override_deleted \u003d False"},{"line_number":810,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":13,"id":"01190a3b_5f3370dc","side":"PARENT","line":807,"in_reply_to":"0cca9529_81b8c469","updated":"2024-01-04 16:32:39.000000000","message":"Acknowledged","commit_id":"c22a7946d1211d1d1ad20ae23c0c5a7cb870c3cd"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"473790f9e386e19ea52e555b5a8e2182dc1a648d","unresolved":false,"context_lines":[{"line_number":799,"context_line":"        # This will allow proxy server who is going to retrieve Namespace"},{"line_number":800,"context_line":"        # to talk to older version of container servers who don\u0027t support"},{"line_number":801,"context_line":"        # Namespace yet during upgrade."},{"line_number":802,"context_line":"        shard_format \u003d req.headers.get("},{"line_number":803,"context_line":"            \u0027x-backend-record-shard-format\u0027, \u0027full\u0027).lower()"},{"line_number":804,"context_line":"        if shard_format \u003d\u003d \u0027namespace\u0027:"},{"line_number":805,"context_line":"            override_deleted \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"310c4650_b7e27662","line":802,"updated":"2024-01-05 01:35:32.000000000","message":"yeah, it should be moved to here.","commit_id":"035f62f58587b06aaa0d740c678715d4a2d6eae7"}],"test/unit/container/test_backend.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4615,"context_line":"            random.sample(shard_ranges, len(shard_ranges)))"},{"line_number":4616,"context_line":"        actual \u003d broker.get_shard_ranges()"},{"line_number":4617,"context_line":"        self.assertEqual([dict(sr) for sr in shard_ranges],"},{"line_number":4618,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4619,"context_line":""},{"line_number":4620,"context_line":"        actual \u003d broker.get_shard_ranges(states\u003dSHARD_UPDATE_STATES,"},{"line_number":4621,"context_line":"                                         includes\u003d\u0027l\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da778996_2c1de8cb","side":"PARENT","line":4618,"updated":"2023-11-13 18:03:29.000000000","message":"ah, more consistent sanity - kudos","commit_id":"d4f5c420f369b9a293db3bca22497e3895605f97"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":4615,"context_line":"            random.sample(shard_ranges, len(shard_ranges)))"},{"line_number":4616,"context_line":"        actual \u003d broker.get_shard_ranges()"},{"line_number":4617,"context_line":"        self.assertEqual([dict(sr) for sr in shard_ranges],"},{"line_number":4618,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4619,"context_line":""},{"line_number":4620,"context_line":"        actual \u003d broker.get_shard_ranges(states\u003dSHARD_UPDATE_STATES,"},{"line_number":4621,"context_line":"                                         includes\u003d\u0027l\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c3001a08_9e25869c","side":"PARENT","line":4618,"in_reply_to":"da778996_2c1de8cb","updated":"2024-01-04 16:32:39.000000000","message":"Acknowledged","commit_id":"d4f5c420f369b9a293db3bca22497e3895605f97"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4238,"context_line":""},{"line_number":4239,"context_line":"    def _check_get_shard_ranges(self, broker, marker\u003dNone, end_marker\u003dNone,"},{"line_number":4240,"context_line":"                                includes\u003dNone, reverse\u003dFalse, states\u003dNone,"},{"line_number":4241,"context_line":"                                fill_gaps\u003dFalse, expected_sr\u003d[]):"},{"line_number":4242,"context_line":"        # For those \u0027get_shard_ranges\u0027 calls who\u0027s params are compatible with"},{"line_number":4243,"context_line":"        # \u0027get_namespaces\u0027, call both of them to cross-check each other."},{"line_number":4244,"context_line":"        actual_sr \u003d broker.get_shard_ranges("}],"source_content_type":"text/x-python","patch_set":1,"id":"b616c187_dd2a1429","line":4241,"updated":"2023-11-13 18:03:29.000000000","message":"the defaults in this helper may unintionally hide a behavior change with the wrapped get_shard_ranges defaults.\n\nperhaps a **kwargs would make it more clear we intend to support all params on the broker methods as-is","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cdd639419c5db35249829f46e9110de066b55b1b","unresolved":false,"context_lines":[{"line_number":4238,"context_line":""},{"line_number":4239,"context_line":"    def _check_get_shard_ranges(self, broker, marker\u003dNone, end_marker\u003dNone,"},{"line_number":4240,"context_line":"                                includes\u003dNone, reverse\u003dFalse, states\u003dNone,"},{"line_number":4241,"context_line":"                                fill_gaps\u003dFalse, expected_sr\u003d[]):"},{"line_number":4242,"context_line":"        # For those \u0027get_shard_ranges\u0027 calls who\u0027s params are compatible with"},{"line_number":4243,"context_line":"        # \u0027get_namespaces\u0027, call both of them to cross-check each other."},{"line_number":4244,"context_line":"        actual_sr \u003d broker.get_shard_ranges("}],"source_content_type":"text/x-python","patch_set":1,"id":"d0c5f900_33576463","line":4241,"in_reply_to":"b616c187_dd2a1429","updated":"2023-11-14 22:25:53.000000000","message":"Ack","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4251,"context_line":"        actual_ns \u003d broker.get_namespaces("},{"line_number":4252,"context_line":"            marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":4253,"context_line":"            reverse\u003dreverse, states\u003dstates, fill_gaps\u003dfill_gaps)"},{"line_number":4254,"context_line":"        self.assertEqual(expected_ns, actual_ns)"},{"line_number":4255,"context_line":""},{"line_number":4256,"context_line":"    @with_tempdir"},{"line_number":4257,"context_line":"    def test_get_shard_ranges(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7730f374_04a4327f","line":4254,"updated":"2023-11-13 18:03:29.000000000","message":"oic, the main benifit is any assert on shard-ranges asserts it gets the name-spaces\n\nThe function name might make that more clear - i missed the comment.","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cdd639419c5db35249829f46e9110de066b55b1b","unresolved":false,"context_lines":[{"line_number":4251,"context_line":"        actual_ns \u003d broker.get_namespaces("},{"line_number":4252,"context_line":"            marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":4253,"context_line":"            reverse\u003dreverse, states\u003dstates, fill_gaps\u003dfill_gaps)"},{"line_number":4254,"context_line":"        self.assertEqual(expected_ns, actual_ns)"},{"line_number":4255,"context_line":""},{"line_number":4256,"context_line":"    @with_tempdir"},{"line_number":4257,"context_line":"    def test_get_shard_ranges(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"62f65a63_3e969c67","line":4254,"in_reply_to":"7730f374_04a4327f","updated":"2023-11-14 22:25:53.000000000","message":"Ack","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4304,"context_line":"            expected_sr\u003dshard_ranges[2:3])"},{"line_number":4305,"context_line":""},{"line_number":4306,"context_line":"        actual \u003d broker.get_shard_ranges(marker\u003d\u0027e\u0027, end_marker\u003d\u0027e\u0027)"},{"line_number":4307,"context_line":"        self.assertFalse([dict(sr) for sr in actual])"},{"line_number":4308,"context_line":""},{"line_number":4309,"context_line":"        # includes overrides include_own"},{"line_number":4310,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027b\u0027, include_own\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f0517641_c8617b58","line":4307,"updated":"2023-11-13 18:03:29.000000000","message":"is this just saying `expected_sr\u003d[]`","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cdd639419c5db35249829f46e9110de066b55b1b","unresolved":false,"context_lines":[{"line_number":4304,"context_line":"            expected_sr\u003dshard_ranges[2:3])"},{"line_number":4305,"context_line":""},{"line_number":4306,"context_line":"        actual \u003d broker.get_shard_ranges(marker\u003d\u0027e\u0027, end_marker\u003d\u0027e\u0027)"},{"line_number":4307,"context_line":"        self.assertFalse([dict(sr) for sr in actual])"},{"line_number":4308,"context_line":""},{"line_number":4309,"context_line":"        # includes overrides include_own"},{"line_number":4310,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027b\u0027, include_own\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da1ea4a8_88ce1af3","line":4307,"in_reply_to":"f0517641_c8617b58","updated":"2023-11-14 22:25:53.000000000","message":"Ack","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4308,"context_line":""},{"line_number":4309,"context_line":"        # includes overrides include_own"},{"line_number":4310,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027b\u0027, include_own\u003dTrue)"},{"line_number":4311,"context_line":"        self.assertEqual([dict(shard_ranges[0])], [dict(sr) for sr in actual])"},{"line_number":4312,"context_line":"        # ... unless they coincide"},{"line_number":4313,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027t\u0027, include_own\u003dTrue)"},{"line_number":4314,"context_line":"        self.assertEqual([dict(own_shard_range)], [dict(sr) for sr in actual])"}],"source_content_type":"text/x-python","patch_set":1,"id":"68a878e1_c077da5d","line":4311,"updated":"2023-11-13 18:03:29.000000000","message":"are we trying to highlight get_namespaces doesn\u0027t support include_own?","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34b7d76b87d8aedc80b052f55239be9c5d6070ab","unresolved":true,"context_lines":[{"line_number":4308,"context_line":""},{"line_number":4309,"context_line":"        # includes overrides include_own"},{"line_number":4310,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027b\u0027, include_own\u003dTrue)"},{"line_number":4311,"context_line":"        self.assertEqual([dict(shard_ranges[0])], [dict(sr) for sr in actual])"},{"line_number":4312,"context_line":"        # ... unless they coincide"},{"line_number":4313,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027t\u0027, include_own\u003dTrue)"},{"line_number":4314,"context_line":"        self.assertEqual([dict(own_shard_range)], [dict(sr) for sr in actual])"}],"source_content_type":"text/x-python","patch_set":1,"id":"b0fc754d_9695f69b","line":4311,"in_reply_to":"68a878e1_c077da5d","updated":"2023-11-15 13:04:06.000000000","message":"hmmm, might help to emphasise the differences by using a complementary helper like _check_get_sr()\n\nor separate the shard range specific scenarios to a different test?","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"15a22df241fbbafb90e1b1abacb850f8f23ec6f5","unresolved":false,"context_lines":[{"line_number":4308,"context_line":""},{"line_number":4309,"context_line":"        # includes overrides include_own"},{"line_number":4310,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027b\u0027, include_own\u003dTrue)"},{"line_number":4311,"context_line":"        self.assertEqual([dict(shard_ranges[0])], [dict(sr) for sr in actual])"},{"line_number":4312,"context_line":"        # ... unless they coincide"},{"line_number":4313,"context_line":"        actual \u003d broker.get_shard_ranges(includes\u003d\u0027t\u0027, include_own\u003dTrue)"},{"line_number":4314,"context_line":"        self.assertEqual([dict(own_shard_range)], [dict(sr) for sr in actual])"}],"source_content_type":"text/x-python","patch_set":1,"id":"548d2f86_f6b58282","line":4311,"in_reply_to":"b0fc754d_9695f69b","updated":"2023-11-16 06:08:06.000000000","message":"added _check_get_sr()","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4337,"context_line":"            broker, marker\u003d\u0027e\u0027, end_marker\u003d\u0027\u0027, expected_sr\u003dundeleted[2:])"},{"line_number":4338,"context_line":"        self._check_get_shard_ranges("},{"line_number":4339,"context_line":"            broker, marker\u003d\u0027e\u0027, end_marker\u003d\u0027\u0027, reverse\u003dTrue,"},{"line_number":4340,"context_line":"            expected_sr\u003dlist(reversed(undeleted[:3])))"},{"line_number":4341,"context_line":""},{"line_number":4342,"context_line":"        # marker is Namespace.MIN"},{"line_number":4343,"context_line":"        self._check_get_shard_ranges("}],"source_content_type":"text/x-python","patch_set":1,"id":"9581eb3a_68e3b6c1","line":4340,"updated":"2023-11-13 18:03:29.000000000","message":"net negative line count here - nice abstraction","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":4337,"context_line":"            broker, marker\u003d\u0027e\u0027, end_marker\u003d\u0027\u0027, expected_sr\u003dundeleted[2:])"},{"line_number":4338,"context_line":"        self._check_get_shard_ranges("},{"line_number":4339,"context_line":"            broker, marker\u003d\u0027e\u0027, end_marker\u003d\u0027\u0027, reverse\u003dTrue,"},{"line_number":4340,"context_line":"            expected_sr\u003dlist(reversed(undeleted[:3])))"},{"line_number":4341,"context_line":""},{"line_number":4342,"context_line":"        # marker is Namespace.MIN"},{"line_number":4343,"context_line":"        self._check_get_shard_ranges("}],"source_content_type":"text/x-python","patch_set":1,"id":"64fc3306_8602b0c6","line":4340,"in_reply_to":"9581eb3a_68e3b6c1","updated":"2024-01-04 16:32:39.000000000","message":"Acknowledged","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4346,"context_line":"        actual \u003d broker.get_shard_ranges(marker\u003d\u0027\u0027, end_marker\u003d\u0027d\u0027,"},{"line_number":4347,"context_line":"                                         reverse\u003dTrue, include_deleted\u003dTrue)"},{"line_number":4348,"context_line":"        self.assertEqual([dict(sr) for sr in reversed(shard_ranges[2:])],"},{"line_number":4349,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4350,"context_line":""},{"line_number":4351,"context_line":"        # marker, end_marker span entire namespace"},{"line_number":4352,"context_line":"        self._check_get_shard_ranges("}],"source_content_type":"text/x-python","patch_set":1,"id":"006cf2f0_c125f1c5","line":4349,"updated":"2023-11-13 18:03:29.000000000","message":"again here we\u0027re (subtly) highlighting include_deleted is not supported by namespaces","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":4346,"context_line":"        actual \u003d broker.get_shard_ranges(marker\u003d\u0027\u0027, end_marker\u003d\u0027d\u0027,"},{"line_number":4347,"context_line":"                                         reverse\u003dTrue, include_deleted\u003dTrue)"},{"line_number":4348,"context_line":"        self.assertEqual([dict(sr) for sr in reversed(shard_ranges[2:])],"},{"line_number":4349,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4350,"context_line":""},{"line_number":4351,"context_line":"        # marker, end_marker span entire namespace"},{"line_number":4352,"context_line":"        self._check_get_shard_ranges("}],"source_content_type":"text/x-python","patch_set":1,"id":"2b81f561_ea27fd7f","line":4349,"in_reply_to":"006cf2f0_c125f1c5","updated":"2024-01-04 16:32:39.000000000","message":"Acknowledged","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4431,"context_line":"        self._check_get_shard_ranges("},{"line_number":4432,"context_line":"            broker, fill_gaps\u003dTrue, end_marker\u003d\u0027h\u0027, expected_sr\u003dundeleted)"},{"line_number":4433,"context_line":"        self._check_get_shard_ranges("},{"line_number":4434,"context_line":"            broker, fill_gaps\u003dTrue, end_marker\u003d\u0027a\u0027, expected_sr\u003d[])"},{"line_number":4435,"context_line":""},{"line_number":4436,"context_line":"        # get everything"},{"line_number":4437,"context_line":"        actual \u003d broker.get_shard_ranges(include_own\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f364558_307f638e","line":4434,"updated":"2023-11-13 18:03:29.000000000","message":"wow, this test really rambles on^W^Wcovers a lot of ground\n\ntwo things stopped me from splitting it up:\n\n1) with_tempdir decorator is sort of noisy on lots of little tests\n2) they asserts sort of bounce around - it\u0027s not clear how to group\n\nIt\u0027d be better to pull them all out to their own TestCase with tempdir and these specific merged shard ranges in setUp - but there\u0027s some multi-test inheritence with different create queries that would make that verbose too.\n\nI\u0027m not saying this is the best we can do; but it\u0027s probably not sufficiently annoying to invest in fixing ATM.","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4488,"context_line":"        # item in that range."},{"line_number":4489,"context_line":"        ltor.set_deleted(next(self.ts))"},{"line_number":4490,"context_line":"        broker.merge_shard_ranges([ltor])"},{"line_number":4491,"context_line":"        self._check_get_shard_ranges(broker, includes\u003d\u0027p\u0027, expected_sr\u003d[])"},{"line_number":4492,"context_line":""},{"line_number":4493,"context_line":"    @with_tempdir"},{"line_number":4494,"context_line":"    def test_overlap_shard_range_order(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"28494187_bb46a3be","line":4491,"updated":"2023-11-13 18:03:29.000000000","message":"I like this test a lot more than the previous - more targeted\n\nthe _check helper makes it a little quicker to read as well and adds coverage to namespaces - kudos","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":4488,"context_line":"        # item in that range."},{"line_number":4489,"context_line":"        ltor.set_deleted(next(self.ts))"},{"line_number":4490,"context_line":"        broker.merge_shard_ranges([ltor])"},{"line_number":4491,"context_line":"        self._check_get_shard_ranges(broker, includes\u003d\u0027p\u0027, expected_sr\u003d[])"},{"line_number":4492,"context_line":""},{"line_number":4493,"context_line":"    @with_tempdir"},{"line_number":4494,"context_line":"    def test_overlap_shard_range_order(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"03a66dce_1cdc069d","line":4491,"in_reply_to":"28494187_bb46a3be","updated":"2024-01-04 16:32:39.000000000","message":"Acknowledged","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4529,"context_line":"        self.assertEqual(expected, ["},{"line_number":4530,"context_line":"            sr.name for sr in broker.get_shard_ranges()])"},{"line_number":4531,"context_line":"        self.assertEqual(expected, ["},{"line_number":4532,"context_line":"            ns.name for ns in broker.get_namespaces()])"},{"line_number":4533,"context_line":""},{"line_number":4534,"context_line":"    @with_tempdir"},{"line_number":4535,"context_line":"    def test_get_shard_ranges_with_sharding_overlaps(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ebfc1aa7_13d91a79","line":4532,"updated":"2023-11-13 18:03:29.000000000","message":"this is a neat test - it seems like the _check helper could be used here as well.","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":4529,"context_line":"        self.assertEqual(expected, ["},{"line_number":4530,"context_line":"            sr.name for sr in broker.get_shard_ranges()])"},{"line_number":4531,"context_line":"        self.assertEqual(expected, ["},{"line_number":4532,"context_line":"            ns.name for ns in broker.get_namespaces()])"},{"line_number":4533,"context_line":""},{"line_number":4534,"context_line":"    @with_tempdir"},{"line_number":4535,"context_line":"    def test_get_shard_ranges_with_sharding_overlaps(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"603c46b2_3afd61c5","line":4532,"in_reply_to":"ebfc1aa7_13d91a79","updated":"2024-01-04 16:32:39.000000000","message":"Done","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4555,"context_line":"        actual \u003d broker.get_shard_ranges()"},{"line_number":4556,"context_line":"        self.assertEqual([dict(sr) for sr in shard_ranges],"},{"line_number":4557,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4558,"context_line":"        self._check_get_shard_ranges(broker, expected_sr\u003dshard_ranges)"},{"line_number":4559,"context_line":""},{"line_number":4560,"context_line":"        self._check_get_shard_ranges("},{"line_number":4561,"context_line":"            broker, states\u003dSHARD_LISTING_STATES,"}],"source_content_type":"text/x-python","patch_set":1,"id":"071b02c2_42936672","line":4558,"updated":"2023-11-13 18:03:29.000000000","message":"oic, this is a new assert - sanity","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":4555,"context_line":"        actual \u003d broker.get_shard_ranges()"},{"line_number":4556,"context_line":"        self.assertEqual([dict(sr) for sr in shard_ranges],"},{"line_number":4557,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4558,"context_line":"        self._check_get_shard_ranges(broker, expected_sr\u003dshard_ranges)"},{"line_number":4559,"context_line":""},{"line_number":4560,"context_line":"        self._check_get_shard_ranges("},{"line_number":4561,"context_line":"            broker, states\u003dSHARD_LISTING_STATES,"}],"source_content_type":"text/x-python","patch_set":1,"id":"b8b28cd7_a8b0135f","line":4558,"in_reply_to":"071b02c2_42936672","updated":"2024-01-04 16:32:39.000000000","message":"Acknowledged","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4759,"context_line":"        # verify that marker \u0026 end_marker plumb through to an SQL condition"},{"line_number":4760,"context_line":"        self.assertIn(\"WHERE deleted \u003d 0 AND name !\u003d ? AND lower \u003c ? AND \""},{"line_number":4761,"context_line":"                      \"(upper \u003d \u0027\u0027 OR upper \u003e ?)\", mock_call_args[0][1])"},{"line_number":4762,"context_line":"        self.assertEqual([\u0027a/c\u0027, \u0027d\u0027, \u0027c\u0027], mock_call_args[0][2])"},{"line_number":4763,"context_line":""},{"line_number":4764,"context_line":"    @with_tempdir"},{"line_number":4765,"context_line":"    def test_get_namespaces_state_filtering(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"e5a35598_8ddbb4db","line":4762,"updated":"2023-11-13 18:03:29.000000000","message":"making an assert on the query is a bit too on the nose for my tastes, but I can see you\u0027re extending an existing test - and the pattern is used elsewhere.  I don\u0027t recall every being specifically annoyed by query assertions when maintaining brokers; I guess we can see if these assert are helpful.","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4bd6c7a0d9277a5f2977701f00bf86098bf2e938","unresolved":true,"context_lines":[{"line_number":4815,"context_line":"        do_test([ShardRange.SHARDED], [])"},{"line_number":4816,"context_line":""},{"line_number":4817,"context_line":"    @with_tempdir"},{"line_number":4818,"context_line":"    def test_get_namespaces_root_container_fill_gap(self, tempdir):"},{"line_number":4819,"context_line":"        # Test GET namespaces from a root container with full namespace."},{"line_number":4820,"context_line":"        own_shard_range \u003d ShardRange(\u0027a/c\u0027, next("},{"line_number":4821,"context_line":"            self.ts), \u0027\u0027, \u0027\u0027, state\u003dShardRange.SHARDED)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b4b5fd5c_8138ee2a","line":4818,"updated":"2023-11-13 18:03:29.000000000","message":"ah, indeed.","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2f7a57667f653c63683a04a9ab8a75e80762d8aa","unresolved":false,"context_lines":[{"line_number":4815,"context_line":"        do_test([ShardRange.SHARDED], [])"},{"line_number":4816,"context_line":""},{"line_number":4817,"context_line":"    @with_tempdir"},{"line_number":4818,"context_line":"    def test_get_namespaces_root_container_fill_gap(self, tempdir):"},{"line_number":4819,"context_line":"        # Test GET namespaces from a root container with full namespace."},{"line_number":4820,"context_line":"        own_shard_range \u003d ShardRange(\u0027a/c\u0027, next("},{"line_number":4821,"context_line":"            self.ts), \u0027\u0027, \u0027\u0027, state\u003dShardRange.SHARDED)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5c2af071_28c177fa","line":4818,"in_reply_to":"b4b5fd5c_8138ee2a","updated":"2024-01-04 16:32:39.000000000","message":"Acknowledged","commit_id":"261ff41c7ea4917f030f656cfafeba4badda09cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34b7d76b87d8aedc80b052f55239be9c5d6070ab","unresolved":true,"context_lines":[{"line_number":4236,"context_line":"        check_bad_value([\u0027\u0027])"},{"line_number":4237,"context_line":"        check_bad_value(\u0027active\u0027)"},{"line_number":4238,"context_line":""},{"line_number":4239,"context_line":"    def _check_get_sr_and_ns(self, broker, expected_sr\u003dNone, **kwargs):"},{"line_number":4240,"context_line":"        \"\"\""},{"line_number":4241,"context_line":"        For those \u0027get_shard_ranges\u0027 calls who\u0027s params are compatible with"},{"line_number":4242,"context_line":"        \u0027get_namespaces\u0027, call both of them to cross-check each other."}],"source_content_type":"text/x-python","patch_set":3,"id":"d5854668_202bbbaf","line":4239,"range":{"start_line":4239,"start_character":43,"end_line":4239,"end_character":59},"updated":"2023-11-15 13:04:06.000000000","message":"I haven\u0027t checked but do we ever call this helper with expected_sr? the very first use sets expected_sr\u003d[] when it could have not passed the arg :) I wonder if the arg could just be required?","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"15a22df241fbbafb90e1b1abacb850f8f23ec6f5","unresolved":false,"context_lines":[{"line_number":4236,"context_line":"        check_bad_value([\u0027\u0027])"},{"line_number":4237,"context_line":"        check_bad_value(\u0027active\u0027)"},{"line_number":4238,"context_line":""},{"line_number":4239,"context_line":"    def _check_get_sr_and_ns(self, broker, expected_sr\u003dNone, **kwargs):"},{"line_number":4240,"context_line":"        \"\"\""},{"line_number":4241,"context_line":"        For those \u0027get_shard_ranges\u0027 calls who\u0027s params are compatible with"},{"line_number":4242,"context_line":"        \u0027get_namespaces\u0027, call both of them to cross-check each other."}],"source_content_type":"text/x-python","patch_set":3,"id":"5c7122b5_c04dfa4b","line":4239,"range":{"start_line":4239,"start_character":43,"end_line":4239,"end_character":59},"in_reply_to":"d5854668_202bbbaf","updated":"2023-11-16 06:08:06.000000000","message":"yes, this arg can be required.","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"34b7d76b87d8aedc80b052f55239be9c5d6070ab","unresolved":true,"context_lines":[{"line_number":4248,"context_line":"        expected_ns \u003d [Namespace(sr.name, sr.lower, sr.upper)"},{"line_number":4249,"context_line":"                       for sr in expected_sr]"},{"line_number":4250,"context_line":"        actual_ns \u003d broker.get_namespaces(**kwargs)"},{"line_number":4251,"context_line":"        self.assertEqual(expected_ns, actual_ns)"},{"line_number":4252,"context_line":""},{"line_number":4253,"context_line":"    @with_tempdir"},{"line_number":4254,"context_line":"    def test_get_shard_ranges(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a7d7955_1a1bbdc1","line":4251,"updated":"2023-11-15 13:04:06.000000000","message":"+1 I like how this helper will keep the similar aspects of the get_namespaces and get_shard_ranges methods in sync\n\nhow about a complementary helper to use where the two methods *differ* - I think get_shard_ranges kwargs are a superset of get_namespaces kwargs, so it might be that a _check_get_sr() is sufficient?","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"15a22df241fbbafb90e1b1abacb850f8f23ec6f5","unresolved":false,"context_lines":[{"line_number":4248,"context_line":"        expected_ns \u003d [Namespace(sr.name, sr.lower, sr.upper)"},{"line_number":4249,"context_line":"                       for sr in expected_sr]"},{"line_number":4250,"context_line":"        actual_ns \u003d broker.get_namespaces(**kwargs)"},{"line_number":4251,"context_line":"        self.assertEqual(expected_ns, actual_ns)"},{"line_number":4252,"context_line":""},{"line_number":4253,"context_line":"    @with_tempdir"},{"line_number":4254,"context_line":"    def test_get_shard_ranges(self, tempdir):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a3a33ba_cc06e856","line":4251,"in_reply_to":"9a7d7955_1a1bbdc1","updated":"2023-11-16 06:08:06.000000000","message":"Good idea, added _check_get_sr().","commit_id":"d14bb63bba0e1983a3b1c925fd6f6e85da95c039"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"473790f9e386e19ea52e555b5a8e2182dc1a648d","unresolved":false,"context_lines":[{"line_number":4244,"context_line":"        self.assertEqual([dict(sr) for sr in expected_sr],"},{"line_number":4245,"context_line":"                         [dict(sr) for sr in actual_sr])"},{"line_number":4246,"context_line":""},{"line_number":4247,"context_line":"    def _check_get_ns(self, broker, expected_ns, **kwargs):"},{"line_number":4248,"context_line":"        actual_ns \u003d broker.get_namespaces(**kwargs)"},{"line_number":4249,"context_line":"        self.assertEqual(expected_ns, actual_ns)"},{"line_number":4250,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"db36a0e2_7afefae3","line":4247,"updated":"2024-01-05 01:35:32.000000000","message":"the new ``_check_get_ns`` helper is nice!","commit_id":"035f62f58587b06aaa0d740c678715d4a2d6eae7"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"473790f9e386e19ea52e555b5a8e2182dc1a648d","unresolved":false,"context_lines":[{"line_number":4739,"context_line":"        broker \u003d self._setup_broker_with_shard_ranges("},{"line_number":4740,"context_line":"            tempdir, own_shard_range, shard_ranges)"},{"line_number":4741,"context_line":""},{"line_number":4742,"context_line":"        def do_test(states, expected_ns):"},{"line_number":4743,"context_line":"            self._check_get_sr_and_ns(broker, expected_ns, states\u003dstates)"},{"line_number":4744,"context_line":"            filler_lower \u003d expected_ns[-1].upper if expected_ns else \u0027a\u0027"},{"line_number":4745,"context_line":"            filler \u003d [Namespace(\u0027a/c\u0027, filler_lower, \u0027z\u0027)]"}],"source_content_type":"text/x-python","patch_set":14,"id":"44ac849d_a5eb21cf","line":4742,"updated":"2024-01-05 01:35:32.000000000","message":"``expected_ns`` actually is expected shard range objects, I will rename it.","commit_id":"035f62f58587b06aaa0d740c678715d4a2d6eae7"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"473790f9e386e19ea52e555b5a8e2182dc1a648d","unresolved":false,"context_lines":[{"line_number":4743,"context_line":"            self._check_get_sr_and_ns(broker, expected_ns, states\u003dstates)"},{"line_number":4744,"context_line":"            filler_lower \u003d expected_ns[-1].upper if expected_ns else \u0027a\u0027"},{"line_number":4745,"context_line":"            filler \u003d [Namespace(\u0027a/c\u0027, filler_lower, \u0027z\u0027)]"},{"line_number":4746,"context_line":"            self._check_get_ns(broker, expected_ns + filler,"},{"line_number":4747,"context_line":"                               states\u003dstates, fill_gaps\u003dTrue)"},{"line_number":4748,"context_line":""},{"line_number":4749,"context_line":"        do_test(ShardRange.CREATED, shard_ranges[:2])"}],"source_content_type":"text/x-python","patch_set":14,"id":"abbf62d1_903b221f","line":4746,"updated":"2024-01-05 01:35:32.000000000","message":"``expected_ns + filler`` is a list with expected shard range objects and filler namespace object, even though ``assertEqual`` works because ShardRange and Namespace share the same comparison functions, but I will create a list of Namespace objects here to make it more obvious.","commit_id":"035f62f58587b06aaa0d740c678715d4a2d6eae7"}]}