)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"17c93c0394cd1eaa5ce2df7c1b4bb0e166e24729","unresolved":true,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"On the author\u0027s machine, using a DB with ~12000 shard ranges this"},{"line_number":18,"context_line":"patch reduces the get_own_shard_range() execution time by a factor of"},{"line_number":19,"context_line":"approximately 100."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I43f79de3f0603b9fab8bf6f7b4c3b1892a8919b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"2eaef2eb_b8ad8621","line":19,"range":{"start_line":19,"start_character":14,"end_line":19,"end_character":17},"updated":"2023-07-14 16:36:05.000000000","message":"🤩","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"5de660a87c0422a1c05691cdb008f3b6e8b4c5cc","unresolved":true,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"On the author\u0027s machine, using a DB with ~12000 shard ranges this"},{"line_number":18,"context_line":"patch reduces the get_own_shard_range() execution time by a factor of"},{"line_number":19,"context_line":"approximately 100."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I43f79de3f0603b9fab8bf6f7b4c3b1892a8919b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"37b05121_09e4e3b7","line":19,"range":{"start_line":19,"start_character":14,"end_line":19,"end_character":17},"in_reply_to":"2eaef2eb_b8ad8621","updated":"2023-07-24 04:41:09.000000000","message":"Woah!!","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9dac66c6e2f3361234d49610fd211c46870e24be","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"On the author\u0027s machine, using a DB with ~12000 shard ranges this"},{"line_number":18,"context_line":"patch reduces the get_own_shard_range() execution time by a factor of"},{"line_number":19,"context_line":"approximately 100."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I43f79de3f0603b9fab8bf6f7b4c3b1892a8919b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"25492877_c89bdc88","line":19,"range":{"start_line":19,"start_character":14,"end_line":19,"end_character":17},"in_reply_to":"37b05121_09e4e3b7","updated":"2023-07-24 10:17:26.000000000","message":"Ack","commit_id":"197a29d78b23f679b17d371a132503a042689582"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99d87c35388c8086ff8193c79e3f6672fde6782d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cdd9d879_520b3fac","updated":"2023-07-18 13:51:19.000000000","message":"___ TestReservedNamespaceMergePolicyIndex.test_reconciler_move_object_twice ____\n\t\u003e               self.fail(\u0027Found unexpected object %r in the queue\u0027 % obj)\n\tE               AssertionError: Found unexpected object {u\u0027bytes\u0027: 0, u\u0027last_modified\u0027: u\u00272023-07-17T18:14:54.756370\u0027, u\u0027hash\u0027: u\u00271689617694.75637_3\u0027, u\u0027name\u0027: u\u00272:/AUTH_test/\\x00container\\x009e8ec62a-7dfa-4a64-a11b-d5109b39e4e7/\\x00object\\x00ee663427-3658-4578-812f-1ace87d0ba0b\u0027, u\u0027content_type\u0027: u\u0027application/x-delete\u0027} in the queue\n\n\ttest/probe/test_container_merge_policy_index.py:554: AssertionError\n\nsuspicious!","commit_id":"f029965b7ad6b209268cbbdf4fc99d69237f6f38"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"32a8d61df9a2cfd02b3c21f1bfc4328d40792306","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fff9e48e_2ca3bf4c","updated":"2023-07-18 16:20:07.000000000","message":"hmmm, I see a different assertion failure in the same probe test on master when cycling the test in a py3 vsaio\n\n```\n\u003e               self.assertEqual(obj[\u0027name\u0027], expected)\nE               AssertionError: \u00271:/AUTH_test/\\x00container\\x002e0e537e-c1[72 chars]b6de\u0027 !\u003d \u00270:/AUTH_test/\\x00container\\x002e0e537e-c1[72 chars]b6de\u0027\nE               - 1:/AUTH_test/container2e0e537e-c12a-4506-8fc4-45471d702848/objectdf4d3593-776b-4e51-a592-a5948c2eb6de\nE               ? ^\nE               + 0:/AUTH_test/container2e0e537e-c12a-4506-8fc4-45471d702848/objectdf4d3593-776b-4e51-a592-a5948c2eb6de\nE               ? ^\n\ntest/probe/test_container_merge_policy_index.py:536: AssertionError\n```","commit_id":"f029965b7ad6b209268cbbdf4fc99d69237f6f38"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6802c43f90330aa2096183b70d89f8e993a49bd6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c57d3c59_b99d63f1","updated":"2023-07-25 00:22:13.000000000","message":"Al responded to my question, and basically we\u0027re making available the limit api in sqlite when building the SQL request. Sqlite supports limit \u003e\u003d 0, so that\u0027s what we allow.\nSounds reasonable to me!","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"5de660a87c0422a1c05691cdb008f3b6e8b4c5cc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b0a83906_f55eff0e","updated":"2023-07-24 04:41:09.000000000","message":"I only have a question about allowing LIMIT 0, because I don\u0027t see the benefit in always returning an empty list.\n\nOtherwise, this looks like a no brainer.","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d5611916f01e5f58e34444db2840348dbac54bb6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3b9a4eb4_ed31e5cc","updated":"2023-08-02 22:26:25.000000000","message":"I think I\u0027d be OK with merging this as-is... but the format string delta gives me a little pause.","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"57bec01eb6d22cf8b9252aef32b8f85ce970af20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3e079b87_48c92202","updated":"2023-07-20 18:26:36.000000000","message":"LGTM.","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0d3d4322fb0b205b0f30f17209d2f8df5b346140","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"5ea7f43e_0abca2bc","updated":"2023-08-03 17:20:47.000000000","message":"recheck\n\nSome 502 in `openstack.tests.functional.placement.v1.test_trait.TestTrait.test_resource_provider_inventory`; unrelated to us.\n\nMatt and Jian both like it, let\u0027s merge it!","commit_id":"e9e0c4a95f06dadf43e3ba6d18c29adb39105319"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f0e6245d0635c9cfbef22e12ed7c13205ebf21f1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"223f21e3_01a2eade","updated":"2023-08-04 08:11:42.000000000","message":"merging based on Tim\u0027s previous approval.\n\ntest failure unrelated:\n```\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nFailed 1 tests - output below:\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nopenstack.tests.functional.cloud.test_project_cleanup.TestProjectCleanup.test_block_storage_cleanup\n---------------------------------------------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/openstack/tests/functional/cloud/test_project_cleanup.py\", line 206, in test_block_storage_cleanup\n    self.assertEqual(0, len(list(self.conn.block_storage.backups())))\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/openstack/tests/base.py\", line 97, in assertEqual\n    return super(TestCase, self).assertEqual(\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/.tox/functional/lib/python3.10/site-packages/testtools/testcase.py\", line 394, in assertEqual\n    self.assertThat(observed, matcher, message)\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/.tox/functional/lib/python3.10/site-packages/testtools/testcase.py\", line 481, in assertThat\n    raise mismatch_error\n\n    testtools.matchers._impl.MismatchError: 0 !\u003d 1\n\n```","commit_id":"6e8d82c97e28026e03f71bf38ccbcd2f1c606afa"}],"swift/container/backend.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1c86b4800fa56b0bc06afe4f839f3ea6bd6ba35c","unresolved":true,"context_lines":[{"line_number":1850,"context_line":"        :param include_own: boolean that governs whether the row whose name"},{"line_number":1851,"context_line":"            matches the broker\u0027s path is included in the returned list. If"},{"line_number":1852,"context_line":"            True, that row is included, otherwise it is not included. Default"},{"line_number":1853,"context_line":"            is False."},{"line_number":1854,"context_line":"        :param exclude_others: boolean that governs whether the rows whose"},{"line_number":1855,"context_line":"            names do not match the broker\u0027s path are included in the returned"},{"line_number":1856,"context_line":"            list. If True, those rows are not included, otherwise they are"}],"source_content_type":"text/x-python","patch_set":1,"id":"2745cd8d_725c65be","line":1853,"updated":"2023-07-17 16:41:22.000000000","message":"I think it\u0027s *probably* reasonable to not conflate this with the limit kwarg even tho we use them together - my thinking is: this method is already complex; we could at least try and simplify by keeping behavior as explicit as possible.","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"32a8d61df9a2cfd02b3c21f1bfc4328d40792306","unresolved":true,"context_lines":[{"line_number":1850,"context_line":"        :param include_own: boolean that governs whether the row whose name"},{"line_number":1851,"context_line":"            matches the broker\u0027s path is included in the returned list. If"},{"line_number":1852,"context_line":"            True, that row is included, otherwise it is not included. Default"},{"line_number":1853,"context_line":"            is False."},{"line_number":1854,"context_line":"        :param exclude_others: boolean that governs whether the rows whose"},{"line_number":1855,"context_line":"            names do not match the broker\u0027s path are included in the returned"},{"line_number":1856,"context_line":"            list. If True, those rows are not included, otherwise they are"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9ac478_e933ac9c","line":1853,"in_reply_to":"2745cd8d_725c65be","updated":"2023-07-18 16:20:07.000000000","message":"worth remembering that there are use cases that set `include_own\u003dTrue` and `exclude_others\u003dFalse` i.e. `include_own` does not in itself imply \"only 1 row required\"\n\n`include_own\u003dTrue and exclude_others\u003dTrue` DOES imply \"only 1 row required\" but as discussed it is perhaps clearer to explicitly pass limit\u003d1?","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":1850,"context_line":"        :param include_own: boolean that governs whether the row whose name"},{"line_number":1851,"context_line":"            matches the broker\u0027s path is included in the returned list. If"},{"line_number":1852,"context_line":"            True, that row is included, otherwise it is not included. Default"},{"line_number":1853,"context_line":"            is False."},{"line_number":1854,"context_line":"        :param exclude_others: boolean that governs whether the rows whose"},{"line_number":1855,"context_line":"            names do not match the broker\u0027s path are included in the returned"},{"line_number":1856,"context_line":"            list. If True, those rows are not included, otherwise they are"}],"source_content_type":"text/x-python","patch_set":1,"id":"847ecd7a_88e3b872","line":1853,"in_reply_to":"da9ac478_e933ac9c","updated":"2023-07-20 11:15:30.000000000","message":"Done","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1c86b4800fa56b0bc06afe4f839f3ea6bd6ba35c","unresolved":true,"context_lines":[{"line_number":1924,"context_line":"            include_own\u003dTrue, include_deleted\u003dTrue, exclude_others\u003dTrue,"},{"line_number":1925,"context_line":"            limit\u003d1)"},{"line_number":1926,"context_line":"        if shard_ranges:"},{"line_number":1927,"context_line":"            own_shard_range \u003d shard_ranges[0]"},{"line_number":1928,"context_line":"        elif no_default:"},{"line_number":1929,"context_line":"            return None"},{"line_number":1930,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"e52a57b1_d55b1fd2","line":1927,"updated":"2023-07-17 16:41:22.000000000","message":"even if the list had two rows (it shouldn\u0027t) we\u0027d already throw it away and not know - so doing in the limit in the database does NOT risk hiding a potential bug any worse than we are already.","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":1924,"context_line":"            include_own\u003dTrue, include_deleted\u003dTrue, exclude_others\u003dTrue,"},{"line_number":1925,"context_line":"            limit\u003d1)"},{"line_number":1926,"context_line":"        if shard_ranges:"},{"line_number":1927,"context_line":"            own_shard_range \u003d shard_ranges[0]"},{"line_number":1928,"context_line":"        elif no_default:"},{"line_number":1929,"context_line":"            return None"},{"line_number":1930,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fd0daa73_e6d1333a","line":1927,"in_reply_to":"e52a57b1_d55b1fd2","updated":"2023-07-20 11:15:30.000000000","message":"Ack","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a5faf60a1d06eb0a401538672b8cdcb0f00b92fd","unresolved":true,"context_lines":[{"line_number":1877,"context_line":"                marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":1878,"context_line":"                include_deleted\u003dinclude_deleted, states\u003dstates,"},{"line_number":1879,"context_line":"                include_own\u003dinclude_own, exclude_others\u003dexclude_others)]"},{"line_number":1880,"context_line":""},{"line_number":1881,"context_line":"        shard_ranges.sort(key\u003dShardRange.sort_key)"},{"line_number":1882,"context_line":"        if includes:"},{"line_number":1883,"context_line":"            return shard_ranges[:1] if shard_ranges else []"}],"source_content_type":"text/x-python","patch_set":2,"id":"ae3df348_1c2f8b81","line":1880,"updated":"2023-07-18 22:10:18.000000000","message":"leftover blank line?","commit_id":"f029965b7ad6b209268cbbdf4fc99d69237f6f38"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":1877,"context_line":"                marker\u003dmarker, end_marker\u003dend_marker, includes\u003dincludes,"},{"line_number":1878,"context_line":"                include_deleted\u003dinclude_deleted, states\u003dstates,"},{"line_number":1879,"context_line":"                include_own\u003dinclude_own, exclude_others\u003dexclude_others)]"},{"line_number":1880,"context_line":""},{"line_number":1881,"context_line":"        shard_ranges.sort(key\u003dShardRange.sort_key)"},{"line_number":1882,"context_line":"        if includes:"},{"line_number":1883,"context_line":"            return shard_ranges[:1] if shard_ranges else []"}],"source_content_type":"text/x-python","patch_set":2,"id":"3c91e297_20dee68e","line":1880,"in_reply_to":"ae3df348_1c2f8b81","updated":"2023-07-20 11:15:30.000000000","message":"Done","commit_id":"f029965b7ad6b209268cbbdf4fc99d69237f6f38"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"a5faf60a1d06eb0a401538672b8cdcb0f00b92fd","unresolved":true,"context_lines":[{"line_number":1919,"context_line":"        :return: an instance of :class:`~swift.common.utils.ShardRange`"},{"line_number":1920,"context_line":"        \"\"\""},{"line_number":1921,"context_line":"        rows \u003d self._get_shard_range_rows("},{"line_number":1922,"context_line":"            include_own\u003dTrue, include_deleted\u003dTrue, exclude_others\u003dTrue,"},{"line_number":1923,"context_line":"            limit\u003d1)"},{"line_number":1924,"context_line":"        if rows:"},{"line_number":1925,"context_line":"            own_shard_range \u003d ShardRange(*rows[0])"}],"source_content_type":"text/x-python","patch_set":2,"id":"f7527ebe_e5af0cd8","line":1922,"updated":"2023-07-18 22:10:18.000000000","message":"Looking the call place, I am leaning towards to skip \"limit\u003d1\" and add the limit internally within _get_shard_range_rows() for \"include_own\u003dTrue and exclude_others\u003dTrue\".","commit_id":"f029965b7ad6b209268cbbdf4fc99d69237f6f38"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":1919,"context_line":"        :return: an instance of :class:`~swift.common.utils.ShardRange`"},{"line_number":1920,"context_line":"        \"\"\""},{"line_number":1921,"context_line":"        rows \u003d self._get_shard_range_rows("},{"line_number":1922,"context_line":"            include_own\u003dTrue, include_deleted\u003dTrue, exclude_others\u003dTrue,"},{"line_number":1923,"context_line":"            limit\u003d1)"},{"line_number":1924,"context_line":"        if rows:"},{"line_number":1925,"context_line":"            own_shard_range \u003d ShardRange(*rows[0])"}],"source_content_type":"text/x-python","patch_set":2,"id":"02a36020_f1634ed2","line":1922,"in_reply_to":"f7527ebe_e5af0cd8","updated":"2023-07-20 11:15:30.000000000","message":"Discussed verbally: we do have at least one use case (in shard-info.py) where we use include_own\u003dTrue and exclude_others\u003dTrue but do NOT want to limit the results to 1 item.\n\nBut, there is also an argument that this method is merely constructing an SQL query and should therefore remain quite \u0027dumb\u0027 - it already has a rich interface and it may be better to keep expanding the interface (leave inference to the caller) rather than start to add inferences to the method.","commit_id":"f029965b7ad6b209268cbbdf4fc99d69237f6f38"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ddbf7c8b0f9c59685937fc4e45c5606167307cbe","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                params.extend((includes, includes))"},{"line_number":1768,"context_line":"            if conditions:"},{"line_number":1769,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1770,"context_line":"            if isinstance(limit, int) and limit \u003e\u003d 0:"},{"line_number":1771,"context_line":"                condition +\u003d \u0027 LIMIT %d\u0027 % limit"},{"line_number":1772,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1773,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"}],"source_content_type":"text/x-python","patch_set":5,"id":"dcb8675b_c9e7691b","line":1770,"updated":"2023-07-19 21:54:20.000000000","message":"this is where previous patch failed some test cases.","commit_id":"bbde27ce22f5b4cbf707a493e69eca98178d81fb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                params.extend((includes, includes))"},{"line_number":1768,"context_line":"            if conditions:"},{"line_number":1769,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1770,"context_line":"            if isinstance(limit, int) and limit \u003e\u003d 0:"},{"line_number":1771,"context_line":"                condition +\u003d \u0027 LIMIT %d\u0027 % limit"},{"line_number":1772,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1773,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"}],"source_content_type":"text/x-python","patch_set":5,"id":"c0501691_4a523b41","line":1770,"in_reply_to":"dcb8675b_c9e7691b","updated":"2023-07-20 11:15:30.000000000","message":"than you for fixing my mistake (I realise I was testing under py2.7 on my laptop, and in py2.7 \u0027None\u0027 could be compared with an int!)\n\nI\u0027m not sure about isinstance(limit, int) - this would silently ignore crazy values like 1.1 or \u00271.1\u0027 or \u0027one\u0027, and I think I\u0027d prefer those programmer errors to be raised rather than ignored. (The use of %d in my original on the next line is also suspect for the same reason)\n\nSo I think I\u0027d prefer \n\n```\nif limit is not None and limit \u003e\u003d 0:\n    condition +\u003d \u0027 LIMIT %s\u0027 % limit\n```\n\nso anything other than \u0027None\u0027 (the allowed default value) will blow up if not a valid data type (i.e not an int). At least, with py3.","commit_id":"bbde27ce22f5b4cbf707a493e69eca98178d81fb"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"5de660a87c0422a1c05691cdb008f3b6e8b4c5cc","unresolved":true,"context_lines":[{"line_number":1770,"context_line":"                params.extend((includes, includes))"},{"line_number":1771,"context_line":"            if conditions:"},{"line_number":1772,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1773,"context_line":"            if limit is not None and limit \u003e\u003d 0:"},{"line_number":1774,"context_line":"                condition +\u003d \u0027 LIMIT %s\u0027 % limit"},{"line_number":1775,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1776,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"21947d5a_870cb224","line":1773,"range":{"start_line":1773,"start_character":44,"end_line":1773,"end_character":47},"updated":"2023-07-24 04:41:09.000000000","message":"would we ever actually use LIMIT 0? That just returns an empty list. Not sure if we should make that an option?","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9dac66c6e2f3361234d49610fd211c46870e24be","unresolved":true,"context_lines":[{"line_number":1770,"context_line":"                params.extend((includes, includes))"},{"line_number":1771,"context_line":"            if conditions:"},{"line_number":1772,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1773,"context_line":"            if limit is not None and limit \u003e\u003d 0:"},{"line_number":1774,"context_line":"                condition +\u003d \u0027 LIMIT %s\u0027 % limit"},{"line_number":1775,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1776,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"e71041bd_be2a93aa","line":1773,"range":{"start_line":1773,"start_character":44,"end_line":1773,"end_character":47},"in_reply_to":"21947d5a_870cb224","updated":"2023-07-24 10:17:26.000000000","message":"I chose to support limit\u003d0 because the method is basically a helper for building a sql query and sqlite will handle LIMIT 0.\n\nThe immediate use case only requires limit\u003d1, so I could even have added a \"single_item\u003dTrue/False\" arg instead, but another way of seeing it is that the use case *for this _ method* is to build sql query from python args, so therefore limit\u003d0 should be supported (but not limit\u003c0 which sqlite would ignore.","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c856f5fdd472d821f683e2b7cfaea2406f042d51","unresolved":false,"context_lines":[{"line_number":1770,"context_line":"                params.extend((includes, includes))"},{"line_number":1771,"context_line":"            if conditions:"},{"line_number":1772,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1773,"context_line":"            if limit is not None and limit \u003e\u003d 0:"},{"line_number":1774,"context_line":"                condition +\u003d \u0027 LIMIT %s\u0027 % limit"},{"line_number":1775,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1776,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fb048f60_60f21bc7","line":1773,"range":{"start_line":1773,"start_character":44,"end_line":1773,"end_character":47},"in_reply_to":"d2500564_7e6ca979","updated":"2023-08-03 13:11:48.000000000","message":"Done","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"6802c43f90330aa2096183b70d89f8e993a49bd6","unresolved":true,"context_lines":[{"line_number":1770,"context_line":"                params.extend((includes, includes))"},{"line_number":1771,"context_line":"            if conditions:"},{"line_number":1772,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1773,"context_line":"            if limit is not None and limit \u003e\u003d 0:"},{"line_number":1774,"context_line":"                condition +\u003d \u0027 LIMIT %s\u0027 % limit"},{"line_number":1775,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1776,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"}],"source_content_type":"text/x-python","patch_set":6,"id":"d2500564_7e6ca979","line":1773,"range":{"start_line":1773,"start_character":44,"end_line":1773,"end_character":47},"in_reply_to":"e71041bd_be2a93aa","updated":"2023-07-25 00:22:13.000000000","message":"yeah ok, makes sense. So long as someone never uses `limit\u003d0` in the function and wonder why nothing is returned.. but that is user/developer error rather then error with the code.","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d5611916f01e5f58e34444db2840348dbac54bb6","unresolved":true,"context_lines":[{"line_number":1771,"context_line":"            if conditions:"},{"line_number":1772,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1773,"context_line":"            if limit is not None and limit \u003e\u003d 0:"},{"line_number":1774,"context_line":"                condition +\u003d \u0027 LIMIT %s\u0027 % limit"},{"line_number":1775,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1776,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"},{"line_number":1777,"context_line":"                if column in defaults:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f1a64f5_de23ac99","line":1774,"range":{"start_line":1774,"start_character":38,"end_line":1774,"end_character":39},"updated":"2023-08-02 22:26:25.000000000","message":"Little funny that this changed from `%d` to `%s` since I last looked at this patch. I think I preferred it before -- do we have a use-case for passing non-integers? It kinda looks like a SQL injection waiting to happen...","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0d3d4322fb0b205b0f30f17209d2f8df5b346140","unresolved":false,"context_lines":[{"line_number":1771,"context_line":"            if conditions:"},{"line_number":1772,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1773,"context_line":"            if limit is not None and limit \u003e\u003d 0:"},{"line_number":1774,"context_line":"                condition +\u003d \u0027 LIMIT %s\u0027 % limit"},{"line_number":1775,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1776,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"},{"line_number":1777,"context_line":"                if column in defaults:"}],"source_content_type":"text/x-python","patch_set":6,"id":"663e8dd6_ca6f0904","line":1774,"range":{"start_line":1774,"start_character":38,"end_line":1774,"end_character":39},"in_reply_to":"15f691af_30f6197e","updated":"2023-08-03 17:20:47.000000000","message":"```\n\u003e\u003e\u003e \u0027%d\u0027 % 1.1\n\u00271\u0027\n```\nOh jeeze, I forgot about that foot-gun! `str.format` does it better:\n```\n\u003e\u003e\u003e \u0027{0:d}\u0027.format(1.1)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\nValueError: Unknown format code \u0027d\u0027 for object of type \u0027float\u0027\n```\nbut I\u0027m perfectly content with the `%d`.","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c856f5fdd472d821f683e2b7cfaea2406f042d51","unresolved":true,"context_lines":[{"line_number":1771,"context_line":"            if conditions:"},{"line_number":1772,"context_line":"                condition \u003d \u0027 WHERE \u0027 + \u0027 AND \u0027.join(conditions)"},{"line_number":1773,"context_line":"            if limit is not None and limit \u003e\u003d 0:"},{"line_number":1774,"context_line":"                condition +\u003d \u0027 LIMIT %s\u0027 % limit"},{"line_number":1775,"context_line":"            columns \u003d SHARD_RANGE_KEYS[:-2]"},{"line_number":1776,"context_line":"            for column in SHARD_RANGE_KEYS[-2:]:"},{"line_number":1777,"context_line":"                if column in defaults:"}],"source_content_type":"text/x-python","patch_set":6,"id":"15f691af_30f6197e","line":1774,"range":{"start_line":1774,"start_character":38,"end_line":1774,"end_character":39},"in_reply_to":"7f1a64f5_de23ac99","updated":"2023-08-03 13:11:48.000000000","message":"I think I maybe changed it so that sqlite would blow up if a float was passed in for limit:\n\n```\n\u003e       actual \u003d broker._get_shard_range_rows(include_deleted\u003dTrue, limit\u003d1.1)\n\ntest_backend.py:4652: \n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \n../../../swift/container/backend.py:1796: in _get_shard_range_rows\n    return do_query(conn, defaults)\n../../../swift/container/backend.py:1786: in do_query\n    data \u003d conn.execute(sql, params)\n../../../swift/common/db.py:152: in execute\n    curs.execute(*args, **kwargs)\n../../../swift/common/db.py:173: in execute\n    return _db_timeout(\n../../../swift/common/db.py:99: in _db_timeout\n    return call()\n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \n\n\u003e   self.timeout, self.db_file, lambda: sqlite3.Cursor.execute(\n        self, *args, **kwargs))\nE       sqlite3.IntegrityError: datatype mismatch\n\n../../../swift/common/db.py:174: IntegrityError\n```\n\nbut I take your point!","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d5611916f01e5f58e34444db2840348dbac54bb6","unresolved":true,"context_lines":[{"line_number":1934,"context_line":"            default shard range is returned."},{"line_number":1935,"context_line":"        :return: an instance of :class:`~swift.common.utils.ShardRange`"},{"line_number":1936,"context_line":"        \"\"\""},{"line_number":1937,"context_line":"        rows \u003d self._get_shard_range_rows("},{"line_number":1938,"context_line":"            include_own\u003dTrue, include_deleted\u003dTrue, exclude_others\u003dTrue,"},{"line_number":1939,"context_line":"            limit\u003d1)"},{"line_number":1940,"context_line":"        if rows:"}],"source_content_type":"text/x-python","patch_set":6,"id":"93851c51_350a267f","line":1937,"updated":"2023-08-02 22:26:25.000000000","message":"Peeling off that layer makes a lot of sense given the kwargs we use and saves us some plumbing 👍","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c856f5fdd472d821f683e2b7cfaea2406f042d51","unresolved":false,"context_lines":[{"line_number":1934,"context_line":"            default shard range is returned."},{"line_number":1935,"context_line":"        :return: an instance of :class:`~swift.common.utils.ShardRange`"},{"line_number":1936,"context_line":"        \"\"\""},{"line_number":1937,"context_line":"        rows \u003d self._get_shard_range_rows("},{"line_number":1938,"context_line":"            include_own\u003dTrue, include_deleted\u003dTrue, exclude_others\u003dTrue,"},{"line_number":1939,"context_line":"            limit\u003d1)"},{"line_number":1940,"context_line":"        if rows:"}],"source_content_type":"text/x-python","patch_set":6,"id":"02ac4935_b8af686f","line":1937,"in_reply_to":"93851c51_350a267f","updated":"2023-08-03 13:11:48.000000000","message":"Ack","commit_id":"0a70315e2bdbb235eabb08dbda96f35a2a848700"}],"test/unit/container/test_backend.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"17c93c0394cd1eaa5ce2df7c1b4bb0e166e24729","unresolved":true,"context_lines":[{"line_number":4192,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4193,"context_line":""},{"line_number":4194,"context_line":"        actual \u003d broker.get_shard_ranges(limit\u003d1)"},{"line_number":4195,"context_line":"        undeleted \u003d shard_ranges[:3] + shard_ranges[4:5]"},{"line_number":4196,"context_line":"        self.assertEqual([dict(undeleted[0])], [dict(sr) for sr in actual])"},{"line_number":4197,"context_line":""},{"line_number":4198,"context_line":"        actual \u003d broker.get_shard_ranges(include_deleted\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"10e25224_c53e23c8","line":4195,"updated":"2023-07-14 16:36:05.000000000","message":"Don\u0027t we already have this? See L4190","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":4192,"context_line":"                         [dict(sr) for sr in actual])"},{"line_number":4193,"context_line":""},{"line_number":4194,"context_line":"        actual \u003d broker.get_shard_ranges(limit\u003d1)"},{"line_number":4195,"context_line":"        undeleted \u003d shard_ranges[:3] + shard_ranges[4:5]"},{"line_number":4196,"context_line":"        self.assertEqual([dict(undeleted[0])], [dict(sr) for sr in actual])"},{"line_number":4197,"context_line":""},{"line_number":4198,"context_line":"        actual \u003d broker.get_shard_ranges(include_deleted\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e2f34d0b_eaab43d1","line":4195,"in_reply_to":"10e25224_c53e23c8","updated":"2023-07-20 11:15:30.000000000","message":"Done","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"17c93c0394cd1eaa5ce2df7c1b4bb0e166e24729","unresolved":true,"context_lines":[{"line_number":4193,"context_line":""},{"line_number":4194,"context_line":"        actual \u003d broker.get_shard_ranges(limit\u003d1)"},{"line_number":4195,"context_line":"        undeleted \u003d shard_ranges[:3] + shard_ranges[4:5]"},{"line_number":4196,"context_line":"        self.assertEqual([dict(undeleted[0])], [dict(sr) for sr in actual])"},{"line_number":4197,"context_line":""},{"line_number":4198,"context_line":"        actual \u003d broker.get_shard_ranges(include_deleted\u003dTrue)"},{"line_number":4199,"context_line":"        self.assertEqual([dict(sr) for sr in shard_ranges],"}],"source_content_type":"text/x-python","patch_set":1,"id":"0b8ed957_089bd0e3","line":4196,"range":{"start_line":4196,"start_character":31,"end_line":4196,"end_character":43},"updated":"2023-07-14 16:36:05.000000000","message":"It looks like (at least on py2) we aren\u0027t guaranteed to get this shard -- probably due to our lack of any sort of `ORDER BY` clause.\n\nMakes me wonder whether there\u0027s any value to having `limit` outside of `get_own_shard_range` (where we expect either 0 or 1 row).\n\nMaybe we could skip plumbing in `limit` and have `get_shard_ranges` automatically add the `LIMIT 1` when `include_own and exclude_others`?","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":4193,"context_line":""},{"line_number":4194,"context_line":"        actual \u003d broker.get_shard_ranges(limit\u003d1)"},{"line_number":4195,"context_line":"        undeleted \u003d shard_ranges[:3] + shard_ranges[4:5]"},{"line_number":4196,"context_line":"        self.assertEqual([dict(undeleted[0])], [dict(sr) for sr in actual])"},{"line_number":4197,"context_line":""},{"line_number":4198,"context_line":"        actual \u003d broker.get_shard_ranges(include_deleted\u003dTrue)"},{"line_number":4199,"context_line":"        self.assertEqual([dict(sr) for sr in shard_ranges],"}],"source_content_type":"text/x-python","patch_set":1,"id":"b37c70bd_c9a7a490","line":4196,"range":{"start_line":4196,"start_character":31,"end_line":4196,"end_character":43},"in_reply_to":"0b8ed957_089bd0e3","updated":"2023-07-20 11:15:30.000000000","message":"see discussion in other comments...\n\n1) in shard_info.py we use include_own and exclude_others and do not filter the result\n\n2) argument to keep the _get_shard_range_rows method implementation dumb (it just assembles and executes SQL) and leave callers to \"drive\" the method interface","commit_id":"197a29d78b23f679b17d371a132503a042689582"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ddbf7c8b0f9c59685937fc4e45c5606167307cbe","unresolved":false,"context_lines":[{"line_number":4500,"context_line":"        self.assertEqual([shard_ranges[2]], actual)"},{"line_number":4501,"context_line":""},{"line_number":4502,"context_line":"    @with_tempdir"},{"line_number":4503,"context_line":"    def test_get_shard_range_rows_with_limit(self, tempdir):"},{"line_number":4504,"context_line":"        db_path \u003d os.path.join(tempdir, \u0027container.db\u0027)"},{"line_number":4505,"context_line":"        broker \u003d ContainerBroker(db_path, account\u003d\u0027a\u0027, container\u003d\u0027c\u0027)"},{"line_number":4506,"context_line":"        broker.initialize(next(self.ts).internal, 0)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9cd95dd9_44cf940e","line":4503,"updated":"2023-07-19 21:54:20.000000000","message":"complete tests, nice!","commit_id":"bbde27ce22f5b4cbf707a493e69eca98178d81fb"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"ddbf7c8b0f9c59685937fc4e45c5606167307cbe","unresolved":false,"context_lines":[{"line_number":4524,"context_line":"        self.assertEqual(3, len(actual))"},{"line_number":4525,"context_line":"        self.assertEqual(3, len(set(actual)))"},{"line_number":4526,"context_line":"        # zero is applied"},{"line_number":4527,"context_line":"        actual \u003d broker._get_shard_range_rows(include_deleted\u003dTrue, limit\u003d0)"},{"line_number":4528,"context_line":"        self.assertFalse(actual)"},{"line_number":4529,"context_line":"        actual \u003d broker._get_shard_range_rows(include_deleted\u003dTrue, limit\u003d1)"},{"line_number":4530,"context_line":"        self.assertEqual(1, len(actual))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3b909828_d60ab792","line":4527,"updated":"2023-07-19 21:54:20.000000000","message":"\"limit\u003d0\" is kind of weird, but it\u0027s okay since this is an internal function.","commit_id":"bbde27ce22f5b4cbf707a493e69eca98178d81fb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"52c65bb3a9d255ddb6b0a31c5567d9cc063454ad","unresolved":false,"context_lines":[{"line_number":4524,"context_line":"        self.assertEqual(3, len(actual))"},{"line_number":4525,"context_line":"        self.assertEqual(3, len(set(actual)))"},{"line_number":4526,"context_line":"        # zero is applied"},{"line_number":4527,"context_line":"        actual \u003d broker._get_shard_range_rows(include_deleted\u003dTrue, limit\u003d0)"},{"line_number":4528,"context_line":"        self.assertFalse(actual)"},{"line_number":4529,"context_line":"        actual \u003d broker._get_shard_range_rows(include_deleted\u003dTrue, limit\u003d1)"},{"line_number":4530,"context_line":"        self.assertEqual(1, len(actual))"}],"source_content_type":"text/x-python","patch_set":5,"id":"b063b018_3c82786b","line":4527,"in_reply_to":"3b909828_d60ab792","updated":"2023-07-20 11:15:30.000000000","message":"ack, it is weird but I wanted the behaviour to reflect the SQL behaviour - SQL LIMIT 0 is a thing, whereas AFAICT LIMIT -1 is ignored","commit_id":"bbde27ce22f5b4cbf707a493e69eca98178d81fb"}]}