)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"114946860ade3428f2737d7978d9a394497e7e7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"84a07d21_8d4f47ae","updated":"2024-01-08 09:59:27.000000000","message":"Given that transform_to_set() has not yet merged, I think we either rip it out now or move on and live with it - a follow up doesn\u0027t make sense to me.\n\nI understand that this feels like churn at the last moment. However, I felt that changing the documented interface to \"list only\", while retaining backwards compatibility for *unknown* out-of-tree callers, was a happy compromise that avoided propagating the more complicated interface. My only reservation was whether I should also add a deprecation warning.","commit_id":"01497a677b4dbccc4f085d27609ec1d0f413444f"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"e98b5c7686523bd3e0150a3f46f377f077cfcf64","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"01b8d64c_475c7d41","updated":"2024-01-06 01:42:55.000000000","message":"we refactored the existing code by adding a helper function ``transform_to_set``, while the interface is still unchanged: \"states can be a list of ints or a single int.\". but this patch changes the interface to be \"\"states is a list of ints\", then we would have to consider upgrade path and backward compatibility. I feel the interface has existed for a long time and it\u0027s acceptable, maybe we should keep it as it is.","commit_id":"01497a677b4dbccc4f085d27609ec1d0f413444f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7f96ff90f8e47dfa83d8ac056f3a9aac4d09e624","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b12d6923_f6aba04c","updated":"2024-01-09 16:40:25.000000000","message":"Alistair wrote it and Clay *and* Jian both like it? I hardly even need to read it ;-)","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"76c0887cc1f32937ade4dcd20ef5063e8fbdb2d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a101e689_090ed899","updated":"2024-01-09 16:31:31.000000000","message":"That\u0027s great this patch was moved to the start of the chain, a much better approach. And Alistair also cleaned up bits to support backward compatibility of out-of-tree usages, since we didn\u0027t find any such use cases. Looks good to me!","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16f6d34271a7cf358618496815c142d8a0f21d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"044e4562_38d32d04","updated":"2024-01-09 15:31:17.000000000","message":"pre-factor FTW!\n\nThe ContainerBroker (perhaps even more-so than DiskFile; and especially since the addition of sharding) is a complex internal interface.  One tool we have to manage the complexity is to remove dynamic interfaces that implicity transform kwarg types where an explicit interface would suffice.\n\nWe looked for out-of-tree code in our cluster management/operations/debugging toolsets that may have been using get_shard_ranges(states\u003d\u003cint\u003e) and found no such examples.  If any such tooling is discovered after this is merged the fix is obvious:\n\n```\nswift/swift/container/backend.py:1737: TypeError: \u0027int\u0027 object is not iterable\n```\n\n... means `states\u003d\u003cint\u003e` should be `states\u003d[\u003cint\u003e]`\n\nI revered the change to sharder and unittests discovered `18 failed, 162 passed` such TypeErrors, covering all the updated call sites.  I think this is a good plan to merge.  I think it simplifies the introduction of get_namespaces.  I\u0027m curious if it covers all of Jianjian\u0027s concerns.","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"}],"swift/container/backend.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"e98b5c7686523bd3e0150a3f46f377f077cfcf64","unresolved":true,"context_lines":[{"line_number":1857,"context_line":"        if isinstance(states, (list, tuple, set)):"},{"line_number":1858,"context_line":"            included_states.update(states)"},{"line_number":1859,"context_line":"        elif states is not None:"},{"line_number":1860,"context_line":"            # tolerate just an int for backwards compatibility"},{"line_number":1861,"context_line":"            included_states.add(states)"},{"line_number":1862,"context_line":""},{"line_number":1863,"context_line":"        # defaults to be used when legacy db\u0027s are missing columns"}],"source_content_type":"text/x-python","patch_set":3,"id":"95ff7ac0_c4d92f78","line":1860,"updated":"2024-01-06 01:42:55.000000000","message":"I like the most of this patch except here, but it\u0027s needed to be backward compatible. And then we have write tests to verify everything will be okay with upgrade. I feel we should keep ``transform_to_set`` as it is, or have this patch as a follow-up and add upgrade test cases.","commit_id":"01497a677b4dbccc4f085d27609ec1d0f413444f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16f6d34271a7cf358618496815c142d8a0f21d4","unresolved":false,"context_lines":[{"line_number":1857,"context_line":"        if isinstance(states, (list, tuple, set)):"},{"line_number":1858,"context_line":"            included_states.update(states)"},{"line_number":1859,"context_line":"        elif states is not None:"},{"line_number":1860,"context_line":"            # tolerate just an int for backwards compatibility"},{"line_number":1861,"context_line":"            included_states.add(states)"},{"line_number":1862,"context_line":""},{"line_number":1863,"context_line":"        # defaults to be used when legacy db\u0027s are missing columns"}],"source_content_type":"text/x-python","patch_set":3,"id":"49550ee2_8e719a63","line":1860,"in_reply_to":"730b6cad_f86c31d2","updated":"2024-01-09 15:31:17.000000000","message":"Acknowledged","commit_id":"01497a677b4dbccc4f085d27609ec1d0f413444f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"114946860ade3428f2737d7978d9a394497e7e7c","unresolved":true,"context_lines":[{"line_number":1857,"context_line":"        if isinstance(states, (list, tuple, set)):"},{"line_number":1858,"context_line":"            included_states.update(states)"},{"line_number":1859,"context_line":"        elif states is not None:"},{"line_number":1860,"context_line":"            # tolerate just an int for backwards compatibility"},{"line_number":1861,"context_line":"            included_states.add(states)"},{"line_number":1862,"context_line":""},{"line_number":1863,"context_line":"        # defaults to be used when legacy db\u0027s are missing columns"}],"source_content_type":"text/x-python","patch_set":3,"id":"730b6cad_f86c31d2","line":1860,"in_reply_to":"95ff7ac0_c4d92f78","updated":"2024-01-08 09:59:27.000000000","message":"the patch is net negative line count, because we had to *add* tests for transform_to_set() which hasn\u0027t yet merged","commit_id":"01497a677b4dbccc4f085d27609ec1d0f413444f"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"76c0887cc1f32937ade4dcd20ef5063e8fbdb2d3","unresolved":false,"context_lines":[{"line_number":1734,"context_line":"        if exclude_others and not include_own:"},{"line_number":1735,"context_line":"            return []"},{"line_number":1736,"context_line":""},{"line_number":1737,"context_line":"        included_states \u003d set(states) if states else None"},{"line_number":1738,"context_line":""},{"line_number":1739,"context_line":"        # defaults to be used when legacy db\u0027s are missing columns"},{"line_number":1740,"context_line":"        default_values \u003d {\u0027reported\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":4,"id":"142ca966_eebd386e","line":1737,"updated":"2024-01-09 16:31:31.000000000","message":"Nice! we got this all cleaned up.","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16f6d34271a7cf358618496815c142d8a0f21d4","unresolved":true,"context_lines":[{"line_number":1864,"context_line":"        :param reverse: reverse the result order."},{"line_number":1865,"context_line":"        :param include_deleted: include items that have the delete marker set."},{"line_number":1866,"context_line":"        :param states: if specified, restricts the returned list to shard"},{"line_number":1867,"context_line":"            ranges that have one of the given states; should be a list of ints."},{"line_number":1868,"context_line":"        :param include_own: boolean that governs whether the row whose name"},{"line_number":1869,"context_line":"            matches the broker\u0027s path is included in the returned list. If"},{"line_number":1870,"context_line":"            True, that row is included unless it is excluded by other"}],"source_content_type":"text/x-python","patch_set":4,"id":"6681e98e_70486ee4","line":1867,"updated":"2024-01-09 15:31:17.000000000","message":"nit: maybe better as\n\n* must be a list of ints or None\n* must be a list of ints if provided","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"}],"swift/container/sharder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16f6d34271a7cf358618496815c142d8a0f21d4","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"                # Shrinking is how we resolve overlaps; we\u0027ve got to"},{"line_number":1309,"context_line":"                # allow multiple shards in that state"},{"line_number":1310,"context_line":"                continue"},{"line_number":1311,"context_line":"            shard_ranges \u003d broker.get_shard_ranges(states\u003d[state])"},{"line_number":1312,"context_line":"            # Transient overlaps can occur during the period immediately after"},{"line_number":1313,"context_line":"            # sharding if a root learns about new child shards before it learns"},{"line_number":1314,"context_line":"            # that the parent has sharded. These overlaps are normally"}],"source_content_type":"text/x-python","patch_set":4,"id":"33aa2f0f_82ca3982","line":1311,"updated":"2024-01-09 15:31:17.000000000","message":"covered by TestSharder.test_audit_*","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16f6d34271a7cf358618496815c142d8a0f21d4","unresolved":true,"context_lines":[{"line_number":1935,"context_line":"        # Create shard containers that are ready to receive redirected object"},{"line_number":1936,"context_line":"        # updates. Do this now, so that redirection can begin immediately"},{"line_number":1937,"context_line":"        # without waiting for cleaving to complete."},{"line_number":1938,"context_line":"        found_ranges \u003d broker.get_shard_ranges(states\u003d[ShardRange.FOUND])"},{"line_number":1939,"context_line":"        created_ranges \u003d []"},{"line_number":1940,"context_line":"        for shard_range in found_ranges:"},{"line_number":1941,"context_line":"            self._increment_stat(\u0027created\u0027, \u0027attempted\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dca70bde_23fdca86","line":1938,"updated":"2024-01-09 15:31:17.000000000","message":"covered by TestSharder.test_cleave_*","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f16f6d34271a7cf358618496815c142d8a0f21d4","unresolved":true,"context_lines":[{"line_number":2233,"context_line":"            else:"},{"line_number":2234,"context_line":"                own_shard_range.update_state(ShardRange.SHARDED)"},{"line_number":2235,"context_line":"                modified_shard_ranges \u003d broker.get_shard_ranges("},{"line_number":2236,"context_line":"                    states\u003d[ShardRange.CLEAVED])"},{"line_number":2237,"context_line":"                for sr in modified_shard_ranges:"},{"line_number":2238,"context_line":"                    sr.update_state(ShardRange.ACTIVE)"},{"line_number":2239,"context_line":"            if (not broker.is_root_container() and not"}],"source_content_type":"text/x-python","patch_set":4,"id":"cde4175d_0fb24f44","line":2236,"updated":"2024-01-09 15:31:17.000000000","message":"covered by TestSharder.test_complete_sharding_*","commit_id":"f3a32367bf2048bb61f61f3468e00cf27064e54d"}]}