)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ad410b06fe52e7df0db576f42b45bc1183e711ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4149c2cd_efe7e53c","updated":"2024-06-24 15:10:59.000000000","message":"At first glance I\u0027m not sure I see the benifit; I think I picked up that the style proposed in the parent seems a bit repetative for the helpers to redeclare stuff like `expirer_divisor\u003dEXPIRER_DIVISOR` and using the `expirer_config` argument hides all that?  Is that only a good thing?\n\nMaybe using function signatures to define the required helper arguments is an new/unusal pattern; I found it early in my days with python and have used it in other code - but maybe not in swift?  OTOH, outside of the `get_config` pattern (which I separate as for PRE-parsed dicts of k\u003d\u003ev *strings*) - I\u0027m also not sure *this* pattern is any more represented as prior art in the swift code base?  Metadata?  Is this *just* a style preference or can we extract some objective benifits?  I\u0027m happy to go this route if there\u0027s a concensus!\n\nI got a little chuckle cause IME Al is normally the one reminding us that our IDEs won\u0027t lint against typos in dict key strings.","commit_id":"bd94e09537f79555280de827cde97ea14aebca49"}],"swift/obj/expirer.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ad410b06fe52e7df0db576f42b45bc1183e711ff","unresolved":true,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"def get_expirer_container(x_delete_at, acc, cont, obj, expirer_config,"},{"line_number":96,"context_line":"                          **kwargs):"},{"line_number":97,"context_line":"    \"\"\""},{"line_number":98,"context_line":"    Returns an expiring object task container name for given X-Delete-At and"},{"line_number":99,"context_line":"    (native string) a/c/o."}],"source_content_type":"text/x-python","patch_set":2,"id":"ed96e156_4a40e236","line":96,"updated":"2024-06-24 15:10:59.000000000","message":"so **kwargs probably isn\u0027t needed anymore; if we add more keys to expirer_config in future versions the signature doesn\u0027t need to adapt.","commit_id":"bd94e09537f79555280de827cde97ea14aebca49"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ad410b06fe52e7df0db576f42b45bc1183e711ff","unresolved":true,"context_lines":[{"line_number":100,"context_line":"    \"\"\""},{"line_number":101,"context_line":"    # the offset backwards from the expected day is a hash of size \"per day\""},{"line_number":102,"context_line":"    shard_int \u003d (int(utils.hash_path(acc, cont, obj), 16) %"},{"line_number":103,"context_line":"                 expirer_config[\u0027task_container_per_day\u0027])"},{"line_number":104,"context_line":"    # even though the kwarg is named \"task_container_per_day\" it\u0027s actually"},{"line_number":105,"context_line":"    # \"per_divisor\" if for some reason the deprecated config \"expirer_divisor\""},{"line_number":106,"context_line":"    # option doesn\u0027t have the default value of 86400"}],"source_content_type":"text/x-python","patch_set":2,"id":"8a86e3cf_b4fe6e7f","line":103,"updated":"2024-06-24 15:10:59.000000000","message":"I like the way python lets you pull named args out of a kwarg dict into the signature and don\u0027t love passing around an opaque dict that doesn\u0027t express the required keys in the function signature\n\nIf we go this route I\u0027d almost prefer a proper dataclass/named-tuple and use `expirer_config.task_container_per_day`","commit_id":"bd94e09537f79555280de827cde97ea14aebca49"}]}