)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":32,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":33,"context_line":"Metrics dashboard will need updates to display those changed metrics"},{"line_number":34,"context_line":"correctly, also some infocache metrics are newly added, please see"},{"line_number":35,"context_line":"above message for all changes needed."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Change-Id: I60a9f1c349b4bc78ecb850fb26ae56eb20fa39c6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"e4c42438_6d686b6d","line":35,"updated":"2023-06-22 00:54:10.000000000","message":"this looks good!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a18e3ddd_1812a0fe","updated":"2023-06-13 18:22:47.000000000","message":"I like the direction -- I think centralizing the metrics emission makes it much more likely that we get them consistently. The removal of `except AttributeError: skip_chance \u003d 0` has me worried, though.","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4f25cccd0580310446d09f56a1a30631c159eced","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c2ab4b85_35557f9a","updated":"2023-06-12 14:50:10.000000000","message":"I think the _final_pipeline_app interface could help with the test failures:\n\n\t\telse:\n\t\u003e           skip_chance \u003d app.account_existence_skip_cache\n\tE           AttributeError: \u0027FakeApp\u0027 object has no attribute \u0027account_existence_skip_cache\u0027","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"16643808c45349b9b74e6183a2588a5a7b4ad7c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ad593f9c_644b18b6","updated":"2023-06-21 14:21:23.000000000","message":"thanks for the reviews!","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9863f0efe33f903ed375bbde6a597c685261a43e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d3341583_8f682a0c","in_reply_to":"c2ab4b85_35557f9a","updated":"2023-06-13 18:27:32.000000000","message":"FWIW, I think it\u0027s reasonable for us to add something like\n```\nself._pipeline_final_app \u003d self\n```\nto `FakeApp`, but my test follow-up also has a failure like\n```\n        if container:\n\u003e           skip_chance \u003d app.container_existence_skip_cache\nE           AttributeError: \u0027function\u0027 object has no attribute \u0027container_existence_skip_cache\u0027\n```\nwhere the whole point of the test change is to _not_ have that attribute on one of the middle filters.","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a64b66d6_eade5ef3","updated":"2023-06-22 00:54:10.000000000","message":"Jian, this is fantastic work - I only looked at it breifly and I feel like I owe you a more serious review; it\u0027s possible this should merge as-is.\n\nI noticed a couple of nits and cans of worms - I wonder if you\u0027d care to think about and respond to any of them.  Changing the call to use the public interface in proxy.controllers.container might be obviously impossible if I\u0027d just checkout the change and review it properly.  What do you think about pascal-style function ordering as tool to help in reviews?  Are you more in te camp of lint-or-it-doesn\u0027t-matter or style/code-culture discussion in review is helpful and relevant?  I go back and forth depending on what irks me personally 😄","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8bad23b6_e1a4f5e2","updated":"2023-06-23 05:13:56.000000000","message":"thanks for the detailed reviews!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"6187e418_6a3dd6f3","updated":"2023-06-27 02:05:47.000000000","message":"I think this effort is adding a TON of consistency into these get_info methods; but we might have to break it down into multiple steps to fix all the things.","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"016089ea1210f6a83d0467e433008a583a73dbf5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d774c669_e34e8f78","updated":"2023-07-05 18:44:24.000000000","message":"thanks again for the reviews!","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d89c52ea_4991f227","updated":"2023-07-05 23:25:00.000000000","message":"Yeah, I\u0027m digging it.","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2d13c3dd1df9e57337faa0efeb4bbc34a522c774","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e5e433c5_406c71cf","updated":"2023-07-26 01:43:05.000000000","message":"This looks great. Been running in prod too, and we like the metrics, we should all have them!\n\nTim makes some good suggestions, but maybe they can be done as a follow up. Not blocking or want to change this anymore.","commit_id":"6ccd8950ba17513a86321620fdbc43c5209a7689"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8f6ed5427974f486a1abe72e73bd5974290bbdf4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9ea6f6d6_1e285a7f","updated":"2023-08-03 00:12:33.000000000","message":"Huh... I wonder why this never merged after Matt +A\u0027d","commit_id":"bc300a516b2834d8c4b25e506d60e43589d2d2e9"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5c89b56597dfa99824afaa67868718fa0be280dc","unresolved":true,"context_lines":[{"line_number":791,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":792,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"},{"line_number":793,"context_line":"            # filters, we\u0027ll just have to choose a reasonable default"},{"line_number":794,"context_line":"            skip_chance \u003d 0.0"},{"line_number":795,"context_line":"            logger \u003d None"},{"line_number":796,"context_line":"        else:"},{"line_number":797,"context_line":"            if container:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8082f433_520b6583","side":"PARENT","line":794,"updated":"2023-06-09 19:37:01.000000000","message":"this case \"skip_chance \u003d 0.0\" is removed in the new patch. I doubt we needed it in the first place.","commit_id":"eb3fe260d51d021674b2bccc4b9d1b4cc0757450"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":true,"context_lines":[{"line_number":791,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":792,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"},{"line_number":793,"context_line":"            # filters, we\u0027ll just have to choose a reasonable default"},{"line_number":794,"context_line":"            skip_chance \u003d 0.0"},{"line_number":795,"context_line":"            logger \u003d None"},{"line_number":796,"context_line":"        else:"},{"line_number":797,"context_line":"            if container:"}],"source_content_type":"text/x-python","patch_set":1,"id":"de061090_bbbbf524","side":"PARENT","line":794,"in_reply_to":"8082f433_520b6583","updated":"2023-06-13 18:22:47.000000000","message":"Thanks for calling this out! It makes me a little nervous, though -- I think https://review.opendev.org/c/openstack/swift/+/886010 should demonstrate. Basically, if\n```\n    # Try to cut through all the layers to the proxy app\n    try:\n        app \u003d app._pipeline_final_app\n    except AttributeError:\n        pass\n```\n*does* trip the `AttributeError` and `app` is still some nebulous spot in the pipeline instead of the proxy app, then `account_existence_skip_cache` / `container_existence_skip_cache` won\u0027t be available so we\u0027d get `AttributeError`s in here.\n\nWe\u0027ve definitely got a third party auth middleware that\u0027s built that way, though I *think* it only calls `get_account_info` from the inner-most filter whose `app` reference *should* have a `_pipeline_final_app` -- but that seems just seems to be a bit of good luck.","commit_id":"eb3fe260d51d021674b2bccc4b9d1b4cc0757450"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"16643808c45349b9b74e6183a2588a5a7b4ad7c0","unresolved":false,"context_lines":[{"line_number":791,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":792,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"},{"line_number":793,"context_line":"            # filters, we\u0027ll just have to choose a reasonable default"},{"line_number":794,"context_line":"            skip_chance \u003d 0.0"},{"line_number":795,"context_line":"            logger \u003d None"},{"line_number":796,"context_line":"        else:"},{"line_number":797,"context_line":"            if container:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cb1838e6_50ec96d4","side":"PARENT","line":794,"in_reply_to":"de061090_bbbbf524","updated":"2023-06-21 14:21:23.000000000","message":"thanks for double checking it, yes, this change causes a lot of middleware tests to fail, e.g. middleware.test_quotas.","commit_id":"eb3fe260d51d021674b2bccc4b9d1b4cc0757450"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":true,"context_lines":[{"line_number":446,"context_line":"    info, cache_state \u003d _get_info_from_caches(app, env, account, container)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    if info:"},{"line_number":449,"context_line":"        record_ac_info_cache_metrics(app, cache_state, account, container)"},{"line_number":450,"context_line":"    else:"},{"line_number":451,"context_line":"        # Cache miss; go HEAD the container and populate the caches"},{"line_number":452,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"ce5455c3_65c4679a","line":449,"updated":"2023-06-13 18:22:47.000000000","message":"Alternatively, we set `resp \u003d None` at the start, and have just the one `record_ac_info_cache_metrics` call just before the `return` at the end. \\*shrug\\*","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"16643808c45349b9b74e6183a2588a5a7b4ad7c0","unresolved":false,"context_lines":[{"line_number":446,"context_line":"    info, cache_state \u003d _get_info_from_caches(app, env, account, container)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    if info:"},{"line_number":449,"context_line":"        record_ac_info_cache_metrics(app, cache_state, account, container)"},{"line_number":450,"context_line":"    else:"},{"line_number":451,"context_line":"        # Cache miss; go HEAD the container and populate the caches"},{"line_number":452,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"23dca74c_33ff4979","line":449,"in_reply_to":"ce5455c3_65c4679a","updated":"2023-06-21 14:21:23.000000000","message":"Done","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":true,"context_lines":[{"line_number":462,"context_line":"        if not is_autocreate_account:"},{"line_number":463,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"},{"line_number":464,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"},{"line_number":465,"context_line":"                return headers_to_container_info({}, 0)"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"        req \u003d _prepare_pre_auth_info_request("},{"line_number":468,"context_line":"            env, (\"/%s/%s/%s\" % (version, wsgi_account, wsgi_container)),"}],"source_content_type":"text/x-python","patch_set":1,"id":"9250db2e_b559ff79","line":465,"updated":"2023-06-13 18:22:47.000000000","message":"Oh, hey -- we don\u0027t record any metrics on this return...","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"16643808c45349b9b74e6183a2588a5a7b4ad7c0","unresolved":false,"context_lines":[{"line_number":462,"context_line":"        if not is_autocreate_account:"},{"line_number":463,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"},{"line_number":464,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"},{"line_number":465,"context_line":"                return headers_to_container_info({}, 0)"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"        req \u003d _prepare_pre_auth_info_request("},{"line_number":468,"context_line":"            env, (\"/%s/%s/%s\" % (version, wsgi_account, wsgi_container)),"}],"source_content_type":"text/x-python","patch_set":1,"id":"aaff3bad_9a080597","line":465,"in_reply_to":"9250db2e_b559ff79","updated":"2023-06-21 14:21:23.000000000","message":"Done","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":true,"context_lines":[{"line_number":772,"context_line":"        # the cases of cache_state is memcache miss, error, skip, force_skip"},{"line_number":773,"context_line":"        # or disabled."},{"line_number":774,"context_line":"        if resp is not None:"},{"line_number":775,"context_line":"            # Note: currently there is no case that \u0027resp\u0027 will be None."},{"line_number":776,"context_line":"            logger.increment("},{"line_number":777,"context_line":"                \u0027%s.cache.%s.%d\u0027 % (op_type, cache_state, resp.status_int))"},{"line_number":778,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"edde3b2f_2e1d8b39","line":775,"updated":"2023-06-13 18:22:47.000000000","message":"Does this comment need updating? It kinda seems like it does...","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"16643808c45349b9b74e6183a2588a5a7b4ad7c0","unresolved":false,"context_lines":[{"line_number":772,"context_line":"        # the cases of cache_state is memcache miss, error, skip, force_skip"},{"line_number":773,"context_line":"        # or disabled."},{"line_number":774,"context_line":"        if resp is not None:"},{"line_number":775,"context_line":"            # Note: currently there is no case that \u0027resp\u0027 will be None."},{"line_number":776,"context_line":"            logger.increment("},{"line_number":777,"context_line":"                \u0027%s.cache.%s.%d\u0027 % (op_type, cache_state, resp.status_int))"},{"line_number":778,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"34e2846a_4f0d2c75","line":775,"in_reply_to":"edde3b2f_2e1d8b39","updated":"2023-06-21 14:21:23.000000000","message":"Done","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":true,"context_lines":[{"line_number":801,"context_line":"        # Only the middleware entry-points get a reference to the"},{"line_number":802,"context_line":"        # proxy-server app; if a middleware composes itself as multiple"},{"line_number":803,"context_line":"        # filters, we\u0027ll just have to choose a reasonable default"},{"line_number":804,"context_line":"        logger \u003d None"},{"line_number":805,"context_line":"    else:"},{"line_number":806,"context_line":"        logger \u003d proxy_app.logger"},{"line_number":807,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"6ade90c4_60c08223","line":804,"updated":"2023-06-13 18:22:47.000000000","message":"Alternatively, we could just return early here.","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"16643808c45349b9b74e6183a2588a5a7b4ad7c0","unresolved":false,"context_lines":[{"line_number":801,"context_line":"        # Only the middleware entry-points get a reference to the"},{"line_number":802,"context_line":"        # proxy-server app; if a middleware composes itself as multiple"},{"line_number":803,"context_line":"        # filters, we\u0027ll just have to choose a reasonable default"},{"line_number":804,"context_line":"        logger \u003d None"},{"line_number":805,"context_line":"    else:"},{"line_number":806,"context_line":"        logger \u003d proxy_app.logger"},{"line_number":807,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"4b8431e7_fdf6d12e","line":804,"in_reply_to":"6ade90c4_60c08223","updated":"2023-06-21 14:21:23.000000000","message":"resolved...doesn\u0027t apply to new patch anymore.","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":true,"context_lines":[{"line_number":824,"context_line":"    \"\"\""},{"line_number":825,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":826,"context_line":"    if not memcache:"},{"line_number":827,"context_line":"        return None, \u0027disabled\u0027"},{"line_number":828,"context_line":""},{"line_number":829,"context_line":"    cache_key \u003d get_cache_key(account, container)"},{"line_number":830,"context_line":"    if container:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ce395d6a_49636644","line":827,"updated":"2023-06-13 18:22:47.000000000","message":"+1 on the guard return, saves a bunch of indent and makes it obvious when and why we were returning `None` before.","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"16643808c45349b9b74e6183a2588a5a7b4ad7c0","unresolved":false,"context_lines":[{"line_number":824,"context_line":"    \"\"\""},{"line_number":825,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":826,"context_line":"    if not memcache:"},{"line_number":827,"context_line":"        return None, \u0027disabled\u0027"},{"line_number":828,"context_line":""},{"line_number":829,"context_line":"    cache_key \u003d get_cache_key(account, container)"},{"line_number":830,"context_line":"    if container:"}],"source_content_type":"text/x-python","patch_set":1,"id":"326266b1_1d0f40ca","line":827,"in_reply_to":"ce395d6a_49636644","updated":"2023-06-21 14:21:23.000000000","message":"Ack","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":464,"context_line":"        if not is_autocreate_account:"},{"line_number":465,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"},{"line_number":466,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"},{"line_number":467,"context_line":"                record_ac_info_cache_metrics(logger, cache_state, container)"},{"line_number":468,"context_line":"                return headers_to_container_info({}, 0)"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"        req \u003d _prepare_pre_auth_info_request("}],"source_content_type":"text/x-python","patch_set":3,"id":"c77e2659_43c92b53","line":467,"updated":"2023-06-22 00:54:10.000000000","message":"my first assumption when reading the diff top to bottom on this file was that this was an existing symbol - because, otherwise, I would have seen `def some_new_function_you_need_to_understand` already!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"51eaca9c63d182e1d1bd9b7b7c2efd589e8eb4ee","unresolved":false,"context_lines":[{"line_number":464,"context_line":"        if not is_autocreate_account:"},{"line_number":465,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"},{"line_number":466,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"},{"line_number":467,"context_line":"                record_ac_info_cache_metrics(logger, cache_state, container)"},{"line_number":468,"context_line":"                return headers_to_container_info({}, 0)"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"        req \u003d _prepare_pre_auth_info_request("}],"source_content_type":"text/x-python","patch_set":3,"id":"919d1159_a3ab48f1","line":467,"in_reply_to":"afdf61b0_697910ed","updated":"2023-06-23 10:31:13.000000000","message":"+1 the code is inconsistent but I\u0027m all for define before you use it","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":false,"context_lines":[{"line_number":464,"context_line":"        if not is_autocreate_account:"},{"line_number":465,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"},{"line_number":466,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"},{"line_number":467,"context_line":"                record_ac_info_cache_metrics(logger, cache_state, container)"},{"line_number":468,"context_line":"                return headers_to_container_info({}, 0)"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"        req \u003d _prepare_pre_auth_info_request("}],"source_content_type":"text/x-python","patch_set":3,"id":"afdf61b0_697910ed","line":467,"in_reply_to":"c77e2659_43c92b53","updated":"2023-06-23 05:13:56.000000000","message":"Thanks for bringing up this topic. I am totally with you on this coding style: for non-class-member functions, pascal style is the way to go! In this file, the main API functions (like get_object_info/get_container_info/get_account_info) were originally defined in the front, but all the internal functions are defined behind them, that\u0027s why in the first patch I put record_ac_info_cache_metrics() behind the call sites along with other internal functions. I will move all three main functions to the back seats.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":542,"context_line":"    except AttributeError:"},{"line_number":543,"context_line":"        logger \u003d None"},{"line_number":544,"context_line":"    else:"},{"line_number":545,"context_line":"        logger \u003d app.logger"},{"line_number":546,"context_line":"    # Check in environment cache and in memcache (in that order)"},{"line_number":547,"context_line":"    info, cache_state \u003d _get_info_from_caches(app, env, account)"},{"line_number":548,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ef10065a_17ef5e15","line":545,"updated":"2023-06-22 00:54:10.000000000","message":"gah, well now i want\n\n    app, logger \u003d get_final_app_and_failsafe_logger(app)\n\nsomewhere over closer to where we add in all this _pipeline_final_app stuff as a public inteface that does something really clever so we don\u0027t have to plumb through an Optional[logger] type.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"599fe634d269804c79a48ee5e473e6b6cd936d03","unresolved":false,"context_lines":[{"line_number":542,"context_line":"    except AttributeError:"},{"line_number":543,"context_line":"        logger \u003d None"},{"line_number":544,"context_line":"    else:"},{"line_number":545,"context_line":"        logger \u003d app.logger"},{"line_number":546,"context_line":"    # Check in environment cache and in memcache (in that order)"},{"line_number":547,"context_line":"    info, cache_state \u003d _get_info_from_caches(app, env, account)"},{"line_number":548,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"69cc67ba_961932bd","line":545,"in_reply_to":"cf63b23e_49ca49ea","updated":"2023-06-23 21:34:26.000000000","message":"I reverted this back, since three of those functions which needed this all look different after I rebase this patch on top of this: https://review.opendev.org/c/openstack/swift/+/884931","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":false,"context_lines":[{"line_number":542,"context_line":"    except AttributeError:"},{"line_number":543,"context_line":"        logger \u003d None"},{"line_number":544,"context_line":"    else:"},{"line_number":545,"context_line":"        logger \u003d app.logger"},{"line_number":546,"context_line":"    # Check in environment cache and in memcache (in that order)"},{"line_number":547,"context_line":"    info, cache_state \u003d _get_info_from_caches(app, env, account)"},{"line_number":548,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"cf63b23e_49ca49ea","line":545,"in_reply_to":"ef10065a_17ef5e15","updated":"2023-06-23 05:13:56.000000000","message":"Done","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":549,"context_line":"    # Cache miss; go HEAD the account and populate the caches"},{"line_number":550,"context_line":"    if info:"},{"line_number":551,"context_line":"        resp \u003d None"},{"line_number":552,"context_line":"    else:"},{"line_number":553,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})"},{"line_number":554,"context_line":"        req \u003d _prepare_pre_auth_info_request("},{"line_number":555,"context_line":"            env, \"/%s/%s\" % (version, wsgi_account),"}],"source_content_type":"text/x-python","patch_set":3,"id":"8fd4110b_9385fd0b","line":552,"updated":"2023-06-22 00:54:10.000000000","message":"ah you flipped the \"did the cache layer look up here work?\" here and it reads quite nice.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":589,"context_line":"            info[field] \u003d int(info[field])"},{"line_number":590,"context_line":""},{"line_number":591,"context_line":"    record_ac_info_cache_metrics("},{"line_number":592,"context_line":"        logger, cache_state, container\u003dNone, resp\u003dresp)"},{"line_number":593,"context_line":"    return info"},{"line_number":594,"context_line":""},{"line_number":595,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3c47a272_c1915f48","line":592,"updated":"2023-06-22 00:54:10.000000000","message":"signature is a little tricky here - I appreciate the explicit use of kwargs for clarity - kudos!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":801,"context_line":"    # function, since loggers won\u0027t have prefixes under some usages."},{"line_number":802,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":803,"context_line":"    if logger:"},{"line_number":804,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"},{"line_number":805,"context_line":""},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"def _get_info_from_memcache(app, env, account, container\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"94112d39_6d20ea2b","line":804,"updated":"2023-06-22 00:54:10.000000000","message":"I wish this was defined above call sites, pascal style!  When I first saw the new symbol looking at the diff I assumed it was an import of an existing interface! \n Maybe opening the code in a decent ide will break me of this antiquated/uncessary preference.  Or, perhaps other old timers may appreciate my astetic and the cognitive conviencne when simply reading diffs, but alas we are not the interpreter and AFAIK defining this here works fine.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":false,"context_lines":[{"line_number":801,"context_line":"    # function, since loggers won\u0027t have prefixes under some usages."},{"line_number":802,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":803,"context_line":"    if logger:"},{"line_number":804,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"},{"line_number":805,"context_line":""},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"def _get_info_from_memcache(app, env, account, container\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3db2585c_e456983b","line":804,"in_reply_to":"94112d39_6d20ea2b","updated":"2023-06-23 05:13:56.000000000","message":"Done","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":815,"context_line":""},{"line_number":816,"context_line":"    :returns: a tuple of two values, the first is a dictionary of cached info"},{"line_number":817,"context_line":"      on cache hit, None on miss or if memcache is not in use; the second is"},{"line_number":818,"context_line":"      cache state."},{"line_number":819,"context_line":"    \"\"\""},{"line_number":820,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":821,"context_line":"    if not memcache:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fee23b2b_df2bf7fa","line":818,"updated":"2023-06-22 00:54:10.000000000","message":"kudos on the doc-string update!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":864,"context_line":"        info \u003d new_info"},{"line_number":865,"context_line":"    if info:"},{"line_number":866,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})[cache_key] \u003d info"},{"line_number":867,"context_line":"    return info, cache_state"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":""},{"line_number":870,"context_line":"def _get_info_from_caches(app, env, account, container\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"f92528e2_075aa7ee","line":867,"updated":"2023-06-22 00:54:10.000000000","message":"at first i thought i wasn\u0027t going to like the return signature change on an existing interface, but I was wrong, this is a great function and the refactor is nice.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":882,"context_line":"        info, cache_state \u003d _get_info_from_memcache("},{"line_number":883,"context_line":"            app, env, account, container)"},{"line_number":884,"context_line":"    else:"},{"line_number":885,"context_line":"        cache_state \u003d \u0027infocache_hit\u0027"},{"line_number":886,"context_line":"    return info, cache_state"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"735800fc_77b63f45","line":885,"updated":"2023-06-22 00:54:10.000000000","message":"this threw me, I had to think a minute to understand I would prefer `if info` and the _get_from_memcache in the else","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":false,"context_lines":[{"line_number":882,"context_line":"        info, cache_state \u003d _get_info_from_memcache("},{"line_number":883,"context_line":"            app, env, account, container)"},{"line_number":884,"context_line":"    else:"},{"line_number":885,"context_line":"        cache_state \u003d \u0027infocache_hit\u0027"},{"line_number":886,"context_line":"    return info, cache_state"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bda5ed99_12fa8676","line":885,"in_reply_to":"735800fc_77b63f45","updated":"2023-06-23 05:13:56.000000000","message":"good point!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"51eaca9c63d182e1d1bd9b7b7c2efd589e8eb4ee","unresolved":true,"context_lines":[{"line_number":593,"context_line":"    :param  resp: the response from either backend or cache hit."},{"line_number":594,"context_line":"    \"\"\""},{"line_number":595,"context_line":"    # Note: don\u0027t use MetricsPrefixLoggerAdapter type of logger with this"},{"line_number":596,"context_line":"    # function, since loggers won\u0027t have prefixes under some usages."},{"line_number":597,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":598,"context_line":"    if logger:"},{"line_number":599,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7b5146ba_69de13ad","line":596,"updated":"2023-06-23 10:31:13.000000000","message":"I don\u0027t understand the comment. What would happen if we did use a MetricsPrefixLoggerAdapter ?","commit_id":"8126cc521aea8ccdc0d1422f1dfe3927bf49048a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d0edcbb3c823a35a07329f200cf0153080c86ef7","unresolved":false,"context_lines":[{"line_number":593,"context_line":"    :param  resp: the response from either backend or cache hit."},{"line_number":594,"context_line":"    \"\"\""},{"line_number":595,"context_line":"    # Note: don\u0027t use MetricsPrefixLoggerAdapter type of logger with this"},{"line_number":596,"context_line":"    # function, since loggers won\u0027t have prefixes under some usages."},{"line_number":597,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":598,"context_line":"    if logger:"},{"line_number":599,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bb5b05a7_aa105660","line":596,"in_reply_to":"7b5146ba_69de13ad","updated":"2023-06-24 03:50:07.000000000","message":"Got them deleted, not applicable anymore with new change.","commit_id":"8126cc521aea8ccdc0d1422f1dfe3927bf49048a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"51eaca9c63d182e1d1bd9b7b7c2efd589e8eb4ee","unresolved":true,"context_lines":[{"line_number":775,"context_line":"def _get_final_app_and_failsafe_logger(app):"},{"line_number":776,"context_line":"    # Try to cut through all the layers to the proxy app"},{"line_number":777,"context_line":"    try:"},{"line_number":778,"context_line":"        app \u003d app._pipeline_final_app"},{"line_number":779,"context_line":"    except AttributeError:"},{"line_number":780,"context_line":"        logger \u003d None"},{"line_number":781,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"0fa7ce1a_d88ff26d","line":778,"updated":"2023-06-23 10:31:13.000000000","message":"probably worth rebasing this patch on https://review.opendev.org/c/openstack/swift/+/884931","commit_id":"8126cc521aea8ccdc0d1422f1dfe3927bf49048a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"599fe634d269804c79a48ee5e473e6b6cd936d03","unresolved":false,"context_lines":[{"line_number":775,"context_line":"def _get_final_app_and_failsafe_logger(app):"},{"line_number":776,"context_line":"    # Try to cut through all the layers to the proxy app"},{"line_number":777,"context_line":"    try:"},{"line_number":778,"context_line":"        app \u003d app._pipeline_final_app"},{"line_number":779,"context_line":"    except AttributeError:"},{"line_number":780,"context_line":"        logger \u003d None"},{"line_number":781,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"20e903f7_6b59fead","line":778,"in_reply_to":"0fa7ce1a_d88ff26d","updated":"2023-06-23 21:34:26.000000000","message":"Done","commit_id":"8126cc521aea8ccdc0d1422f1dfe3927bf49048a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"51eaca9c63d182e1d1bd9b7b7c2efd589e8eb4ee","unresolved":true,"context_lines":[{"line_number":777,"context_line":"    try:"},{"line_number":778,"context_line":"        app \u003d app._pipeline_final_app"},{"line_number":779,"context_line":"    except AttributeError:"},{"line_number":780,"context_line":"        logger \u003d None"},{"line_number":781,"context_line":"    else:"},{"line_number":782,"context_line":"        logger \u003d app.logger"},{"line_number":783,"context_line":"    return app, logger"}],"source_content_type":"text/x-python","patch_set":4,"id":"d681773c_7dfe9dde","line":780,"updated":"2023-06-23 10:31:13.000000000","message":"what if app has a logger but not a _pipeline_final_app ? should we try app.logger, then fallback to app._pipeline_final_app.logger if it exists?","commit_id":"8126cc521aea8ccdc0d1422f1dfe3927bf49048a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d0edcbb3c823a35a07329f200cf0153080c86ef7","unresolved":true,"context_lines":[{"line_number":777,"context_line":"    try:"},{"line_number":778,"context_line":"        app \u003d app._pipeline_final_app"},{"line_number":779,"context_line":"    except AttributeError:"},{"line_number":780,"context_line":"        logger \u003d None"},{"line_number":781,"context_line":"    else:"},{"line_number":782,"context_line":"        logger \u003d app.logger"},{"line_number":783,"context_line":"    return app, logger"}],"source_content_type":"text/x-python","patch_set":4,"id":"a7d55f03_a73bffb9","line":780,"in_reply_to":"d681773c_7dfe9dde","updated":"2023-06-24 03:50:07.000000000","message":"after rebase, now I have this. Could you please help check again if my understanding of \u0027_pipeline_request_logging_app\u0027 is correct?\n\n    try:\n        logged_app \u003d app._pipeline_request_logging_app\n        proxy_app \u003d app._pipeline_final_app\n    except AttributeError:\n        logged_app \u003d proxy_app \u003d app\n        logger \u003d None\n    else:\n        logger \u003d logged_app.logger","commit_id":"8126cc521aea8ccdc0d1422f1dfe3927bf49048a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":800,"context_line":"                skip_chance \u003d proxy_app.container_existence_skip_cache"},{"line_number":801,"context_line":"            else:"},{"line_number":802,"context_line":"                skip_chance \u003d proxy_app.account_existence_skip_cache"},{"line_number":803,"context_line":"            logger \u003d proxy_app.logger"},{"line_number":804,"context_line":"        info_type \u003d \u0027container\u0027 if container else \u0027account\u0027"},{"line_number":805,"context_line":"        if skip_chance and random.random() \u003c skip_chance:"},{"line_number":806,"context_line":"            info \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"57cc7bd3_e8d3de13","side":"PARENT","line":803,"updated":"2023-06-27 02:05:47.000000000","message":"i like how it comes out that all the access to logger happens under this method now","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":597,"context_line":"    except AttributeError:"},{"line_number":598,"context_line":"        logger \u003d None"},{"line_number":599,"context_line":"    else:"},{"line_number":600,"context_line":"        logger \u003d proxy_app.logger"},{"line_number":601,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":602,"context_line":"    if logger:"},{"line_number":603,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"}],"source_content_type":"text/x-python","patch_set":6,"id":"c11bc0ae_d6147e5e","line":600,"updated":"2023-06-27 02:05:47.000000000","message":"oic, this is except/else\n\nso if we have a _pipeline_final_app we can count on it having a .logger attribute","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"95b417cec0beaaa961ca9499bfd11306eca38bd8","unresolved":false,"context_lines":[{"line_number":597,"context_line":"    except AttributeError:"},{"line_number":598,"context_line":"        logger \u003d None"},{"line_number":599,"context_line":"    else:"},{"line_number":600,"context_line":"        logger \u003d proxy_app.logger"},{"line_number":601,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":602,"context_line":"    if logger:"},{"line_number":603,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1c5cdd8c_4965091d","line":600,"in_reply_to":"c11bc0ae_d6147e5e","updated":"2023-07-03 20:19:53.000000000","message":"Ack","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":598,"context_line":"        logger \u003d None"},{"line_number":599,"context_line":"    else:"},{"line_number":600,"context_line":"        logger \u003d proxy_app.logger"},{"line_number":601,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":602,"context_line":"    if logger:"},{"line_number":603,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"},{"line_number":604,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bce30a80_e5230427","line":601,"updated":"2023-06-27 02:05:47.000000000","message":"this is the part that makes this function be record_ac_info - i\u0027m not sure I see the benifit of passing in container\u003dTrue/None as a kwarg param vs explict op_type","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"95b417cec0beaaa961ca9499bfd11306eca38bd8","unresolved":false,"context_lines":[{"line_number":598,"context_line":"        logger \u003d None"},{"line_number":599,"context_line":"    else:"},{"line_number":600,"context_line":"        logger \u003d proxy_app.logger"},{"line_number":601,"context_line":"    op_type \u003d \u0027container.info\u0027 if container else \u0027account.info\u0027"},{"line_number":602,"context_line":"    if logger:"},{"line_number":603,"context_line":"        record_cache_op_metrics(logger, op_type, cache_state, resp)"},{"line_number":604,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"4a26d657_6eba7aaf","line":601,"in_reply_to":"bce30a80_e5230427","updated":"2023-07-03 20:19:53.000000000","message":"\"record_cache_op_metrics\" was added when I worked on metrics for shard range caches, I feel it has a generic interface to be used by all different cases, shard range cache and container info cache. We could add a new function with container\u003dTrue/None as a kwarg param, but I feel reusing is better.\n\ndef record_cache_op_metrics(logger, op_type, cache_state, resp\u003dNone):","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":738,"context_line":"        path \u003d \u0027/v1/%s\u0027 % (account,)"},{"line_number":739,"context_line":"        path_env \u003d env.copy()"},{"line_number":740,"context_line":"        path_env[\u0027PATH_INFO\u0027] \u003d path"},{"line_number":741,"context_line":"        return get_account_info(path_env, app, swift_source\u003dswift_source)"},{"line_number":742,"context_line":""},{"line_number":743,"context_line":""},{"line_number":744,"context_line":"def _get_object_info(app, env, account, container, obj, swift_source\u003dNone):"}],"source_content_type":"text/x-python","patch_set":6,"id":"00e97cb6_50e48d7a","line":741,"updated":"2023-06-27 02:05:47.000000000","message":"maybe a reasonable pre-factor would be to pull out all these loose functions into their own module and get ordered correctly - then change them to have consistent telemetry","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":828,"context_line":"    # Try to cut through all the layers to the proxy app"},{"line_number":829,"context_line":"    # (while also preserving logging)"},{"line_number":830,"context_line":"    try:"},{"line_number":831,"context_line":"        logged_app \u003d app._pipeline_request_logging_app"},{"line_number":832,"context_line":"        proxy_app \u003d app._pipeline_final_app"},{"line_number":833,"context_line":"    except AttributeError:"},{"line_number":834,"context_line":"        logged_app \u003d proxy_app \u003d app"}],"source_content_type":"text/x-python","patch_set":6,"id":"46330123_11242f49","line":831,"updated":"2023-06-27 02:05:47.000000000","message":"I think \"logged\" might be a typo?\n\n    logging_app \u003d ...logging_app\n\nmight be better?   logger_app might be ok too.","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"95b417cec0beaaa961ca9499bfd11306eca38bd8","unresolved":false,"context_lines":[{"line_number":828,"context_line":"    # Try to cut through all the layers to the proxy app"},{"line_number":829,"context_line":"    # (while also preserving logging)"},{"line_number":830,"context_line":"    try:"},{"line_number":831,"context_line":"        logged_app \u003d app._pipeline_request_logging_app"},{"line_number":832,"context_line":"        proxy_app \u003d app._pipeline_final_app"},{"line_number":833,"context_line":"    except AttributeError:"},{"line_number":834,"context_line":"        logged_app \u003d proxy_app \u003d app"}],"source_content_type":"text/x-python","patch_set":6,"id":"4e282138_d3f5b335","line":831,"in_reply_to":"46330123_11242f49","updated":"2023-07-03 20:19:53.000000000","message":"this change was made by the patch I rebased on: https://review.opendev.org/c/openstack/swift/+/884931/","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":831,"context_line":"        logged_app \u003d app._pipeline_request_logging_app"},{"line_number":832,"context_line":"        proxy_app \u003d app._pipeline_final_app"},{"line_number":833,"context_line":"    except AttributeError:"},{"line_number":834,"context_line":"        logged_app \u003d proxy_app \u003d app"},{"line_number":835,"context_line":"    # Check in environment cache and in memcache (in that order)"},{"line_number":836,"context_line":"    info, cache_state \u003d _get_info_from_caches("},{"line_number":837,"context_line":"        proxy_app, env, account, container)"}],"source_content_type":"text/x-python","patch_set":6,"id":"c5f63eb9_f6ce8483","line":834,"updated":"2023-06-27 02:05:47.000000000","message":"i think it\u0027s a smell that we\u0027re handling two AttributeErrors at the same time\n\nis it even theoretically possible that we could have one of these but not both?","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"95b417cec0beaaa961ca9499bfd11306eca38bd8","unresolved":false,"context_lines":[{"line_number":831,"context_line":"        logged_app \u003d app._pipeline_request_logging_app"},{"line_number":832,"context_line":"        proxy_app \u003d app._pipeline_final_app"},{"line_number":833,"context_line":"    except AttributeError:"},{"line_number":834,"context_line":"        logged_app \u003d proxy_app \u003d app"},{"line_number":835,"context_line":"    # Check in environment cache and in memcache (in that order)"},{"line_number":836,"context_line":"    info, cache_state \u003d _get_info_from_caches("},{"line_number":837,"context_line":"        proxy_app, env, account, container)"}],"source_content_type":"text/x-python","patch_set":6,"id":"aca072fa_3fa5db2a","line":834,"in_reply_to":"c5f63eb9_f6ce8483","updated":"2023-07-03 20:19:53.000000000","message":"this change was made by the patch 884931 which this patch is rebased on.","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":834,"context_line":"        logged_app \u003d proxy_app \u003d app"},{"line_number":835,"context_line":"    # Check in environment cache and in memcache (in that order)"},{"line_number":836,"context_line":"    info, cache_state \u003d _get_info_from_caches("},{"line_number":837,"context_line":"        proxy_app, env, account, container)"},{"line_number":838,"context_line":""},{"line_number":839,"context_line":"    if info:"},{"line_number":840,"context_line":"        resp \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"32dd67de_1bf52150","line":837,"updated":"2023-06-27 02:05:47.000000000","message":"I think everyone under _get_info_from_caches knows to look for _pipeline_final_app to get any configured attributes - you may as well just pass app in here.","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":841,"context_line":"    elif cache_only:"},{"line_number":842,"context_line":"        # Cache miss and caller doesn\u0027t want to HEAD the backend container."},{"line_number":843,"context_line":"        record_ac_info_cache_metrics(logged_app, cache_state, container)"},{"line_number":844,"context_line":"        return headers_to_container_info({}, 0)"},{"line_number":845,"context_line":"    else:"},{"line_number":846,"context_line":"        # Cache miss; go HEAD the container and populate the caches"},{"line_number":847,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})"}],"source_content_type":"text/x-python","patch_set":6,"id":"32997bcd_a6481482","line":844,"updated":"2023-06-27 02:05:47.000000000","message":"ok, so we\u0027re not \"just\" moving these methods - this behavior is new","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"571fcbe23de79fbf4e436ff1f2c2088739ba9632","unresolved":false,"context_lines":[{"line_number":841,"context_line":"    elif cache_only:"},{"line_number":842,"context_line":"        # Cache miss and caller doesn\u0027t want to HEAD the backend container."},{"line_number":843,"context_line":"        record_ac_info_cache_metrics(logged_app, cache_state, container)"},{"line_number":844,"context_line":"        return headers_to_container_info({}, 0)"},{"line_number":845,"context_line":"    else:"},{"line_number":846,"context_line":"        # Cache miss; go HEAD the container and populate the caches"},{"line_number":847,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})"}],"source_content_type":"text/x-python","patch_set":6,"id":"81c40851_95140188","line":844,"in_reply_to":"32997bcd_a6481482","updated":"2023-07-04 01:01:35.000000000","message":"Ack","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":842,"context_line":"        # Cache miss and caller doesn\u0027t want to HEAD the backend container."},{"line_number":843,"context_line":"        record_ac_info_cache_metrics(logged_app, cache_state, container)"},{"line_number":844,"context_line":"        return headers_to_container_info({}, 0)"},{"line_number":845,"context_line":"    else:"},{"line_number":846,"context_line":"        # Cache miss; go HEAD the container and populate the caches"},{"line_number":847,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})"},{"line_number":848,"context_line":"        # Before checking the container, make sure the account exists."}],"source_content_type":"text/x-python","patch_set":6,"id":"9dfbdbb7_fb04ce30","line":845,"updated":"2023-06-27 02:05:47.000000000","message":"it might have been reasonable to make this whole branch \"elif not cache_only\" and let the \"if not info\" case always fall through, I actually ended up with\n\n\t-    if info:\n\t-        resp \u003d None\n\t-    elif cache_only:\n\t-        # Cache miss and caller doesn\u0027t want to HEAD the backend container.\n\t-        record_ac_info_cache_metrics(logged_app, cache_state, container)\n\t-        return headers_to_container_info({}, 0)\n\t-    else:\n\t+    resp \u003d None\n\t+    if not (info or cache_only):\n\t\t # Cache miss; go HEAD the container and populate the caches","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"016089ea1210f6a83d0467e433008a583a73dbf5","unresolved":false,"context_lines":[{"line_number":842,"context_line":"        # Cache miss and caller doesn\u0027t want to HEAD the backend container."},{"line_number":843,"context_line":"        record_ac_info_cache_metrics(logged_app, cache_state, container)"},{"line_number":844,"context_line":"        return headers_to_container_info({}, 0)"},{"line_number":845,"context_line":"    else:"},{"line_number":846,"context_line":"        # Cache miss; go HEAD the container and populate the caches"},{"line_number":847,"context_line":"        env.setdefault(\u0027swift.infocache\u0027, {})"},{"line_number":848,"context_line":"        # Before checking the container, make sure the account exists."}],"source_content_type":"text/x-python","patch_set":6,"id":"37e37a9b_092f2d4d","line":845,"in_reply_to":"9dfbdbb7_fb04ce30","updated":"2023-07-05 18:44:24.000000000","message":"Done","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":853,"context_line":"        # on disk or not."},{"line_number":854,"context_line":"        is_autocreate_account \u003d account.startswith("},{"line_number":855,"context_line":"            getattr(proxy_app, \u0027auto_create_account_prefix\u0027,"},{"line_number":856,"context_line":"                    constraints.AUTO_CREATE_ACCOUNT_PREFIX))"},{"line_number":857,"context_line":"        if not is_autocreate_account:"},{"line_number":858,"context_line":"            account_info \u003d get_account_info(env, logged_app, swift_source)"},{"line_number":859,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3e1d5c6d_48b6e1cf","line":856,"updated":"2023-06-27 02:05:47.000000000","message":"this is the only place this method needs a proxy_app attribute\n\notherwise we might have gotten away with\n\n    app \u003d app._pipeline_request_logging_app\n    \n... but I don\u0027t see an easy way to avoid this method wanting both proxy_app and logger_app","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"571fcbe23de79fbf4e436ff1f2c2088739ba9632","unresolved":false,"context_lines":[{"line_number":853,"context_line":"        # on disk or not."},{"line_number":854,"context_line":"        is_autocreate_account \u003d account.startswith("},{"line_number":855,"context_line":"            getattr(proxy_app, \u0027auto_create_account_prefix\u0027,"},{"line_number":856,"context_line":"                    constraints.AUTO_CREATE_ACCOUNT_PREFIX))"},{"line_number":857,"context_line":"        if not is_autocreate_account:"},{"line_number":858,"context_line":"            account_info \u003d get_account_info(env, logged_app, swift_source)"},{"line_number":859,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"}],"source_content_type":"text/x-python","patch_set":6,"id":"e3baf1ec_249d9fa9","line":856,"in_reply_to":"3e1d5c6d_48b6e1cf","updated":"2023-07-04 01:01:35.000000000","message":"Ack","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":854,"context_line":"        is_autocreate_account \u003d account.startswith("},{"line_number":855,"context_line":"            getattr(proxy_app, \u0027auto_create_account_prefix\u0027,"},{"line_number":856,"context_line":"                    constraints.AUTO_CREATE_ACCOUNT_PREFIX))"},{"line_number":857,"context_line":"        if not is_autocreate_account:"},{"line_number":858,"context_line":"            account_info \u003d get_account_info(env, logged_app, swift_source)"},{"line_number":859,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"},{"line_number":860,"context_line":"                record_ac_info_cache_metrics("}],"source_content_type":"text/x-python","patch_set":6,"id":"7923781b_827b01f5","line":857,"updated":"2023-06-27 02:05:47.000000000","message":"if you made this:\n\n\n    if not is_autocreate_account(app, account):\n         ...\n         \nYou could bury all the AttirbuteError/getattr stuff and just deal with only app in this method directly.","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":855,"context_line":"            getattr(proxy_app, \u0027auto_create_account_prefix\u0027,"},{"line_number":856,"context_line":"                    constraints.AUTO_CREATE_ACCOUNT_PREFIX))"},{"line_number":857,"context_line":"        if not is_autocreate_account:"},{"line_number":858,"context_line":"            account_info \u003d get_account_info(env, logged_app, swift_source)"},{"line_number":859,"context_line":"            if not account_info or not is_success(account_info[\u0027status\u0027]):"},{"line_number":860,"context_line":"                record_ac_info_cache_metrics("},{"line_number":861,"context_line":"                    logged_app, cache_state, container)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3caf15b6_1d899c10","line":858,"updated":"2023-06-27 02:05:47.000000000","message":"under the hood this is looking for logged_app._pipeline_request_logging_app","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":868,"context_line":"        # caller to keep the result private-ish"},{"line_number":869,"context_line":"        req.headers[\u0027X-Backend-Allow-Reserved-Names\u0027] \u003d \u0027true\u0027"},{"line_number":870,"context_line":"        resp \u003d req.get_response(logged_app)"},{"line_number":871,"context_line":"        drain_and_close(resp)"},{"line_number":872,"context_line":"        # Check in infocache to see if the proxy (or anyone else) already"},{"line_number":873,"context_line":"        # populated the cache for us. If they did, just use what\u0027s there."},{"line_number":874,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":6,"id":"e210ced4_aac1a343","line":871,"updated":"2023-06-27 02:05:47.000000000","message":"this is really the only part where we care about using \"logged_app\" instead of just the \"app\" that was passed in.\n\n... and we do something similar in get_account_info, maybe we could bury the _pipeline_request_logging_app under a new function:\n\n     resp \u003d get_logged_resp(app, req)","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"571fcbe23de79fbf4e436ff1f2c2088739ba9632","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        # caller to keep the result private-ish"},{"line_number":869,"context_line":"        req.headers[\u0027X-Backend-Allow-Reserved-Names\u0027] \u003d \u0027true\u0027"},{"line_number":870,"context_line":"        resp \u003d req.get_response(logged_app)"},{"line_number":871,"context_line":"        drain_and_close(resp)"},{"line_number":872,"context_line":"        # Check in infocache to see if the proxy (or anyone else) already"},{"line_number":873,"context_line":"        # populated the cache for us. If they did, just use what\u0027s there."},{"line_number":874,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":6,"id":"5c94395c_941588e0","line":871,"in_reply_to":"e210ced4_aac1a343","updated":"2023-07-04 01:01:35.000000000","message":"this change was done in patch https://review.opendev.org/c/openstack/swift/+/884931/","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":880,"context_line":"    if info:"},{"line_number":881,"context_line":"        info \u003d deepcopy(info)  # avoid mutating what\u0027s in swift.infocache"},{"line_number":882,"context_line":"    else:"},{"line_number":883,"context_line":"        info \u003d headers_to_container_info({}, 503)"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"    # Old data format in memcache immediately after a Swift upgrade; clean"},{"line_number":886,"context_line":"    # it up so consumers of get_container_info() aren\u0027t exposed to it."}],"source_content_type":"text/x-python","patch_set":6,"id":"c0fbab0c_9f900193","line":883,"updated":"2023-06-27 02:05:47.000000000","message":"I guess this is the main difference between \"tried to make backend request and still don\u0027t have any info\" vs \"no backend request because cache_only\u003dTrue\"","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"016089ea1210f6a83d0467e433008a583a73dbf5","unresolved":false,"context_lines":[{"line_number":880,"context_line":"    if info:"},{"line_number":881,"context_line":"        info \u003d deepcopy(info)  # avoid mutating what\u0027s in swift.infocache"},{"line_number":882,"context_line":"    else:"},{"line_number":883,"context_line":"        info \u003d headers_to_container_info({}, 503)"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"    # Old data format in memcache immediately after a Swift upgrade; clean"},{"line_number":886,"context_line":"    # it up so consumers of get_container_info() aren\u0027t exposed to it."}],"source_content_type":"text/x-python","patch_set":6,"id":"7e5129cb_2c158483","line":883,"in_reply_to":"c0fbab0c_9f900193","updated":"2023-07-05 18:44:24.000000000","message":"Ack","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":911,"context_line":"    return info"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":""},{"line_number":914,"context_line":"def get_account_info(env, app, swift_source\u003dNone):"},{"line_number":915,"context_line":"    \"\"\""},{"line_number":916,"context_line":"    Get the info structure for an account, based on env and app."},{"line_number":917,"context_line":"    This is useful to middlewares."}],"source_content_type":"text/x-python","patch_set":6,"id":"9e20fc4f_e0c485bf","line":914,"updated":"2023-06-27 02:05:47.000000000","message":"ok, cache_only interface is container_info only","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"571fcbe23de79fbf4e436ff1f2c2088739ba9632","unresolved":false,"context_lines":[{"line_number":911,"context_line":"    return info"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":""},{"line_number":914,"context_line":"def get_account_info(env, app, swift_source\u003dNone):"},{"line_number":915,"context_line":"    \"\"\""},{"line_number":916,"context_line":"    Get the info structure for an account, based on env and app."},{"line_number":917,"context_line":"    This is useful to middlewares."}],"source_content_type":"text/x-python","patch_set":6,"id":"39b6fff1_369bf82e","line":914,"in_reply_to":"9e20fc4f_e0c485bf","updated":"2023-07-04 01:01:35.000000000","message":"Ack","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":false,"context_lines":[{"line_number":445,"context_line":"    Get the info structure for a container, based on env and app."},{"line_number":446,"context_line":"    This is useful to middlewares."},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    :param env: the environment used by the current request"},{"line_number":449,"context_line":"    :param app: the application object"},{"line_number":450,"context_line":"    :param swift_source: Used to mark the request as originating out of"},{"line_number":451,"context_line":"                         middleware. Will be logged in proxy logs."}],"source_content_type":"text/x-python","patch_set":7,"id":"07b88506_348e21d1","line":448,"updated":"2023-07-05 23:25:00.000000000","message":"Better doc strings! 🤩","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":true,"context_lines":[{"line_number":446,"context_line":"    This is useful to middlewares."},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    :param env: the environment used by the current request"},{"line_number":449,"context_line":"    :param app: the application object"},{"line_number":450,"context_line":"    :param swift_source: Used to mark the request as originating out of"},{"line_number":451,"context_line":"                         middleware. Will be logged in proxy logs."},{"line_number":452,"context_line":"    :param cache_only: If true, indicates that caller doesn\u0027t want to HEAD the"}],"source_content_type":"text/x-python","patch_set":7,"id":"af86ac53_6ee4df67","line":449,"updated":"2023-07-05 23:25:00.000000000","message":"We might want to call out that we\u0027d really prefer one of the pipelined WSGI applications created by `loadapp`, so it\u0027ll get decorated properly. \\*shrug\\*","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":true,"context_lines":[{"line_number":450,"context_line":"    :param swift_source: Used to mark the request as originating out of"},{"line_number":451,"context_line":"                         middleware. Will be logged in proxy logs."},{"line_number":452,"context_line":"    :param cache_only: If true, indicates that caller doesn\u0027t want to HEAD the"},{"line_number":453,"context_line":"                       backend container when cache miss."},{"line_number":454,"context_line":"    :returns: the object info"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"    .. note::"}],"source_content_type":"text/x-python","patch_set":7,"id":"4fa7acf2_69e89e02","line":453,"updated":"2023-07-05 23:25:00.000000000","message":"Oh, this is new...","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5ccba13338797afc77d34d7603ad2e6b6c5d444f","unresolved":false,"context_lines":[{"line_number":450,"context_line":"    :param swift_source: Used to mark the request as originating out of"},{"line_number":451,"context_line":"                         middleware. Will be logged in proxy logs."},{"line_number":452,"context_line":"    :param cache_only: If true, indicates that caller doesn\u0027t want to HEAD the"},{"line_number":453,"context_line":"                       backend container when cache miss."},{"line_number":454,"context_line":"    :returns: the object info"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"    .. note::"}],"source_content_type":"text/x-python","patch_set":7,"id":"7c40a20d_d1d929b0","line":453,"in_reply_to":"4fa7acf2_69e89e02","updated":"2023-07-26 22:50:18.000000000","message":"Ack","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":true,"context_lines":[{"line_number":519,"context_line":"    if info:"},{"line_number":520,"context_line":"        info \u003d deepcopy(info)  # avoid mutating what\u0027s in swift.infocache"},{"line_number":521,"context_line":"    else:"},{"line_number":522,"context_line":"        status_int \u003d 0 if cache_only else 503"},{"line_number":523,"context_line":"        info \u003d headers_to_container_info({}, status_int)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    # Old data format in memcache immediately after a Swift upgrade; clean"}],"source_content_type":"text/x-python","patch_set":7,"id":"b0659d97_2953fbd4","line":522,"range":{"start_line":522,"start_character":21,"end_line":522,"end_character":22},"updated":"2023-07-05 23:25:00.000000000","message":"OK, we\u0027ve got some precedent for this with the\n```\nreturn headers_to_container_info({}, 0)\n```\nabove when we can\u0027t get account info.","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5ccba13338797afc77d34d7603ad2e6b6c5d444f","unresolved":false,"context_lines":[{"line_number":519,"context_line":"    if info:"},{"line_number":520,"context_line":"        info \u003d deepcopy(info)  # avoid mutating what\u0027s in swift.infocache"},{"line_number":521,"context_line":"    else:"},{"line_number":522,"context_line":"        status_int \u003d 0 if cache_only else 503"},{"line_number":523,"context_line":"        info \u003d headers_to_container_info({}, status_int)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"    # Old data format in memcache immediately after a Swift upgrade; clean"}],"source_content_type":"text/x-python","patch_set":7,"id":"b5eba840_6ef1ae9f","line":522,"range":{"start_line":522,"start_character":21,"end_line":522,"end_character":22},"in_reply_to":"b0659d97_2953fbd4","updated":"2023-07-26 22:50:18.000000000","message":"Ack","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":true,"context_lines":[{"line_number":803,"context_line":"        logger.increment(\u0027%s.infocache.hit\u0027 % op_type)"},{"line_number":804,"context_line":"    elif cache_state \u003d\u003d \u0027hit\u0027:"},{"line_number":805,"context_line":"        # memcache hits."},{"line_number":806,"context_line":"        logger.increment(\u0027%s.cache.hit\u0027 % op_type)"},{"line_number":807,"context_line":"    else:"},{"line_number":808,"context_line":"        # the cases of cache_state is memcache miss, error, skip, force_skip"},{"line_number":809,"context_line":"        # or disabled."}],"source_content_type":"text/x-python","patch_set":7,"id":"88222394_a459fcf2","line":806,"updated":"2023-07-05 23:25:00.000000000","message":"With the new `else:` block, I think we could take out this whole `elif` block. Then maybe flatten it down to something like\n```\n    if cache_state \u003d\u003d \u0027infocache_hit\u0027:\n        logger.increment(\u0027%s.infocache.hit\u0027 % op_type)\n    elif resp:\n        # went to backend\n        logger.increment(\n            \u0027%s.cache.%s.%d\u0027 % (op_type, cache_state, resp.status_int))\n    else:\n        # memcache hit; also, in some situations, we choose not to lookup\n        # backend after cache miss.\n        logger.increment(\u0027%s.cache.%s\u0027 % (op_type, cache_state))\n```","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5ccba13338797afc77d34d7603ad2e6b6c5d444f","unresolved":false,"context_lines":[{"line_number":803,"context_line":"        logger.increment(\u0027%s.infocache.hit\u0027 % op_type)"},{"line_number":804,"context_line":"    elif cache_state \u003d\u003d \u0027hit\u0027:"},{"line_number":805,"context_line":"        # memcache hits."},{"line_number":806,"context_line":"        logger.increment(\u0027%s.cache.hit\u0027 % op_type)"},{"line_number":807,"context_line":"    else:"},{"line_number":808,"context_line":"        # the cases of cache_state is memcache miss, error, skip, force_skip"},{"line_number":809,"context_line":"        # or disabled."}],"source_content_type":"text/x-python","patch_set":7,"id":"b0a2eba6_92280a69","line":806,"in_reply_to":"88222394_a459fcf2","updated":"2023-07-26 22:50:18.000000000","message":"I made the suggested changes, but ran into a few of failed test cases. In ContainerController GETorHEAD path, when it queries listing shard range from cache, it fabricated a Response object, and it is passed to record_cache_op_metrics() https://github.com/NVIDIA/swift/blob/master/swift/proxy/controllers/container.py#L198\nSo the metric name for listing shard range cache becomes \"container.shard_listing.cache.hit.200\"","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":true,"context_lines":[{"line_number":831,"context_line":"    \"\"\""},{"line_number":832,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":833,"context_line":"    if not memcache:"},{"line_number":834,"context_line":"        return None, \u0027disabled\u0027"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"    try:"},{"line_number":837,"context_line":"        proxy_app \u003d app._pipeline_final_app"}],"source_content_type":"text/x-python","patch_set":7,"id":"73986f94_5be45c62","line":834,"updated":"2023-07-05 23:25:00.000000000","message":"I\u0027m beginning to think we might want a proper [`Enum`](https://docs.python.org/3/library/enum.html) for these cache states -- maybe after we fully drop py2?","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5ccba13338797afc77d34d7603ad2e6b6c5d444f","unresolved":false,"context_lines":[{"line_number":831,"context_line":"    \"\"\""},{"line_number":832,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":833,"context_line":"    if not memcache:"},{"line_number":834,"context_line":"        return None, \u0027disabled\u0027"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"    try:"},{"line_number":837,"context_line":"        proxy_app \u003d app._pipeline_final_app"}],"source_content_type":"text/x-python","patch_set":7,"id":"32e3c5a1_e4318ce3","line":834,"in_reply_to":"73986f94_5be45c62","updated":"2023-07-26 22:50:18.000000000","message":"sure, will add a Enum for those cache stats after we are py3 only.","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"}],"swift/proxy/controllers/container.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d2c02e3105392e505d99df74039cdf79ae32d1e2","unresolved":true,"context_lines":[{"line_number":389,"context_line":"                and get_param(req, \u0027states\u0027) \u003d\u003d \u0027listing\u0027"},{"line_number":390,"context_line":"                and record_type !\u003d \u0027object\u0027):"},{"line_number":391,"context_line":"            may_get_listing_shards \u003d True"},{"line_number":392,"context_line":"            info, info_cache_state \u003d _get_info_from_caches("},{"line_number":393,"context_line":"                self.app, req.environ, self.account_name, self.container_name)"},{"line_number":394,"context_line":"            # record metrics for all container info cache cases, since we won\u0027t"},{"line_number":395,"context_line":"            # lookup backend for container info cache miss."}],"source_content_type":"text/x-python","patch_set":1,"id":"e3fdf153_9b6aea96","line":392,"updated":"2023-06-13 18:22:47.000000000","message":"I\u0027m trying to remember why we went with `_get_info_from_caches` instead of `get_container_info` in https://review.opendev.org/c/openstack/swift/+/761659 -- it feels weird to have metrics bleeding out like this :-/","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d0edcbb3c823a35a07329f200cf0153080c86ef7","unresolved":false,"context_lines":[{"line_number":389,"context_line":"                and get_param(req, \u0027states\u0027) \u003d\u003d \u0027listing\u0027"},{"line_number":390,"context_line":"                and record_type !\u003d \u0027object\u0027):"},{"line_number":391,"context_line":"            may_get_listing_shards \u003d True"},{"line_number":392,"context_line":"            info, info_cache_state \u003d _get_info_from_caches("},{"line_number":393,"context_line":"                self.app, req.environ, self.account_name, self.container_name)"},{"line_number":394,"context_line":"            # record metrics for all container info cache cases, since we won\u0027t"},{"line_number":395,"context_line":"            # lookup backend for container info cache miss."}],"source_content_type":"text/x-python","patch_set":1,"id":"16722754_16d3ce2f","line":392,"in_reply_to":"1ad38505_1c876104","updated":"2023-06-24 03:50:07.000000000","message":"Thanks for explaining this, I also added them into comments.","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"51eaca9c63d182e1d1bd9b7b7c2efd589e8eb4ee","unresolved":true,"context_lines":[{"line_number":389,"context_line":"                and get_param(req, \u0027states\u0027) \u003d\u003d \u0027listing\u0027"},{"line_number":390,"context_line":"                and record_type !\u003d \u0027object\u0027):"},{"line_number":391,"context_line":"            may_get_listing_shards \u003d True"},{"line_number":392,"context_line":"            info, info_cache_state \u003d _get_info_from_caches("},{"line_number":393,"context_line":"                self.app, req.environ, self.account_name, self.container_name)"},{"line_number":394,"context_line":"            # record metrics for all container info cache cases, since we won\u0027t"},{"line_number":395,"context_line":"            # lookup backend for container info cache miss."}],"source_content_type":"text/x-python","patch_set":1,"id":"1ad38505_1c876104","line":392,"in_reply_to":"e3fdf153_9b6aea96","updated":"2023-06-23 10:31:13.000000000","message":"IIRC, this code path is trying to construct a GET response from artefacts in cache, so we need both a set of headers (from container info) and a set of shard ranges to be found in cache. If info is not in cache, and therefore a backend request is needed anyway, then the thinking was to go ahead with the backend GET rather than a HEAD that may then be followed by a GET.\n\nWe could parameterise get_container_info to accept a cache_only kwarg and achieve the same thing???","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5c89b56597dfa99824afaa67868718fa0be280dc","unresolved":true,"context_lines":[{"line_number":391,"context_line":"            may_get_listing_shards \u003d True"},{"line_number":392,"context_line":"            info, info_cache_state \u003d _get_info_from_caches("},{"line_number":393,"context_line":"                self.app, req.environ, self.account_name, self.container_name)"},{"line_number":394,"context_line":"            # record metrics for all container info cache cases, since we won\u0027t"},{"line_number":395,"context_line":"            # lookup backend for container info cache miss."},{"line_number":396,"context_line":"            record_ac_info_cache_metrics("},{"line_number":397,"context_line":"                self.app, info_cache_state,"}],"source_content_type":"text/x-python","patch_set":1,"id":"29458530_e81f4a7e","line":394,"updated":"2023-06-09 19:37:01.000000000","message":"for container HEAD of GETorHEAD(), we alway lookup backend instead of memcache. So only need to call record_ac_info_cache_metrics here and once. I do have another pending WIP patch to lookup memcache though(I still feel we should).","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":391,"context_line":"            may_get_listing_shards \u003d True"},{"line_number":392,"context_line":"            info, info_cache_state \u003d _get_info_from_caches("},{"line_number":393,"context_line":"                self.app, req.environ, self.account_name, self.container_name)"},{"line_number":394,"context_line":"            # record metrics for all container info cache cases, since we won\u0027t"},{"line_number":395,"context_line":"            # lookup backend for container info cache miss."},{"line_number":396,"context_line":"            record_ac_info_cache_metrics("},{"line_number":397,"context_line":"                self.app, info_cache_state,"}],"source_content_type":"text/x-python","patch_set":1,"id":"581cf9d0_97165087","line":394,"in_reply_to":"29458530_e81f4a7e","updated":"2023-06-22 00:54:10.000000000","message":"I think Tim\u0027s comment is just noticing something, adding the metrics in this diff next to where you have to change the call return signature looks mostly reasonable until you consider the context of what\u0027s actually going on in this branch.\n\nI don\u0027t think this branch is relataed at all to the idea of using container info to respond to client facing HEAD requests (which I still feel might be reasoable to do)","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":true,"context_lines":[{"line_number":391,"context_line":"            may_get_listing_shards \u003d True"},{"line_number":392,"context_line":"            info, info_cache_state \u003d _get_info_from_caches("},{"line_number":393,"context_line":"                self.app, req.environ, self.account_name, self.container_name)"},{"line_number":394,"context_line":"            # record metrics for all container info cache cases, since we won\u0027t"},{"line_number":395,"context_line":"            # lookup backend for container info cache miss."},{"line_number":396,"context_line":"            record_ac_info_cache_metrics("},{"line_number":397,"context_line":"                self.app, info_cache_state,"}],"source_content_type":"text/x-python","patch_set":1,"id":"a2c2b870_7ab63943","line":394,"in_reply_to":"581cf9d0_97165087","updated":"2023-06-23 05:13:56.000000000","message":"yes, this branch is only for listing shard range GET, I was commenting on this line of comment.","commit_id":"b67e53efa75cc142bb527551e70c5ae09b34d9e3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":390,"context_line":"                and record_type !\u003d \u0027object\u0027):"},{"line_number":391,"context_line":"            may_get_listing_shards \u003d True"},{"line_number":392,"context_line":"            info, info_cache_state \u003d _get_info_from_caches("},{"line_number":393,"context_line":"                self.app, req.environ, self.account_name, self.container_name)"},{"line_number":394,"context_line":"            # record metrics for all container info cache cases, since we won\u0027t"},{"line_number":395,"context_line":"            # lookup backend for container info cache miss."},{"line_number":396,"context_line":"            record_ac_info_cache_metrics("}],"source_content_type":"text/x-python","patch_set":3,"id":"995f69ae_9fa72a65","line":393,"updated":"2023-06-22 00:54:10.000000000","message":"yes, I don\u0027t understand what\u0027s going on in this branch either - thisis some kind of shard-range listing request, and we want ... this is container info right?  But only internal-client uses proxy for shard-range requests - what are we doing with container info?","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":397,"context_line":"                self.logger.logger, info_cache_state, self.container_name)"},{"line_number":398,"context_line":"        else:"},{"line_number":399,"context_line":"            info \u003d None"},{"line_number":400,"context_line":"            may_get_listing_shards \u003d False"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        memcache \u003d cache_from_env(req.environ, True)"},{"line_number":403,"context_line":"        sr_cache_state \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"4cd68b70_d6175e10","line":400,"updated":"2023-06-22 00:54:10.000000000","message":"what is \"may_get_listing_shards\" \u003d False!?","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"d0edcbb3c823a35a07329f200cf0153080c86ef7","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                self.logger.logger, info_cache_state, self.container_name)"},{"line_number":398,"context_line":"        else:"},{"line_number":399,"context_line":"            info \u003d None"},{"line_number":400,"context_line":"            may_get_listing_shards \u003d False"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        memcache \u003d cache_from_env(req.environ, True)"},{"line_number":403,"context_line":"        sr_cache_state \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"af4ba52f_7c97eec8","line":400,"in_reply_to":"111d5afa_58c4ecc4","updated":"2023-06-24 03:50:07.000000000","message":"Ack","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"51eaca9c63d182e1d1bd9b7b7c2efd589e8eb4ee","unresolved":true,"context_lines":[{"line_number":397,"context_line":"                self.logger.logger, info_cache_state, self.container_name)"},{"line_number":398,"context_line":"        else:"},{"line_number":399,"context_line":"            info \u003d None"},{"line_number":400,"context_line":"            may_get_listing_shards \u003d False"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        memcache \u003d cache_from_env(req.environ, True)"},{"line_number":403,"context_line":"        sr_cache_state \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"111d5afa_58c4ecc4","line":400,"in_reply_to":"48a863e0_a42ea4ee","updated":"2023-06-23 10:31:13.000000000","message":"may_get_listing_shards \u003d False means the proxy knows a priori that the request will not get listing shards, so use _GETorHEAD_from_backend immediately.\n\nThe \u0027may_\u0027 part is a hint that the proxy doesn\u0027t ever know for certain if a request *will* return listing shards until it either (a) finds them in cache or (b) receives a backend response, given that the backend response may return *objects*. This is because the proxy doesn\u0027t know for certain (even if it finds container info in cache) if the backend replica that is chosen to handle the request is sharding/sharded. That\u0027s also why we have an \u0027auto\u0027 record_type to indicate to the backend that it should return shards if it can or objects otherwise.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                self.logger.logger, info_cache_state, self.container_name)"},{"line_number":398,"context_line":"        else:"},{"line_number":399,"context_line":"            info \u003d None"},{"line_number":400,"context_line":"            may_get_listing_shards \u003d False"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        memcache \u003d cache_from_env(req.environ, True)"},{"line_number":403,"context_line":"        sr_cache_state \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"48a863e0_a42ea4ee","line":400,"in_reply_to":"4cd68b70_d6175e10","updated":"2023-06-23 05:13:56.000000000","message":"this is the branch for: a. user container HEAD request. b. container GET for updating shard range. maybe in future we should split this function GETorHEAD() into two functions(GET() and HEAD())? then code path will be clearer.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":416,"context_line":"            resp \u003d self._GETorHEAD_from_backend(req)"},{"line_number":417,"context_line":"            if may_get_listing_shards and ("},{"line_number":418,"context_line":"                    not self.app.recheck_listing_shard_ranges or not memcache):"},{"line_number":419,"context_line":"                sr_cache_state \u003d \u0027disabled\u0027"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"        resp_record_type \u003d resp.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027)"},{"line_number":422,"context_line":"        if sr_cache_state:"}],"source_content_type":"text/x-python","patch_set":3,"id":"f169fe6f_37c47e99","line":419,"updated":"2023-06-22 00:54:10.000000000","message":"i guess this method has it\u0027s own cache state logging to worry about, maybe it could use the public get_container_info directly.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1c069ff32d70486d616ce93e03004bbd3bcb2ee6","unresolved":false,"context_lines":[{"line_number":416,"context_line":"            resp \u003d self._GETorHEAD_from_backend(req)"},{"line_number":417,"context_line":"            if may_get_listing_shards and ("},{"line_number":418,"context_line":"                    not self.app.recheck_listing_shard_ranges or not memcache):"},{"line_number":419,"context_line":"                sr_cache_state \u003d \u0027disabled\u0027"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"        resp_record_type \u003d resp.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027)"},{"line_number":422,"context_line":"        if sr_cache_state:"}],"source_content_type":"text/x-python","patch_set":3,"id":"4d9f6639_dde9fdae","line":419,"in_reply_to":"40ba95b9_3786449c","updated":"2023-06-25 05:37:32.000000000","message":"Done","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"78e9e0eebd06f3d732290a44820bb6492f37eb54","unresolved":true,"context_lines":[{"line_number":416,"context_line":"            resp \u003d self._GETorHEAD_from_backend(req)"},{"line_number":417,"context_line":"            if may_get_listing_shards and ("},{"line_number":418,"context_line":"                    not self.app.recheck_listing_shard_ranges or not memcache):"},{"line_number":419,"context_line":"                sr_cache_state \u003d \u0027disabled\u0027"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"        resp_record_type \u003d resp.headers.get(\u0027X-Backend-Record-Type\u0027, \u0027\u0027)"},{"line_number":422,"context_line":"        if sr_cache_state:"}],"source_content_type":"text/x-python","patch_set":3,"id":"40ba95b9_3786449c","line":419,"in_reply_to":"f169fe6f_37c47e99","updated":"2023-06-23 05:13:56.000000000","message":"agreed, waiting for feedback from Alistair on this...","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"23213b33fb3fcbf11b474d21e302a37b657db55e","unresolved":true,"context_lines":[{"line_number":392,"context_line":"            # Only lookup container info from cache and skip the backend HEAD,"},{"line_number":393,"context_line":"            # since we are going to GET the backend container anyway."},{"line_number":394,"context_line":"            info \u003d get_container_info("},{"line_number":395,"context_line":"                req.environ, self.app, swift_source\u003dNone, cache_only\u003dTrue)"},{"line_number":396,"context_line":"        else:"},{"line_number":397,"context_line":"            info \u003d None"},{"line_number":398,"context_line":"            may_get_listing_shards \u003d False"}],"source_content_type":"text/x-python","patch_set":6,"id":"63fafe4b_056a2f5c","line":395,"updated":"2023-06-27 02:05:47.000000000","message":"thanks for the explinaion, this makes sense now and I like the new cache_only kwarg","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"571fcbe23de79fbf4e436ff1f2c2088739ba9632","unresolved":false,"context_lines":[{"line_number":392,"context_line":"            # Only lookup container info from cache and skip the backend HEAD,"},{"line_number":393,"context_line":"            # since we are going to GET the backend container anyway."},{"line_number":394,"context_line":"            info \u003d get_container_info("},{"line_number":395,"context_line":"                req.environ, self.app, swift_source\u003dNone, cache_only\u003dTrue)"},{"line_number":396,"context_line":"        else:"},{"line_number":397,"context_line":"            info \u003d None"},{"line_number":398,"context_line":"            may_get_listing_shards \u003d False"}],"source_content_type":"text/x-python","patch_set":6,"id":"563c6f11_2a3449b4","line":395,"in_reply_to":"63fafe4b_056a2f5c","updated":"2023-07-04 01:01:35.000000000","message":"Ack","commit_id":"a9c43236b24d0b33ba890db3a1494f3b2536f490"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f357df287b232ec97f84981adedbc7e98a42cf83","unresolved":true,"context_lines":[{"line_number":392,"context_line":"            # Only lookup container info from cache and skip the backend HEAD,"},{"line_number":393,"context_line":"            # since we are going to GET the backend container anyway."},{"line_number":394,"context_line":"            info \u003d get_container_info("},{"line_number":395,"context_line":"                req.environ, self.app, swift_source\u003dNone, cache_only\u003dTrue)"},{"line_number":396,"context_line":"        else:"},{"line_number":397,"context_line":"            info \u003d None"},{"line_number":398,"context_line":"            may_get_listing_shards \u003d False"}],"source_content_type":"text/x-python","patch_set":7,"id":"0410e113_469f4176","line":395,"range":{"start_line":395,"start_character":58,"end_line":395,"end_character":73},"updated":"2023-07-05 23:25:00.000000000","message":"Ah, this is where we use that! Yeah, I like it *way* better now.","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5ccba13338797afc77d34d7603ad2e6b6c5d444f","unresolved":false,"context_lines":[{"line_number":392,"context_line":"            # Only lookup container info from cache and skip the backend HEAD,"},{"line_number":393,"context_line":"            # since we are going to GET the backend container anyway."},{"line_number":394,"context_line":"            info \u003d get_container_info("},{"line_number":395,"context_line":"                req.environ, self.app, swift_source\u003dNone, cache_only\u003dTrue)"},{"line_number":396,"context_line":"        else:"},{"line_number":397,"context_line":"            info \u003d None"},{"line_number":398,"context_line":"            may_get_listing_shards \u003d False"}],"source_content_type":"text/x-python","patch_set":7,"id":"8e0c7553_5e89202a","line":395,"range":{"start_line":395,"start_character":58,"end_line":395,"end_character":73},"in_reply_to":"0410e113_469f4176","updated":"2023-07-26 22:50:18.000000000","message":"Ack","commit_id":"8dddd65823aa37511db720612ca6ad013916190a"}],"test/unit/proxy/controllers/test_container.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":2915,"context_line":"        self.assertEqual("},{"line_number":2916,"context_line":"            [x[0][0] for x in self.logger.logger.log_dict[\u0027increment\u0027]],"},{"line_number":2917,"context_line":"            [\u0027container.info.infocache.hit\u0027,"},{"line_number":2918,"context_line":"             \u0027container.shard_listing.infocache.hit\u0027])"},{"line_number":2919,"context_line":""},{"line_number":2920,"context_line":"        # put this back the way we found it for later subtests"},{"line_number":2921,"context_line":"        self.app.container_listing_shard_ranges_skip_cache \u003d 0.0"}],"source_content_type":"text/x-python","patch_set":3,"id":"80fe55ed_4d5b4ae1","line":2918,"updated":"2023-06-22 00:54:10.000000000","message":"this is a behavior change, yes?  This is good, this is what we want, we want to know if we\u0027re getting container info from the infocache hits!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1c069ff32d70486d616ce93e03004bbd3bcb2ee6","unresolved":false,"context_lines":[{"line_number":2915,"context_line":"        self.assertEqual("},{"line_number":2916,"context_line":"            [x[0][0] for x in self.logger.logger.log_dict[\u0027increment\u0027]],"},{"line_number":2917,"context_line":"            [\u0027container.info.infocache.hit\u0027,"},{"line_number":2918,"context_line":"             \u0027container.shard_listing.infocache.hit\u0027])"},{"line_number":2919,"context_line":""},{"line_number":2920,"context_line":"        # put this back the way we found it for later subtests"},{"line_number":2921,"context_line":"        self.app.container_listing_shard_ranges_skip_cache \u003d 0.0"}],"source_content_type":"text/x-python","patch_set":3,"id":"d9e32782_632b5c4a","line":2918,"in_reply_to":"80fe55ed_4d5b4ae1","updated":"2023-06-25 05:37:32.000000000","message":"yes, we are seeing this metric now which we should have.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":3315,"context_line":"                         self.memcache.calls[2][1][1][\u0027sharding_state\u0027])"},{"line_number":3316,"context_line":"        self.assertEqual({\u0027container.info.cache.miss\u0027: 1,"},{"line_number":3317,"context_line":"                          \u0027container.shard_listing.cache.force_skip.200\u0027: 1},"},{"line_number":3318,"context_line":"                         self.logger.get_increment_counts())"},{"line_number":3319,"context_line":""},{"line_number":3320,"context_line":"    def _do_test_GET_shard_ranges_no_cache_write(self, resp_hdrs):"},{"line_number":3321,"context_line":"        # verify that there is a cache lookup to check container info but then"}],"source_content_type":"text/x-python","patch_set":3,"id":"8ae8286b_9968bcb0","line":3318,"updated":"2023-06-22 00:54:10.000000000","message":"hrm... it seems like the container info metrics logging during a shard range listing cache hitting request may have been inconsistent","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":4265,"context_line":"                 \u0027account.info.infocache.hit\u0027: 2,"},{"line_number":4266,"context_line":"                 \u0027container.info.cache.disabled.200\u0027: 1,"},{"line_number":4267,"context_line":"                 \u0027container.info.infocache.hit\u0027: 1,"},{"line_number":4268,"context_line":"                 \u0027object.shard_updating.cache.disabled.200\u0027: 1},"},{"line_number":4269,"context_line":"                stats)"},{"line_number":4270,"context_line":"            backend_requests \u003d fake_conn.requests"},{"line_number":4271,"context_line":"            # verify statsd prefix is not mutated"}],"source_content_type":"text/x-python","patch_set":3,"id":"7bec4cea_81c0f343","line":4268,"updated":"2023-06-22 00:54:10.000000000","message":"I wonder how much math we should consider to think about the metrics explosion\n\nI think the way it worked was every new graphite key was ~40G (or like 40M per node or something?).  SRE might know better how we configure retention, prometheus might have a totally different storage sub-system and AFAIK we\u0027re off the legacy controller metrics seutp!","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"599fe634d269804c79a48ee5e473e6b6cd936d03","unresolved":true,"context_lines":[{"line_number":4265,"context_line":"                 \u0027account.info.infocache.hit\u0027: 2,"},{"line_number":4266,"context_line":"                 \u0027container.info.cache.disabled.200\u0027: 1,"},{"line_number":4267,"context_line":"                 \u0027container.info.infocache.hit\u0027: 1,"},{"line_number":4268,"context_line":"                 \u0027object.shard_updating.cache.disabled.200\u0027: 1},"},{"line_number":4269,"context_line":"                stats)"},{"line_number":4270,"context_line":"            backend_requests \u003d fake_conn.requests"},{"line_number":4271,"context_line":"            # verify statsd prefix is not mutated"}],"source_content_type":"text/x-python","patch_set":3,"id":"171b3532_0c68a9fb","line":4268,"in_reply_to":"7bec4cea_81c0f343","updated":"2023-06-23 21:34:26.000000000","message":"I am checking with Charles and Hugo on this.","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f23f6518973998565025566797b597877f809abc","unresolved":true,"context_lines":[{"line_number":4832,"context_line":"                {\u0027account.info.cache.disabled.200\u0027: 1,"},{"line_number":4833,"context_line":"                 \u0027account.info.infocache.hit\u0027: 2,"},{"line_number":4834,"context_line":"                 \u0027container.info.cache.disabled.200\u0027: 1,"},{"line_number":4835,"context_line":"                 \u0027container.info.infocache.hit\u0027: 1,"},{"line_number":4836,"context_line":"                 \u0027object.shard_updating.cache.disabled.404\u0027: 1},"},{"line_number":4837,"context_line":"                stats)"},{"line_number":4838,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"528d62bf_48280784","line":4835,"updated":"2023-06-22 00:54:10.000000000","message":"i guess the way these tests are setup we don\u0027t get many \"memcache.hit\" tests","commit_id":"a0258c8f33b6a2c03ec3c0a49a7124f795f0230b"}]}