)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ed7d4bd0_13e61793","updated":"2025-09-16 13:53:49.000000000","message":"This clarifies the *outcome* of the setUp by asserting what files have been created, which is really useful, but doesn\u0027t make the existing set up code any clearer in itself.\n\nI\u0027m not sure why the set up is so convoluted and hard to understand when the outcome is apparently quite deterministic and simple to express? I\u0027d rather invest in making the setup code more explicit and therefore easier to understand in itself: see https://review.opendev.org/c/openstack/swift/+/961370","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f5f5c8d3351cb2d95408de813ad508b34fbaf402","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d324bbf7_450bae78","updated":"2025-09-16 14:47:17.000000000","message":"again, this is was just in my way towards 961322: test: do not create timestamp collision unnecessarily | https://review.opendev.org/c/openstack/swift/+/961322\n\nHaving spent some time relearning with this setup code does I\u0027d be quite happy to see it all go away!\n\n961370: tests: simplify TestGlobalSetupObjectReconstructor setUp | https://review.opendev.org/c/openstack/swift/+/961370","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"}],"test/unit/obj/test_reconstructor.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":true,"context_lines":[{"line_number":217,"context_line":"            # 1) part dir with all FI\u0027s matching the local node index"},{"line_number":218,"context_line":"            # 2) part dir with one local and mix of others"},{"line_number":219,"context_line":"            # 3) part dir with no local FI and one or more others"},{"line_number":220,"context_line":"            def part_0(set):"},{"line_number":221,"context_line":"                if set \u003d\u003d 0:"},{"line_number":222,"context_line":"                    # just the local"},{"line_number":223,"context_line":"                    return local_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"970e3523_534e760b","side":"PARENT","line":220,"updated":"2025-09-16 13:53:49.000000000","message":"these are so weird: ``set`` is ``obj_set`` passed in at line 262, but ``obj_num`` is just read from the enclosing scope, set at line 259!\n\n+1 for replacing ``set`` as a variable name","commit_id":"7cdbd21548789d5fe88dc7ac8f8653288bb8f87f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":true,"context_lines":[{"line_number":250,"context_line":"            # function dictionary for defining test scenarios base on set #"},{"line_number":251,"context_line":"            scenarios \u003d {\u00270\u0027: part_0,"},{"line_number":252,"context_line":"                         \u00271\u0027: part_1,"},{"line_number":253,"context_line":"                         \u00272\u0027: part_2}"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"            for part_num in self.part_nums:"},{"line_number":256,"context_line":"                # create 3 unique objects per part, each part"}],"source_content_type":"text/x-python","patch_set":1,"id":"183b8e5f_f5de2782","side":"PARENT","line":253,"updated":"2025-09-16 13:53:49.000000000","message":"...just in case you were close to tracking the all this setup in your brain, let\u0027s throw in some more mapping... 😄","commit_id":"7cdbd21548789d5fe88dc7ac8f8653288bb8f87f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":true,"context_lines":[{"line_number":213,"context_line":"        def _create_frag_archives(policy, obj_path, local_id, obj_set):"},{"line_number":214,"context_line":"            # we\u0027ll create 2 sets of objects in different suffix dirs"},{"line_number":215,"context_line":"            # so we cover all the scenarios we want (3 of them)"},{"line_number":216,"context_line":"            # 1) part dir with all FI\u0027s matching the local node index"},{"line_number":217,"context_line":"            # 2) part dir with one local and mix of others"},{"line_number":218,"context_line":"            # 3) part dir with no local FI and one or more others"},{"line_number":219,"context_line":"            def part_0(obj_num):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5869fef9_043aea46","line":216,"updated":"2025-09-16 13:53:49.000000000","message":"this doesn\u0027t seem to be true","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f5f5c8d3351cb2d95408de813ad508b34fbaf402","unresolved":true,"context_lines":[{"line_number":213,"context_line":"        def _create_frag_archives(policy, obj_path, local_id, obj_set):"},{"line_number":214,"context_line":"            # we\u0027ll create 2 sets of objects in different suffix dirs"},{"line_number":215,"context_line":"            # so we cover all the scenarios we want (3 of them)"},{"line_number":216,"context_line":"            # 1) part dir with all FI\u0027s matching the local node index"},{"line_number":217,"context_line":"            # 2) part dir with one local and mix of others"},{"line_number":218,"context_line":"            # 3) part dir with no local FI and one or more others"},{"line_number":219,"context_line":"            def part_0(obj_num):"}],"source_content_type":"text/x-python","patch_set":1,"id":"c02e5628_19d56b40","line":216,"in_reply_to":"5869fef9_043aea46","updated":"2025-09-16 14:47:17.000000000","message":"hey I didn\u0027t change NOTHING!  😜","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":true,"context_lines":[{"line_number":214,"context_line":"            # we\u0027ll create 2 sets of objects in different suffix dirs"},{"line_number":215,"context_line":"            # so we cover all the scenarios we want (3 of them)"},{"line_number":216,"context_line":"            # 1) part dir with all FI\u0027s matching the local node index"},{"line_number":217,"context_line":"            # 2) part dir with one local and mix of others"},{"line_number":218,"context_line":"            # 3) part dir with no local FI and one or more others"},{"line_number":219,"context_line":"            def part_0(obj_num):"},{"line_number":220,"context_line":"                if obj_set \u003d\u003d 0:"}],"source_content_type":"text/x-python","patch_set":1,"id":"348ae3ea_01b1c741","line":217,"updated":"2025-09-16 13:53:49.000000000","message":"this is part 0?","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":true,"context_lines":[{"line_number":215,"context_line":"            # so we cover all the scenarios we want (3 of them)"},{"line_number":216,"context_line":"            # 1) part dir with all FI\u0027s matching the local node index"},{"line_number":217,"context_line":"            # 2) part dir with one local and mix of others"},{"line_number":218,"context_line":"            # 3) part dir with no local FI and one or more others"},{"line_number":219,"context_line":"            def part_0(obj_num):"},{"line_number":220,"context_line":"                if obj_set \u003d\u003d 0:"},{"line_number":221,"context_line":"                    # just the local"}],"source_content_type":"text/x-python","patch_set":1,"id":"ddd2e52d_7e664153","line":218,"updated":"2025-09-16 13:53:49.000000000","message":"this seems to describe part 1 and part 2","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":true,"context_lines":[{"line_number":216,"context_line":"            # 1) part dir with all FI\u0027s matching the local node index"},{"line_number":217,"context_line":"            # 2) part dir with one local and mix of others"},{"line_number":218,"context_line":"            # 3) part dir with no local FI and one or more others"},{"line_number":219,"context_line":"            def part_0(obj_num):"},{"line_number":220,"context_line":"                if obj_set \u003d\u003d 0:"},{"line_number":221,"context_line":"                    # just the local"},{"line_number":222,"context_line":"                    return local_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e94d93e_9f961096","line":219,"updated":"2025-09-16 13:53:49.000000000","message":"but now ``obj_num`` is passed in and ``obj_set`` is taken from the enclosing scope ?!?\n\nI\u0027d suggest that it would be clearer to pass in ``obj_set`` *and* ``obj_num``, or neither, but I\u0027ve actually concluded that none of this necessary, see line 352.","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f5f5c8d3351cb2d95408de813ad508b34fbaf402","unresolved":true,"context_lines":[{"line_number":216,"context_line":"            # 1) part dir with all FI\u0027s matching the local node index"},{"line_number":217,"context_line":"            # 2) part dir with one local and mix of others"},{"line_number":218,"context_line":"            # 3) part dir with no local FI and one or more others"},{"line_number":219,"context_line":"            def part_0(obj_num):"},{"line_number":220,"context_line":"                if obj_set \u003d\u003d 0:"},{"line_number":221,"context_line":"                    # just the local"},{"line_number":222,"context_line":"                    return local_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e491de3_983b4872","line":219,"in_reply_to":"6e94d93e_9f961096","updated":"2025-09-16 14:47:17.000000000","message":"Since obj_set comes in as a parameter to this function and doesn\u0027t change it seemed like a reasonable candidate for a closure.\n\nWhen I first read the code I couldn\u0027t even grok that as the loop iterates and updates obj_num calling the closure would get the *current* value of obj_num in the loop!?  So I passed it in to make it more obvious it was a \"variable variable\" and not just some static reference to a non-local name.\n\nI much appreciate getting rid of this method:\n\n961370: tests: simplify TestGlobalSetupObjectReconstructor setUp | https://review.opendev.org/c/openstack/swift/+/961370","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ddaa95b767307aa0d92c0d2e4e11573116f7b366","unresolved":true,"context_lines":[{"line_number":349,"context_line":"                (\u0027/a/c/o0\u0027, \u00273c1\u0027, 2, False, False),"},{"line_number":350,"context_line":"                (\u0027/a/c/o1\u0027, \u0027061\u0027, 0, False, False),"},{"line_number":351,"context_line":"            },"},{"line_number":352,"context_line":"        }, part_map)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def tearDown(self):"},{"line_number":355,"context_line":"        rmtree(self.testdir, ignore_errors\u003d1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"44aedb8e_c0df6488","line":352,"updated":"2025-09-16 13:53:49.000000000","message":"I began my review by trying to understand the existing setup code, and getting very confused. I therefore appreciated these assertions that describe the outcome, but also realised that the outcome is *predictable* because the policy and rings are all faked in a predictable way, and the parameterised fragment setup code seems totally unnecessary. We already had a comment telling us exactly what fragments should be in what partition, and now we have assertions of the same, so why not just create them *explicitly* that way?\n\nSee https://review.opendev.org/c/openstack/swift/+/961370 for an alternative","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f5f5c8d3351cb2d95408de813ad508b34fbaf402","unresolved":true,"context_lines":[{"line_number":349,"context_line":"                (\u0027/a/c/o0\u0027, \u00273c1\u0027, 2, False, False),"},{"line_number":350,"context_line":"                (\u0027/a/c/o1\u0027, \u0027061\u0027, 0, False, False),"},{"line_number":351,"context_line":"            },"},{"line_number":352,"context_line":"        }, part_map)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def tearDown(self):"},{"line_number":355,"context_line":"        rmtree(self.testdir, ignore_errors\u003d1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3157b5e3_d8d030d6","line":352,"in_reply_to":"44aedb8e_c0df6488","updated":"2025-09-16 14:47:17.000000000","message":"I had the same thought, but figured adding essentially \"read-only\" code would be easier to get merged.\n\nThe only reason I care is because the convoluted setup code was at times recreating the exact same data frag with the same name, frag_index and timestamp twice in the same setup.  This was my path towards convincing myself I don\u0027t loose anything by adding \"if already_created: continue\"\n\nsee: https://review.opendev.org/c/openstack/swift/+/961322/1/test/unit/obj/test_reconstructor.py#262","commit_id":"aef4a4a48690ed9b9f3ea4db02228c00a0d600d6"}]}