)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"75674955cf052fd6240f36419571eab01a17695b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a6249910_ce5b48f3","updated":"2024-05-03 19:53:01.000000000","message":"Oh yeah, the tool I wrote: https://paste.opendev.org/show/bQTS00W1tyztvv5L3tja/","commit_id":"b2cd6d3419cae4cc732526e3868694ee880129b6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"3a3e11b4f5a26491848ac20c976fc7e4595f6872","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7cf8ced4_a97ffab0","updated":"2024-05-03 19:50:47.000000000","message":"You expressed an interest in something like this following the utils refactor -- it actually goes way way beyond that, though.\n\nWDWT?","commit_id":"b2cd6d3419cae4cc732526e3868694ee880129b6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef2f658e70bcb0aaacc50aec08f4d93b2b7aa459","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dd545fec_2365a85c","updated":"2024-05-06 16:59:18.000000000","message":"I think most of these look fine.  All the swift code is doing mostly reasonable imports from public interfaces.  However, since we\u0027re probably not going to get rid of some of these backwards-compat import paths, e.g. common.utils.Timestamp - I question the value.\n\nMy main goal with \"updating swift code to do imports right\" would be to get rid of the names in swift.common.utils that we don\u0027t expect out-of-tree tools to use.  In that regard I think it might be more valuable to do this work in tests rather than swift core code itself.\n\nIn particular I don\u0027t really want to maintain utils.logs.get_logger returning a hybrid-ized logger+statsd_client interface object.\n\n915483: stats: Refactor StatsdClient config | https://review.opendev.org/c/openstack/swift/+/915483\n\nEventually I think utils.logs shouldn\u0027t even import statsd_client","commit_id":"f86b3e2f761f4068ec45c7dc0023024d78d3064c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"19fc498c5d9fb62188f20006b787eed188d9611b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f4527a8c_bded9711","updated":"2024-05-07 06:24:25.000000000","message":"Yeah I like that we\u0027re calling the new locations rather then where they used to be.\n\nIf we want to remove the noqa\u0027s then we need to decide we\u0027re happy with potentually breaking 3rd parties and say hey utils where never an API. Maybe we should try and do a demo release with this downstream and see what our 3rd party code does.\n\nIf were going to make this much churn I wonder if we should turn all our multi-line import statements into the same form and be consistent:\n\n    from x import (\n      blah,\n      blah2\n    )\n\nThen maybe write this to our code style guide (or is there a flake8 thing we can turn on to check for this).","commit_id":"f86b3e2f761f4068ec45c7dc0023024d78d3064c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4095fd37a4dfc6f513670d342ef09e08064a02a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"88f88cfb_254bfa32","in_reply_to":"f4527a8c_bded9711","updated":"2024-05-07 15:57:00.000000000","message":"Yeah, there\u0027s definitely a style question here -- that\u0027s part of the buy-in I was hoping to get. I suspect that longer term, the multiline import style I settled on here will avoid merge conflicts (or at least make them easier to resolve) in the future.\n\nOTOH, you look at the diskfile diff and realize we probably should have done a `from swift.common import utils` and use `utils.\u003cx\u003e` everywhere instead. (And again, my tooling wouldn\u0027t catch that! For any specific function, it\u0027s not too bad -- grep for `utils.md5` and rework all instances to be `base.md5`, for example -- but the general case is rather difficult!)\n\nAs far as style checks, we could write our own in-tree flake8 plug-in if we care enough.\n\nFWIW, hacking [has some import guidance](https://docs.openstack.org/hacking/latest/user/hacking.html#imports) but we\u0027ve turned off both `H301` (\"Do not import more than one module per line\") and `H306` (\"Alphabetically order your imports ...\"). For better or worse, we\u0027ve largely ignored the (perhaps unenforceable?) \"Do not import objects, only modules\" guidance.","commit_id":"f86b3e2f761f4068ec45c7dc0023024d78d3064c"}],"swift/account/reaper.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"958f427b86955fcb91fb05fd9888d3ba7a3471f4","unresolved":true,"context_lines":[{"line_number":31,"context_line":"    direct_delete_object, direct_get_container"},{"line_number":32,"context_line":"from swift.common.exceptions import ClientException"},{"line_number":33,"context_line":"from swift.common.request_helpers import USE_REPLICATION_NETWORK_HEADER"},{"line_number":34,"context_line":"from swift.common.ring.ring import Ring"},{"line_number":35,"context_line":"from swift.common.ring.utils import is_local_device"},{"line_number":36,"context_line":"from swift.common.utils import node_to_string"},{"line_number":37,"context_line":"from swift.common.utils.base import md5"}],"source_content_type":"text/x-python","patch_set":2,"id":"9b67d25d_6b4b3441","line":34,"updated":"2024-05-06 18:15:22.000000000","message":"This one\u0027s a little awkward, and definitely commonplace.","commit_id":"f86b3e2f761f4068ec45c7dc0023024d78d3064c"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef2f658e70bcb0aaacc50aec08f4d93b2b7aa459","unresolved":true,"context_lines":[{"line_number":85,"context_line":"from .base import (  # noqa"},{"line_number":86,"context_line":"    md5, get_valid_utf8_str, quote, split_path)"},{"line_number":87,"context_line":"from swift.common.utils.logs import (   # noqa"},{"line_number":88,"context_line":"    SysLogHandler,  # t.u.helpers.setup_servers monkey patch is sketch"},{"line_number":89,"context_line":"    logging_monkey_patch,"},{"line_number":90,"context_line":"    get_logger,"},{"line_number":91,"context_line":"    PrefixLoggerAdapter,"}],"source_content_type":"text/x-python","patch_set":2,"id":"166cde5f_0e355521","side":"PARENT","line":88,"updated":"2024-05-06 16:59:18.000000000","message":"is this the ONLY noqa import we get to remove!?  Not very exciting.","commit_id":"761d9196772bc150fd4923fb04367b919a521d59"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"958f427b86955fcb91fb05fd9888d3ba7a3471f4","unresolved":true,"context_lines":[{"line_number":85,"context_line":"from .base import (  # noqa"},{"line_number":86,"context_line":"    md5, get_valid_utf8_str, quote, split_path)"},{"line_number":87,"context_line":"from swift.common.utils.logs import (   # noqa"},{"line_number":88,"context_line":"    SysLogHandler,  # t.u.helpers.setup_servers monkey patch is sketch"},{"line_number":89,"context_line":"    logging_monkey_patch,"},{"line_number":90,"context_line":"    get_logger,"},{"line_number":91,"context_line":"    PrefixLoggerAdapter,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c023820_7d204232","side":"PARENT","line":88,"in_reply_to":"166cde5f_0e355521","updated":"2024-05-06 18:15:22.000000000","message":"No idea! We *definitely* shouldn\u0027t have it, though -- `setup_servers` doesn\u0027t actually need it.\n\nCleaning up the `# noqa` imports wasn\u0027t actually the goal of the change, though it could be a goal of a larger chain. FWIW, I think these sorts of imports fall into a few categories:\n\n- Actually required imports, no `# noqa` necessary. We *can\u0027t* get rid of `md5` today, because we actually use it here. _Maybe_ we could get rid of it in the future by importing `base` then using `base.md5`?\n- Parts of our public API that we actually kinda like. For instance, `from swift.common.ring import Ring` instead of `from swift.common.ring.ring import Ring`\n- Parts of our public API that that weren\u0027t really intended to become public API, but Hyrum\u0027s law, and we already know of external projects using them. For instance, `config_*_value` functions.\n- Things that _tests_ expect to find here, and we realized that muddying the refactor with a bunch of extra test churn wasn\u0027t going to get it landed any faster.\n- Things that we include *just in case* someone took them as part of our public API, because again, Hyrum\u0027s law. `LOG_LINE_DEFAULT_FORMAT`, `NOTICE`, _LibcWrapper`...\n\n(The really annoying thing about that last bucket is that once you\u0027ve introduced the breakage, you can\u0027t really un-ring that bell -- the third-party will likely just add more crap to work around our breakage, similar to what we\u0027ve had to do [time](https://github.com/openstack/swift/blob/2.33.0/swift/common/manager.py#L27-L31) and [time](https://github.com/openstack/swift/blob/2.33.0/swift/common/middleware/s3api/s3response.py#L17-L20) [again](https://github.com/openstack/swift/blob/2.33.0/swift/common/middleware/s3api/etree.py#L18-L28).)\n\nI think there are a few concurrent goals for *this* change:\n\n1. Make it obvious in swift where some referenced code (also in swift) lives. If I\u0027m down in diskfile and need a refresher on what `encode_timestamps` actually _does_ and what the return looks like, it\u0027s nice to be able to go to the top of the file and get an accurate location for its definition.\n2. Reduce the complexity of imports. Having `A` import from `C` *by way of* `B` seems likely to complicate refactoring of either `C` or `B` (or both), even if `A` needs to import other things from `B`. (I don\u0027t think it\u0027s *directly* related, but this *feels* somewhat related to why we got a `common.base` and `common.config` out of that refactor...)\n3. Offer some guidance (by way of example) to third parties for how they should structure imports going forward. In this regard, some things maybe _should_ get rolled back, and the list of exceptions in my quickly-hacked-together tooling should get updated.","commit_id":"761d9196772bc150fd4923fb04367b919a521d59"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ef2f658e70bcb0aaacc50aec08f4d93b2b7aa459","unresolved":true,"context_lines":[{"line_number":82,"context_line":"# For backwards compatability with 3rd party middlewares"},{"line_number":83,"context_line":"from swift.common.registry import register_swift_info, get_swift_info  # noqa"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"from swift.common.utils.base import (  # noqa"},{"line_number":86,"context_line":"    md5,"},{"line_number":87,"context_line":"    get_valid_utf8_str,"},{"line_number":88,"context_line":"    quote,"}],"source_content_type":"text/x-python","patch_set":2,"id":"f92f450e_8674b7c9","line":85,"updated":"2024-05-06 16:59:18.000000000","message":"is this *prefered* to the relative import, or just churn from tooling?","commit_id":"f86b3e2f761f4068ec45c7dc0023024d78d3064c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4095fd37a4dfc6f513670d342ef09e08064a02a4","unresolved":true,"context_lines":[{"line_number":82,"context_line":"# For backwards compatability with 3rd party middlewares"},{"line_number":83,"context_line":"from swift.common.registry import register_swift_info, get_swift_info  # noqa"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"from swift.common.utils.base import (  # noqa"},{"line_number":86,"context_line":"    md5,"},{"line_number":87,"context_line":"    get_valid_utf8_str,"},{"line_number":88,"context_line":"    quote,"}],"source_content_type":"text/x-python","patch_set":2,"id":"144c0622_d7a4aa7c","line":85,"in_reply_to":"e4bc578f_4cc7a498","updated":"2024-05-07 15:57:00.000000000","message":"Oh hey, `hacking` actually has a check for this! *And* we didn\u0027t nope out of it! `H304`\n\nI\u0027m having trouble tripping it, though, even when I remove the `# noqa` -- weird...","commit_id":"f86b3e2f761f4068ec45c7dc0023024d78d3064c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"958f427b86955fcb91fb05fd9888d3ba7a3471f4","unresolved":true,"context_lines":[{"line_number":82,"context_line":"# For backwards compatability with 3rd party middlewares"},{"line_number":83,"context_line":"from swift.common.registry import register_swift_info, get_swift_info  # noqa"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"from swift.common.utils.base import (  # noqa"},{"line_number":86,"context_line":"    md5,"},{"line_number":87,"context_line":"    get_valid_utf8_str,"},{"line_number":88,"context_line":"    quote,"}],"source_content_type":"text/x-python","patch_set":2,"id":"e4bc578f_4cc7a498","line":85,"in_reply_to":"f92f450e_8674b7c9","updated":"2024-05-06 18:15:22.000000000","message":"*I* prefer it, anyway, and it\u0027s more consistent with the rest of the codebase.","commit_id":"f86b3e2f761f4068ec45c7dc0023024d78d3064c"}]}