)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f351f627979100349f8b646da246a915afde55cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"48c90fa0_258a74c8","updated":"2024-04-29 16:16:09.000000000","message":"I\u0027m rather surprised the py3 tests passed...","commit_id":"dea863b7b31f4d87ee21a3e51057f12a21690868"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c6cf5bb19c5323511f58832d389271357b901c95","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"cdf5239a_f7714bb7","updated":"2024-04-29 22:26:40.000000000","message":"I think the only alternative to fixing the broker is to re-work the test.","commit_id":"1d9bab849ca069deb18c019eee5bfaae56123dd6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"05589af3e03d2141dd95e670265c4c7049b48f6a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"dd841ea5_75ac1a17","updated":"2024-05-02 16:08:29.000000000","message":"recheck\n\nLooks like another probe test hang:\n```\n2024-05-02 10:45:22.961194 | centos-9-stream | test/probe/test_sharder.py::TestManagedContainerSharding::test_manage_shard_ranges_compact PASSED [ 90%]\n2024-05-02 10:45:57.716844 | centos-9-stream | test/probe/test_sharder.py::TestManagedContainerSharding::test_manage_shard_ranges_deleted_child_and_parent_gap PASSED [ 91%]\n2024-05-02 11:38:01.246105 | RUN END RESULT_TIMED_OUT: [untrusted : opendev.org/openstack/swift/tools/playbooks/probetests/run.yaml@feature/mpu]\n```","commit_id":"9eda38ced93274e71ce974251514d80e84426799"}],"swift/container/backend.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c6cf5bb19c5323511f58832d389271357b901c95","unresolved":false,"context_lines":[{"line_number":1363,"context_line":"            # Sort item_list into things that need adding and deleting, based"},{"line_number":1364,"context_line":"            # on results of created_at query."},{"line_number":1365,"context_line":"            to_delete \u003d set()"},{"line_number":1366,"context_line":"            to_add \u003d collections.OrderedDict()"},{"line_number":1367,"context_line":"            for item in item_list:"},{"line_number":1368,"context_line":"                item.setdefault(\u0027storage_policy_index\u0027, 0)  # legacy"},{"line_number":1369,"context_line":"                item_ident \u003d (item[\u0027name\u0027], item[\u0027storage_policy_index\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"18862fc6_cae2dbdc","line":1366,"updated":"2024-04-29 22:26:40.000000000","message":"Still needed to fix the ordering issue for py2.","commit_id":"1d9bab849ca069deb18c019eee5bfaae56123dd6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"170d0b0e8a5b4027f6e5dc30d98824d7de758c4e","unresolved":false,"context_lines":[{"line_number":1363,"context_line":"            # Sort item_list into things that need adding and deleting, based"},{"line_number":1364,"context_line":"            # on results of created_at query."},{"line_number":1365,"context_line":"            to_delete \u003d set()"},{"line_number":1366,"context_line":"            to_add \u003d collections.OrderedDict()"},{"line_number":1367,"context_line":"            for item in item_list:"},{"line_number":1368,"context_line":"                item.setdefault(\u0027storage_policy_index\u0027, 0)  # legacy"},{"line_number":1369,"context_line":"                item_ident \u003d (item[\u0027name\u0027], item[\u0027storage_policy_index\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"f4dbb4d1_662b071f","line":1366,"in_reply_to":"101f6c46_8e885132","updated":"2024-05-02 09:25:44.000000000","message":"I think that out-of-band we agreed this would be best fixed on master (and I also see there may be existing workarounds to fix https://review.opendev.org/c/openstack/swift/+/905064/5/test/unit/container/test_backend.py) so I\u0027m going to revert this and fix the test setup instead.","commit_id":"1d9bab849ca069deb18c019eee5bfaae56123dd6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6e7763632d6750e0a0cd024a8bfdeb906f247a55","unresolved":false,"context_lines":[{"line_number":1363,"context_line":"            # Sort item_list into things that need adding and deleting, based"},{"line_number":1364,"context_line":"            # on results of created_at query."},{"line_number":1365,"context_line":"            to_delete \u003d set()"},{"line_number":1366,"context_line":"            to_add \u003d collections.OrderedDict()"},{"line_number":1367,"context_line":"            for item in item_list:"},{"line_number":1368,"context_line":"                item.setdefault(\u0027storage_policy_index\u0027, 0)  # legacy"},{"line_number":1369,"context_line":"                item_ident \u003d (item[\u0027name\u0027], item[\u0027storage_policy_index\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"101f6c46_8e885132","line":1366,"in_reply_to":"18862fc6_cae2dbdc","updated":"2024-04-30 09:28:18.000000000","message":"This is wholly reasonable, and I expect most would be surprised to learn that the row insertion order was not the same as the order of items passed in.\n\nI\u0027m just a little wary of this change being a drive-by of something completely unrelated, motivated by a lazy test. If this regressed I\u0027d expect something it test_backend.py to fail.\n\nFWIW the workaround isn\u0027t too painful https://review.opendev.org/c/openstack/swift/+/917596\n\nlet\u0027s discuss which way to go","commit_id":"1d9bab849ca069deb18c019eee5bfaae56123dd6"}],"test/unit/container/test_mpu_auditor.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4bcef1d6ad305cba5db0e53af890cdab123cf77","unresolved":true,"context_lines":[{"line_number":71,"context_line":"    def put_objects(self, objects, shuffle_order\u003dTrue):"},{"line_number":72,"context_line":"        # PUT object updates to container in random order"},{"line_number":73,"context_line":"        if shuffle_order:"},{"line_number":74,"context_line":"            random.shuffle(objects)"},{"line_number":75,"context_line":"        for obj in objects:"},{"line_number":76,"context_line":"            headers \u003d {\u0027name\u0027: obj[\u0027name\u0027],"},{"line_number":77,"context_line":"                       \u0027x-timestamp\u0027: obj[\u0027created_at\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"6e6e5b09_f4307144","line":74,"updated":"2024-04-29 16:59:00.000000000","message":"here we mutate the object list that was passed in so it should match the row order in db (that wasn\u0027t my intent, but AFAICT it\u0027s why the test passes on py3)","commit_id":"dea863b7b31f4d87ee21a3e51057f12a21690868"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"170d0b0e8a5b4027f6e5dc30d98824d7de758c4e","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    def put_objects(self, objects, shuffle_order\u003dTrue):"},{"line_number":72,"context_line":"        # PUT object updates to container in random order"},{"line_number":73,"context_line":"        if shuffle_order:"},{"line_number":74,"context_line":"            random.shuffle(objects)"},{"line_number":75,"context_line":"        for obj in objects:"},{"line_number":76,"context_line":"            headers \u003d {\u0027name\u0027: obj[\u0027name\u0027],"},{"line_number":77,"context_line":"                       \u0027x-timestamp\u0027: obj[\u0027created_at\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"c1e3b712_0c779b03","line":74,"in_reply_to":"4c6ad470_b3728543","updated":"2024-05-02 09:25:44.000000000","message":"Acknowledged","commit_id":"dea863b7b31f4d87ee21a3e51057f12a21690868"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8ddf8a5b78e532b77bfbec4ce73b0531ddbb4336","unresolved":true,"context_lines":[{"line_number":71,"context_line":"    def put_objects(self, objects, shuffle_order\u003dTrue):"},{"line_number":72,"context_line":"        # PUT object updates to container in random order"},{"line_number":73,"context_line":"        if shuffle_order:"},{"line_number":74,"context_line":"            random.shuffle(objects)"},{"line_number":75,"context_line":"        for obj in objects:"},{"line_number":76,"context_line":"            headers \u003d {\u0027name\u0027: obj[\u0027name\u0027],"},{"line_number":77,"context_line":"                       \u0027x-timestamp\u0027: obj[\u0027created_at\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"4c6ad470_b3728543","line":74,"in_reply_to":"6e6e5b09_f4307144","updated":"2024-04-29 17:25:25.000000000","message":"Hmm... makes me wonder if we might want to use an `OrderedDict` at https://github.com/openstack/swift/blob/c073933387347bec7a917f34ca7e2f15993b33a5/swift/container/backend.py#L1365 ... The shuffling on py2 seems unfortunate.","commit_id":"dea863b7b31f4d87ee21a3e51057f12a21690868"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f351f627979100349f8b646da246a915afde55cd","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        # stop at max batches"},{"line_number":152,"context_line":"        actual \u003d [b for b in yield_item_batches(self.broker, 0, 2, 50)]"},{"line_number":153,"context_line":"        self.assertEqual(2, len(actual))"},{"line_number":154,"context_line":"        self.assertEqual([it[\u0027name\u0027] for it in items[:50]],"},{"line_number":155,"context_line":"                         [a[\u0027name\u0027] for a in actual[0]])"},{"line_number":156,"context_line":"        self.assertEqual([it[\u0027name\u0027] for it in items[50:100]],"},{"line_number":157,"context_line":"                         [a[\u0027name\u0027] for a in actual[1]])"}],"source_content_type":"text/x-python","patch_set":4,"id":"c11a510a_0eb90050","line":154,"range":{"start_line":154,"start_character":47,"end_line":154,"end_character":52},"updated":"2024-04-29 16:16:09.000000000","message":"Won\u0027t these need to be sorted?","commit_id":"dea863b7b31f4d87ee21a3e51057f12a21690868"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"170d0b0e8a5b4027f6e5dc30d98824d7de758c4e","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        # stop at max batches"},{"line_number":152,"context_line":"        actual \u003d [b for b in yield_item_batches(self.broker, 0, 2, 50)]"},{"line_number":153,"context_line":"        self.assertEqual(2, len(actual))"},{"line_number":154,"context_line":"        self.assertEqual([it[\u0027name\u0027] for it in items[:50]],"},{"line_number":155,"context_line":"                         [a[\u0027name\u0027] for a in actual[0]])"},{"line_number":156,"context_line":"        self.assertEqual([it[\u0027name\u0027] for it in items[50:100]],"},{"line_number":157,"context_line":"                         [a[\u0027name\u0027] for a in actual[1]])"}],"source_content_type":"text/x-python","patch_set":4,"id":"dc0ce310_28d981a2","line":154,"range":{"start_line":154,"start_character":47,"end_line":154,"end_character":52},"in_reply_to":"6b155c09_769dc368","updated":"2024-05-02 09:25:44.000000000","message":"Done","commit_id":"dea863b7b31f4d87ee21a3e51057f12a21690868"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4bcef1d6ad305cba5db0e53af890cdab123cf77","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        # stop at max batches"},{"line_number":152,"context_line":"        actual \u003d [b for b in yield_item_batches(self.broker, 0, 2, 50)]"},{"line_number":153,"context_line":"        self.assertEqual(2, len(actual))"},{"line_number":154,"context_line":"        self.assertEqual([it[\u0027name\u0027] for it in items[:50]],"},{"line_number":155,"context_line":"                         [a[\u0027name\u0027] for a in actual[0]])"},{"line_number":156,"context_line":"        self.assertEqual([it[\u0027name\u0027] for it in items[50:100]],"},{"line_number":157,"context_line":"                         [a[\u0027name\u0027] for a in actual[1]])"}],"source_content_type":"text/x-python","patch_set":4,"id":"6b155c09_769dc368","line":154,"range":{"start_line":154,"start_character":47,"end_line":154,"end_character":52},"in_reply_to":"c11a510a_0eb90050","updated":"2024-04-29 16:59:00.000000000","message":"welllll...yes but no 😊\n\nif put_objects behaved as intended these need sorting, but put_objects is mutating items to be in the same order as the db updates 😞\n\nWhich begs question , why does this fail under py2.7 ? I think that may be because there\u0027s another randomisation in the broker when it organises the pending commits into a dict and then reads dict.values() - in py3 that would maintain order, but not in py2.7.\n\nhere:\nhttps://github.com/openstack/swift/blob/c073933387347bec7a917f34ca7e2f15993b33a5/swift/container/backend.py#L1381-L1390\n\nSo I *think* I had intended to call put_objects with shuffle_order\u003dTrue to avoid this anyway. sigh. But at least we caught a bug in out_objects()","commit_id":"dea863b7b31f4d87ee21a3e51057f12a21690868"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8ddf8a5b78e532b77bfbec4ce73b0531ddbb4336","unresolved":true,"context_lines":[{"line_number":71,"context_line":"    def put_objects(self, objects, shuffle_order\u003dTrue):"},{"line_number":72,"context_line":"        # PUT object updates to container in random order"},{"line_number":73,"context_line":"        if shuffle_order:"},{"line_number":74,"context_line":"            # don\u0027t mutate the passed-in list!"},{"line_number":75,"context_line":"            objects \u003d [o for o in objects]"},{"line_number":76,"context_line":"            random.shuffle(objects)"},{"line_number":77,"context_line":"        for obj in objects:"}],"source_content_type":"text/x-python","patch_set":5,"id":"6532cfeb_e3ecb1c7","line":74,"updated":"2024-04-29 17:25:25.000000000","message":"Alternatively, [docs recommend](https://docs.python.org/3/library/random.html#random.shuffle)\n```\nsample(x, k\u003dlen(x))\n```\n\\*shrug\\*","commit_id":"9485d690f1228dc511df7f0823a15722c5ff0a80"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"170d0b0e8a5b4027f6e5dc30d98824d7de758c4e","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    def put_objects(self, objects, shuffle_order\u003dTrue):"},{"line_number":72,"context_line":"        # PUT object updates to container in random order"},{"line_number":73,"context_line":"        if shuffle_order:"},{"line_number":74,"context_line":"            # don\u0027t mutate the passed-in list!"},{"line_number":75,"context_line":"            objects \u003d [o for o in objects]"},{"line_number":76,"context_line":"            random.shuffle(objects)"},{"line_number":77,"context_line":"        for obj in objects:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fff2a82d_62ae361d","line":74,"in_reply_to":"6532cfeb_e3ecb1c7","updated":"2024-05-02 09:25:44.000000000","message":"Done","commit_id":"9485d690f1228dc511df7f0823a15722c5ff0a80"}]}
