)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"04fc4dd692792b9977e7b55243dc93ac37cad4fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7fdd7428_bde0acab","updated":"2023-06-05 14:14:29.000000000","message":"I\u0027m finding it a little confusing to differentiate \"logged_app\" from apps that have loggers (including the ultimate_app) - is there any name that would provide more distinction? \"request_logging_app\"??","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"28502b6e_8d9b39fa","updated":"2023-06-09 14:01:27.000000000","message":"I\u0027m concerned about the lack of defence in app_property, and we\u0027ll need some tests in test_wsgi at least.","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e31fcea35951a8e03b3196fc7eaf0659aefd4dd4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4cb4a91e_f5abcd3e","updated":"2023-06-08 21:19:15.000000000","message":"I\u0027m left wondering whether anyone has _ever_ successfully run with a non-default `auto_create_account_prefix` 😕","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"472ab9be9e1a1c5e716deb741756af6ef5c0dfad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"dabbc637_54fc367b","updated":"2023-06-14 11:52:29.000000000","message":"@Tim, I added some tests and rebased - couple of queries for you in the test though","commit_id":"6481cb4ce9e18be7bd279ab50eadfa76863283ac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"57fb031e78b0c9d4d37e7cff5ee99acbacf8508d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"d2f9ee51_db90a760","updated":"2023-06-14 18:42:00.000000000","message":"Ugh, I botched things yesterday such that this doesn\u0027t even do what it was supposed to any more (at least, not in all cases) :-(","commit_id":"167d3b5eaf8f92975f932af2ea544b434db247da"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"00319689c62f8606a56ec821bda3413f3e3209bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"e3e6bf13_845ae839","updated":"2023-07-13 06:53:54.000000000","message":"Maybe more of a NIT, but question/comment inline","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"55d9d1ee84cca954d5c9c8f745e09c22f12709a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"5957e169_b817f87f","updated":"2023-07-14 09:00:25.000000000","message":"+1 pending resolution of Matt\u0027s point about the request_logging_app._pipeline","commit_id":"783c980c3e2e3431e7e6d3a4c2e5b280c43c92c7"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"e223ddce90c0e8d8cd50ea69f821c09d7dc6148f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"2f5fb7e1_40a55cc2","updated":"2023-07-14 01:48:56.000000000","message":"kk, I think I\u0027m on board with this. If we do want to do some refactor cleanup as Al suggests maybe we can do that as a follow up.\nWe now expect an app to have a _pipeline etc, I wonder if this pipeline should actaully be the pipeline for that wsgi app, so in the case of request_logging_app it\u0027s _pipline should be: [request_logging_app, ultimate_app]\n\nThat\u0027s the question that\u0027s holding off my +2.","commit_id":"783c980c3e2e3431e7e6d3a4c2e5b280c43c92c7"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"add6c447d97ac79115239f6a095c53185efdddf3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"98fb33d0_a598376b","updated":"2023-08-01 12:13:19.000000000","message":"AFAICT there was no test coverage of the actual bug fix - see https://review.opendev.org/c/openstack/swift/+/890196","commit_id":"6d82511e3fc81577bf7ef3eb812ee0ace820b2d0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cebe69b9622eedc19a5a6e49d7f745438e7a1db5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"73c06027_c55028a4","updated":"2023-08-02 10:26:08.000000000","message":"my remaining comments were nit-like - let\u0027s get this merged (there\u0027s a bunch of dependent patches piled up on it) and build on it","commit_id":"c51e81f640274db31bf45b5253d8e99d19a7e3d6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"43f4b4ea563f39a3d3416ff8c2d0fed4bffcfd43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"50281924_9b69c3ef","updated":"2023-08-02 10:34:27.000000000","message":"trivial follow-up https://review.opendev.org/c/openstack/swift/+/890325","commit_id":"c51e81f640274db31bf45b5253d8e99d19a7e3d6"}],"swift/common/middleware/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"def app_property(name):"},{"line_number":21,"context_line":"    return property(lambda self: getattr(self.app, name))"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class RewriteContext(WSGIContext):"}],"source_content_type":"text/x-python","patch_set":6,"id":"8e09bc8a_edf0302b","line":21,"updated":"2023-06-09 14:01:27.000000000","message":"this assumes that app has the attr \u0027name\u0027 but in other places (base.py) we\u0027re careful to catch AttributeError when accessing these attrs","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7512289c7f76adee9d99d990091612ec21c1cffb","unresolved":true,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"def app_property(name):"},{"line_number":21,"context_line":"    return property(lambda self: getattr(self.app, name))"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class RewriteContext(WSGIContext):"}],"source_content_type":"text/x-python","patch_set":6,"id":"c0acd9c3_93da670b","line":21,"in_reply_to":"8e09bc8a_edf0302b","updated":"2023-06-12 17:22:16.000000000","message":"OIC (maybe) - if self.app does not have attr \u0027name\u0027 then this will raise AttributeError (I was thinking before that it defaulted to None ?!?) - then in base.py we try/except when accessing the property","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4c6f82d7f58ea943a09a876e76bc09a0190ede03","unresolved":true,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"def app_property(name):"},{"line_number":21,"context_line":"    return property(lambda self: getattr(self.app, name))"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class RewriteContext(WSGIContext):"}],"source_content_type":"text/x-python","patch_set":6,"id":"982d79fe_e8db4dfc","line":21,"in_reply_to":"8e09bc8a_edf0302b","updated":"2023-06-12 17:53:59.000000000","message":"Yes, exactly -- though there _is_ a slight difference in behavior: previously, we\u0027d get that `AttributeError` during start-up, but now it\u0027s delayed until run-time. That\u0027s fine, though, since we\u0027re already guarding against it.\n\nI\u0027m not sure we can really _avoid_ having the (run-time) checks like we do in base.py, either -- `get_account_info` and `get_container_info` are established parts of our API for middleware, and we have no guarantees about what callers will pass us.\n\nBut yeah, if `self.app` doesn\u0027t actually have an attribute named `name`, we\u0027ll still raise an `AttributeError` when trying to access it on `self`, so the checks in base.py will all still work.","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"bfd21f23a76c24bf9d69e2cafa3247088a38a2c9","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"def app_property(name):"},{"line_number":21,"context_line":"    return property(lambda self: getattr(self.app, name))"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class RewriteContext(WSGIContext):"}],"source_content_type":"text/x-python","patch_set":6,"id":"c56be136_1afffe63","line":21,"in_reply_to":"982d79fe_e8db4dfc","updated":"2023-06-12 19:51:46.000000000","message":"Done","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"}],"swift/common/middleware/s3api/s3api.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":173,"context_line":"    # odds to skip cache"},{"line_number":174,"context_line":"    _pipeline_final_app \u003d app_property(\u0027_pipeline_final_app\u0027)"},{"line_number":175,"context_line":"    _pipeline_request_logging_app \u003d app_property("},{"line_number":176,"context_line":"        \u0027_pipeline_request_logging_app\u0027)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def __call__(self, env, start_response):"},{"line_number":179,"context_line":"        # a lot of this is cribbed from listing_formats / swob.Request"}],"source_content_type":"text/x-python","patch_set":6,"id":"bcd7fbf4_0cce94b0","line":176,"updated":"2023-06-09 14:01:27.000000000","message":"see my comment in wsgi.py - maybe we should encapsulate all these pipeline properties into a single object","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"add6c447d97ac79115239f6a095c53185efdddf3","unresolved":true,"context_lines":[{"line_number":173,"context_line":"    # odds to skip cache"},{"line_number":174,"context_line":"    _pipeline_final_app \u003d app_property(\u0027_pipeline_final_app\u0027)"},{"line_number":175,"context_line":"    _pipeline_request_logging_app \u003d app_property("},{"line_number":176,"context_line":"        \u0027_pipeline_request_logging_app\u0027)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def __call__(self, env, start_response):"},{"line_number":179,"context_line":"        # a lot of this is cribbed from listing_formats / swob.Request"}],"source_content_type":"text/x-python","patch_set":6,"id":"639c348f_d62ef225","line":176,"in_reply_to":"bcd7fbf4_0cce94b0","updated":"2023-08-01 12:13:19.000000000","message":"understanding check: AFAICT these properties are not actually used by this middleware, but they are added in order to maintain the pattern that every middleware has the getters available?","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"}],"swift/common/middleware/versioned_writes/object_versioning.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":1384,"context_line":"    # odds to skip cache"},{"line_number":1385,"context_line":"    _pipeline_final_app \u003d app_property(\u0027_pipeline_final_app\u0027)"},{"line_number":1386,"context_line":"    _pipeline_request_logging_app \u003d app_property("},{"line_number":1387,"context_line":"        \u0027_pipeline_request_logging_app\u0027)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"    def account_request(self, req, api_version, account, start_response):"},{"line_number":1390,"context_line":"        account_ctx \u003d AccountContext(self.app, self.logger)"}],"source_content_type":"text/x-python","patch_set":6,"id":"108beca5_c507ab2c","line":1387,"updated":"2023-06-09 14:01:27.000000000","message":"+1 for relocating to this class","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"539879aff0cac9df96c7d93d075e7b5b9f2fed4c","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"    # odds to skip cache"},{"line_number":1385,"context_line":"    _pipeline_final_app \u003d app_property(\u0027_pipeline_final_app\u0027)"},{"line_number":1386,"context_line":"    _pipeline_request_logging_app \u003d app_property("},{"line_number":1387,"context_line":"        \u0027_pipeline_request_logging_app\u0027)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"    def account_request(self, req, api_version, account, start_response):"},{"line_number":1390,"context_line":"        account_ctx \u003d AccountContext(self.app, self.logger)"}],"source_content_type":"text/x-python","patch_set":6,"id":"6069dd71_5ed16e2b","line":1387,"in_reply_to":"108beca5_c507ab2c","updated":"2023-06-12 18:37:02.000000000","message":"Ack","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"}],"swift/common/wsgi.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"04fc4dd692792b9977e7b55243dc93ac37cad4fb","unresolved":true,"context_lines":[{"line_number":364,"context_line":"        pipeline \u003d [ultimate_app]"},{"line_number":365,"context_line":"        ultimate_app._pipeline \u003d pipeline"},{"line_number":366,"context_line":"        ultimate_app._pipeline_final_app \u003d ultimate_app"},{"line_number":367,"context_line":"        proxy_loggers \u003d []"},{"line_number":368,"context_line":"        app \u003d ultimate_app"},{"line_number":369,"context_line":"        for filter_app in filters:"},{"line_number":370,"context_line":"            app \u003d filter_app(pipeline[0])"}],"source_content_type":"text/x-python","patch_set":4,"id":"93bf7364_62dc2cc8","line":367,"updated":"2023-06-05 14:14:29.000000000","message":"IIUC only index 0 in the list that is used, so could use a var logged_app \u003d None and only set it if still None","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"96eb857a26377f4244d37bcbbfa18e749eba0d9e","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        pipeline \u003d [ultimate_app]"},{"line_number":365,"context_line":"        ultimate_app._pipeline \u003d pipeline"},{"line_number":366,"context_line":"        ultimate_app._pipeline_final_app \u003d ultimate_app"},{"line_number":367,"context_line":"        proxy_loggers \u003d []"},{"line_number":368,"context_line":"        app \u003d ultimate_app"},{"line_number":369,"context_line":"        for filter_app in filters:"},{"line_number":370,"context_line":"            app \u003d filter_app(pipeline[0])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7d778ed3_e60e4b90","line":367,"in_reply_to":"93bf7364_62dc2cc8","updated":"2023-06-05 22:51:16.000000000","message":"Good point.","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"fa7b7b5611435e3bfe74a577b8eea38a4d6bc686","unresolved":true,"context_lines":[{"line_number":369,"context_line":"        for filter_app in filters:"},{"line_number":370,"context_line":"            app \u003d filter_app(pipeline[0])"},{"line_number":371,"context_line":"            pipeline.insert(0, app)"},{"line_number":372,"context_line":"            if app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":373,"context_line":"                proxy_loggers.append(filter_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"}],"source_content_type":"text/x-python","patch_set":4,"id":"57535385_5abce016","line":372,"updated":"2023-06-02 21:56:43.000000000","message":"Can\u0027t use `isinstance`; pulling in `swift.common.middleware.proxy_logging` would cause a circular import.","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":false,"context_lines":[{"line_number":369,"context_line":"        for filter_app in filters:"},{"line_number":370,"context_line":"            app \u003d filter_app(pipeline[0])"},{"line_number":371,"context_line":"            pipeline.insert(0, app)"},{"line_number":372,"context_line":"            if app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":373,"context_line":"                proxy_loggers.append(filter_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"}],"source_content_type":"text/x-python","patch_set":4,"id":"df72e6c7_4561eb7d","line":372,"in_reply_to":"4af5abff_7a9e6c5c","updated":"2023-06-09 14:01:27.000000000","message":"Done","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"04fc4dd692792b9977e7b55243dc93ac37cad4fb","unresolved":true,"context_lines":[{"line_number":369,"context_line":"        for filter_app in filters:"},{"line_number":370,"context_line":"            app \u003d filter_app(pipeline[0])"},{"line_number":371,"context_line":"            pipeline.insert(0, app)"},{"line_number":372,"context_line":"            if app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":373,"context_line":"                proxy_loggers.append(filter_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"}],"source_content_type":"text/x-python","patch_set":4,"id":"4af5abff_7a9e6c5c","line":372,"in_reply_to":"57535385_5abce016","updated":"2023-06-05 14:14:29.000000000","message":"or, add a var like \u0027_proxy_logger\u0027 to ProxyLoggingMiddleware to be a little less brittle (if someone were to ever subclass ProxyLoggingMiddleware)","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"fa7b7b5611435e3bfe74a577b8eea38a4d6bc686","unresolved":true,"context_lines":[{"line_number":373,"context_line":"                proxy_loggers.append(filter_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        proxy_loggers.append(lambda app: app)"},{"line_number":377,"context_line":"        logged_app \u003d proxy_loggers[0](ultimate_app)"},{"line_number":378,"context_line":"        for x in pipeline:"},{"line_number":379,"context_line":"            x._pipeline_logged_app \u003d logged_app"}],"source_content_type":"text/x-python","patch_set":4,"id":"dacee4fe_4ea08529","line":376,"updated":"2023-06-02 21:56:43.000000000","message":"Make sure we don\u0027t trip an `IndexError` (for example, with backend servers).","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4c6f82d7f58ea943a09a876e76bc09a0190ede03","unresolved":false,"context_lines":[{"line_number":373,"context_line":"                proxy_loggers.append(filter_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        proxy_loggers.append(lambda app: app)"},{"line_number":377,"context_line":"        logged_app \u003d proxy_loggers[0](ultimate_app)"},{"line_number":378,"context_line":"        for x in pipeline:"},{"line_number":379,"context_line":"            x._pipeline_logged_app \u003d logged_app"}],"source_content_type":"text/x-python","patch_set":4,"id":"733d16c9_c73c0544","line":376,"in_reply_to":"dacee4fe_4ea08529","updated":"2023-06-12 17:53:59.000000000","message":"Ack","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"fa7b7b5611435e3bfe74a577b8eea38a4d6bc686","unresolved":true,"context_lines":[{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        proxy_loggers.append(lambda app: app)"},{"line_number":377,"context_line":"        logged_app \u003d proxy_loggers[0](ultimate_app)"},{"line_number":378,"context_line":"        for x in pipeline:"},{"line_number":379,"context_line":"            x._pipeline_logged_app \u003d logged_app"},{"line_number":380,"context_line":"        return app"}],"source_content_type":"text/x-python","patch_set":4,"id":"95757cf2_ee651666","line":377,"updated":"2023-06-02 21:56:43.000000000","message":"Note that this may not correspond to *any* position in the pipeline -- we\u0027re making a **new** filtered app here!","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"96eb857a26377f4244d37bcbbfa18e749eba0d9e","unresolved":true,"context_lines":[{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        proxy_loggers.append(lambda app: app)"},{"line_number":377,"context_line":"        logged_app \u003d proxy_loggers[0](ultimate_app)"},{"line_number":378,"context_line":"        for x in pipeline:"},{"line_number":379,"context_line":"            x._pipeline_logged_app \u003d logged_app"},{"line_number":380,"context_line":"        return app"}],"source_content_type":"text/x-python","patch_set":4,"id":"8fc0cee4_44763e86","line":377,"in_reply_to":"476cb0c5_9b40e0f1","updated":"2023-06-05 22:51:16.000000000","message":"In case someone\u0027s misconfigured their pipeline, like\n```\npipeline \u003d catch_errors gatekeeper healthcheck proxy-logging cache\n listing_formats container_sync bulk tempurl ratelimit tempauth copy\n container-quotas account-quotas proxy-logging slo dlo versioned_writes\n symlink proxy-server\n```","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":false,"context_lines":[{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        proxy_loggers.append(lambda app: app)"},{"line_number":377,"context_line":"        logged_app \u003d proxy_loggers[0](ultimate_app)"},{"line_number":378,"context_line":"        for x in pipeline:"},{"line_number":379,"context_line":"            x._pipeline_logged_app \u003d logged_app"},{"line_number":380,"context_line":"        return app"}],"source_content_type":"text/x-python","patch_set":4,"id":"3857539a_e42272bf","line":377,"in_reply_to":"8fc0cee4_44763e86","updated":"2023-06-09 14:01:27.000000000","message":"Done","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"51bfbe8bc823811194d19b03dc0c53cfe2c4ed25","unresolved":true,"context_lines":[{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        proxy_loggers.append(lambda app: app)"},{"line_number":377,"context_line":"        logged_app \u003d proxy_loggers[0](ultimate_app)"},{"line_number":378,"context_line":"        for x in pipeline:"},{"line_number":379,"context_line":"            x._pipeline_logged_app \u003d logged_app"},{"line_number":380,"context_line":"        return app"}],"source_content_type":"text/x-python","patch_set":4,"id":"cd24eb50_22e86316","line":377,"in_reply_to":"95757cf2_ee651666","updated":"2023-06-04 20:12:04.000000000","message":"Oh! But then we need to hang the `_pipeline`\\* properties off *it*, too...","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"04fc4dd692792b9977e7b55243dc93ac37cad4fb","unresolved":true,"context_lines":[{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        proxy_loggers.append(lambda app: app)"},{"line_number":377,"context_line":"        logged_app \u003d proxy_loggers[0](ultimate_app)"},{"line_number":378,"context_line":"        for x in pipeline:"},{"line_number":379,"context_line":"            x._pipeline_logged_app \u003d logged_app"},{"line_number":380,"context_line":"        return app"}],"source_content_type":"text/x-python","patch_set":4,"id":"476cb0c5_9b40e0f1","line":377,"in_reply_to":"cd24eb50_22e86316","updated":"2023-06-05 14:14:29.000000000","message":"I didn\u0027t understand why you made a *new* instance of the app?","commit_id":"ba89a93a5e14e58fb6808fc976bcd249eaedbb2b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":364,"context_line":"        ultimate_app._pipeline \u003d pipeline"},{"line_number":365,"context_line":"        ultimate_app._pipeline_final_app \u003d ultimate_app"},{"line_number":366,"context_line":"        app \u003d ultimate_app"},{"line_number":367,"context_line":"        request_logging_app \u003d None"},{"line_number":368,"context_line":"        for filter_app in filters:"},{"line_number":369,"context_line":"            app \u003d filter_app(pipeline[0])"},{"line_number":370,"context_line":"            pipeline.insert(0, app)"}],"source_content_type":"text/x-python","patch_set":6,"id":"32f003d2_7f04881f","line":367,"updated":"2023-06-09 14:01:27.000000000","message":"this could assign to ultimate_app, and the condition at line 371 be\n\n  if request_logging_app is ultimate_app etc.\n  \nthen 376-377 aren\u0027t necessary","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"539879aff0cac9df96c7d93d075e7b5b9f2fed4c","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        ultimate_app._pipeline \u003d pipeline"},{"line_number":365,"context_line":"        ultimate_app._pipeline_final_app \u003d ultimate_app"},{"line_number":366,"context_line":"        app \u003d ultimate_app"},{"line_number":367,"context_line":"        request_logging_app \u003d None"},{"line_number":368,"context_line":"        for filter_app in filters:"},{"line_number":369,"context_line":"            app \u003d filter_app(pipeline[0])"},{"line_number":370,"context_line":"            pipeline.insert(0, app)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f92e043_45770eb9","line":367,"in_reply_to":"32f003d2_7f04881f","updated":"2023-06-12 18:37:02.000000000","message":"Done","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":370,"context_line":"            pipeline.insert(0, app)"},{"line_number":371,"context_line":"            if request_logging_app is None and \\"},{"line_number":372,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":373,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        if request_logging_app is None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"b860e4fb_009d05e3","line":373,"updated":"2023-06-09 14:01:27.000000000","message":"ok, this goes beyond getting us back to logging the subrequests. IIUC, this also adds some defence against other middlewares messing with the subrequest response before the request_logging_app populates infocache.\n\nBut, middleware could still mess with infocache, so does this really provide any guarantee over the mutability of infocache account/container info? Or am I still not getting the point?\n\nThis could move to a follow-on patch.","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"78e743eb25ebcb2451e975c6fe53a42482359d99","unresolved":false,"context_lines":[{"line_number":370,"context_line":"            pipeline.insert(0, app)"},{"line_number":371,"context_line":"            if request_logging_app is None and \\"},{"line_number":372,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":373,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        if request_logging_app is None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"41bc0c96_8484a495","line":373,"in_reply_to":"aa1431e7_fa202e03","updated":"2023-06-23 14:31:17.000000000","message":"Done","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4c6f82d7f58ea943a09a876e76bc09a0190ede03","unresolved":true,"context_lines":[{"line_number":370,"context_line":"            pipeline.insert(0, app)"},{"line_number":371,"context_line":"            if request_logging_app is None and \\"},{"line_number":372,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":373,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":374,"context_line":"            app._pipeline \u003d pipeline"},{"line_number":375,"context_line":"            app._pipeline_final_app \u003d ultimate_app"},{"line_number":376,"context_line":"        if request_logging_app is None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"aa1431e7_fa202e03","line":373,"in_reply_to":"b860e4fb_009d05e3","updated":"2023-06-12 17:53:59.000000000","message":"So basically every time I look at `get_account_info`/`get_container_info` I forget how exactly it works 😞\n\n_Every time_ I expect it to work like it did before https://github.com/openstack/swift/commit/58f4e0f2 :\n\n- Check if we have cached info; if we do, return it\n- Issue a HEAD via `app`; read the response\n- Convert the headers of the response from `app` to an info-style dict\n- Use the dict to set cache and return it\n\nThis is the mental model I keep worrying about, and the worry is that any middleware in between could go tweaking headers and making it so it\u0027s really hard to have consistent account/container info, since you never know which point in the pipeline got to do the conversion when reading from cache.\n\nBut that\u0027s not how it works at all, and in fact, it\u0027s never worked that way the whole time I\u0027ve been working on Swift! For a good long while, it looked like this:\n\n- Check if we have cached info; if we do, return it\n- Issue a HEAD via `app`; this works its way down the pipeline to the proxy-server app\n- Proxy server does its HEAD and reads the response\n- Convert the headers of the response _still in the proxy server_ to an info-style dict; use it to set cache\n- Unwind the stack a while; response gets read back up in `get_*_info`\n- _Most of the time_, we check info cache again, find the dict set by the proxy-server, and return it; discarding the response\n- _If it\u0027s not there_, though, we convert the headers of the response _that has been through the pipeline_ to an info-style dict, then use the dict to try again to set cache, and return it\n\nThat \"set in the proxy server, then (almost) always read out of cache\" gets me every time -- Sam even had some joke in https://github.com/openstack/swift/commit/1c88d2cb about how it got \"deposited by elves or something\"\n\n---\n\nAll of which is to say, I think I\u0027ve been worrying over nothing. It\u0027s probably fine for us to just use the right-most logging middleware; there\u0027s some chance of extra HEAD requests, but that\u0027s only if operators have a middleware right of that that would make info requests.","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":379,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":380,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":381,"context_line":"        for x in pipeline:"},{"line_number":382,"context_line":"            x._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        return app"},{"line_number":384,"context_line":"    return ctx.create()"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"f5c12bbe_a58d2932","line":382,"updated":"2023-06-09 14:01:27.000000000","message":"off-topic: might be nice if app._pipeline pointed to a single object that encapsulates the list of filters, ultimate app and request_logging app - then there would be no need to go back and set an attr on all filters","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"78e743eb25ebcb2451e975c6fe53a42482359d99","unresolved":false,"context_lines":[{"line_number":379,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":380,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":381,"context_line":"        for x in pipeline:"},{"line_number":382,"context_line":"            x._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        return app"},{"line_number":384,"context_line":"    return ctx.create()"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dd833141_0a17d040","line":382,"in_reply_to":"6ae382eb_807bf7fa","updated":"2023-06-23 14:31:17.000000000","message":"Ack","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4c6f82d7f58ea943a09a876e76bc09a0190ede03","unresolved":true,"context_lines":[{"line_number":379,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":380,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":381,"context_line":"        for x in pipeline:"},{"line_number":382,"context_line":"            x._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        return app"},{"line_number":384,"context_line":"    return ctx.create()"},{"line_number":385,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"6ae382eb_807bf7fa","line":382,"in_reply_to":"f5c12bbe_a58d2932","updated":"2023-06-12 17:53:59.000000000","message":"Yeah, I was starting to think about doing it that way, too. I\u0027ll have to be a little cautious, though -- if we\u0027re taking out the dedicated logging pipeline, I start to worry about loop detection...","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"00319689c62f8606a56ec821bda3413f3e3209bd","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            pipeline.insert(0, app)"},{"line_number":368,"context_line":"            if request_logging_app is ultimate_app and \\"},{"line_number":369,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":370,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        for app in pipeline:"},{"line_number":373,"context_line":"            app._pipeline \u003d pipeline"}],"source_content_type":"text/x-python","patch_set":11,"id":"b8eca014_67a910a6","line":370,"updated":"2023-07-13 06:53:54.000000000","message":"So if we ever did decide to put a middleware between the 2nd ProxyLoggingMiddleware and the proxy then this would never be traversed when we use sub requests.\n\nCouldn\u0027t it just be:\n\n  requests_logging_app \u003d app\n  \nAs app at the time of the iteration is the ProxyLoggingMiddleware -\u003e [potential new middleware] -\u003e Proxy, as app is recreated and inserted into pipeline each time. Or is there some benefit for having a new 3rd and unique ProxyLoggingMiddleware object in memory?","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"add6c447d97ac79115239f6a095c53185efdddf3","unresolved":false,"context_lines":[{"line_number":367,"context_line":"            pipeline.insert(0, app)"},{"line_number":368,"context_line":"            if request_logging_app is ultimate_app and \\"},{"line_number":369,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":370,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        for app in pipeline:"},{"line_number":373,"context_line":"            app._pipeline \u003d pipeline"}],"source_content_type":"text/x-python","patch_set":11,"id":"063989a2_9f3d36e9","line":370,"in_reply_to":"44f2cb62_22652083","updated":"2023-08-01 12:13:19.000000000","message":"Done","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"e223ddce90c0e8d8cd50ea69f821c09d7dc6148f","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            pipeline.insert(0, app)"},{"line_number":368,"context_line":"            if request_logging_app is ultimate_app and \\"},{"line_number":369,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":370,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        for app in pipeline:"},{"line_number":373,"context_line":"            app._pipeline \u003d pipeline"}],"source_content_type":"text/x-python","patch_set":11,"id":"dec3fba3_9624f0b3","line":370,"in_reply_to":"6cc2d8ab_de7207ec","updated":"2023-07-14 01:48:56.000000000","message":"You mean the right most proxy-logging (the latest one in the chain and just before the proxy). This reverses the pipeline, so I guess once it\u0027s returned it\u0027s the left most :P\n\nOK, so the worry is something could slip in there and if its a middleware that makes subrequests we could have looping happening. Yeah OK I see that risk. I guess the easist way to mitigate that is to define our own final app which is really just `proxylogging(proxy())` which is what your doing here.. kk. I think I\u0027m on board","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6b813aca6409075a9a9464457d76b580803a88f4","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            pipeline.insert(0, app)"},{"line_number":368,"context_line":"            if request_logging_app is ultimate_app and \\"},{"line_number":369,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":370,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        for app in pipeline:"},{"line_number":373,"context_line":"            app._pipeline \u003d pipeline"}],"source_content_type":"text/x-python","patch_set":11,"id":"bdb50b69_4babf204","line":370,"in_reply_to":"b8eca014_67a910a6","updated":"2023-07-13 18:35:26.000000000","message":"So I think the *idea* of using `_pipeline_final_app` is good -- remove any possibility of some custom middleware interfering with get-info results -- we just need to make sure we actually log the requests.\n\nImagine for a moment that there was something like container-sync or 1space as the middleware in between left-most proxy-logging and the proxy app -- we aren\u0027t even guaranteed that we\u0027ll have _called_ the proxy-app. I have a strong preference for making sure that *all* of the code called as part of `get_account_info` and `get_container_info` is in swift and *only* swift.","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ff217e662feea963e92421d958041442bedb3560","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            pipeline.insert(0, app)"},{"line_number":368,"context_line":"            if request_logging_app is ultimate_app and \\"},{"line_number":369,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":370,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        for app in pipeline:"},{"line_number":373,"context_line":"            app._pipeline \u003d pipeline"}],"source_content_type":"text/x-python","patch_set":11,"id":"6cc2d8ab_de7207ec","line":370,"in_reply_to":"bdb50b69_4babf204","updated":"2023-07-13 18:51:03.000000000","message":"Oh -- also, there\u0027s a risk: if a 3rd party middleware is between the left-most proxy-logging *and* it makes `get_container_info` requests as part of a container GET/HEAD request, or `get_account_info` requests as part of an account GET/HEAD request, we would likely have a loop that would have been fixed by https://review.opendev.org/c/openstack/swift/+/875819","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"55d9d1ee84cca954d5c9c8f745e09c22f12709a8","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            pipeline.insert(0, app)"},{"line_number":368,"context_line":"            if request_logging_app is ultimate_app and \\"},{"line_number":369,"context_line":"                    app.__class__.__name__ \u003d\u003d \u0027ProxyLoggingMiddleware\u0027:"},{"line_number":370,"context_line":"                request_logging_app \u003d filter_app(ultimate_app)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        for app in pipeline:"},{"line_number":373,"context_line":"            app._pipeline \u003d pipeline"}],"source_content_type":"text/x-python","patch_set":11,"id":"44f2cb62_22652083","line":370,"in_reply_to":"dec3fba3_9624f0b3","updated":"2023-07-14 09:00:25.000000000","message":"+1 IIUC the goal here is to deliberately not allow any other middlewares in the request_logging_app pipeline for 2 reasons: 1. to prevent any loops, 2. to avoid middlewares that might mess with the request/response in unforeseen ways","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"78e743eb25ebcb2451e975c6fe53a42482359d99","unresolved":true,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Do it for the logging app, too"},{"line_number":381,"context_line":"        request_logging_app._pipeline \u003d pipeline"},{"line_number":382,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":384,"context_line":"        return pipeline[0]"},{"line_number":385,"context_line":"    return ctx.create()"}],"source_content_type":"text/x-python","patch_set":11,"id":"0ebbe2b0_1ad0838a","line":382,"updated":"2023-06-23 14:31:17.000000000","message":"if request_logging_app._pipeline_request_logging_app were ever used would it not result in a loop? request_logging_app would itself have called get_container/account_info...","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"154450367411f341fc1c991fba5e0ddb2aec4edb","unresolved":true,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Do it for the logging app, too"},{"line_number":381,"context_line":"        request_logging_app._pipeline \u003d pipeline"},{"line_number":382,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":384,"context_line":"        return pipeline[0]"},{"line_number":385,"context_line":"    return ctx.create()"}],"source_content_type":"text/x-python","patch_set":11,"id":"a6b1eafc_f7169874","line":382,"in_reply_to":"0ebbe2b0_1ad0838a","updated":"2023-06-23 23:30:20.000000000","message":"These probably aren\u0027t necessary (who would call `get_container_info` with `self._pipeline_request_logging_app` instead of `self.app`?), but it felt a little weird to *not* include them on the logging app.\n\nYeah, you could probably create something of a loop in the call graph by calling `get_account_info`/`get_container_info` in `ProxyLoggingMiddleware` -- though even that *can* be done safely, for example by only calling `get_container_info` for object requests, or `get_account_info` for object or container requests.\n\nTrouble only really comes if you try to get account info as part of an account HEAD request, or get container info as part of a container HEAD request -- I find it unlikely that we might do that in the future if we\u0027re limiting ourselves to just considering proxy logging and the proxy app. If we **do**, I think I\u0027d rather have it manifest as the `RecursionError` than some unlogged extra request.\n\nFWIW, we *already* have to consider call-graph loops even just in the proxy.","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"55d9d1ee84cca954d5c9c8f745e09c22f12709a8","unresolved":true,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Do it for the logging app, too"},{"line_number":381,"context_line":"        request_logging_app._pipeline \u003d pipeline"},{"line_number":382,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":384,"context_line":"        return pipeline[0]"},{"line_number":385,"context_line":"    return ctx.create()"}],"source_content_type":"text/x-python","patch_set":11,"id":"c75ae7e8_b5084bb6","line":382,"in_reply_to":"7449bb80_6203e237","updated":"2023-07-14 09:00:25.000000000","message":"\u003e Or do we just treat the request_logging_app with it\u0027s own unique (and correct) pipeline.\n\nthat\u0027s a good point IMHO. It might be confusing if we ever did inspect request_logging_app._pipeline to have it give us a false impression of which pipeline it belongs to","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"e223ddce90c0e8d8cd50ea69f821c09d7dc6148f","unresolved":true,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Do it for the logging app, too"},{"line_number":381,"context_line":"        request_logging_app._pipeline \u003d pipeline"},{"line_number":382,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":384,"context_line":"        return pipeline[0]"},{"line_number":385,"context_line":"    return ctx.create()"}],"source_content_type":"text/x-python","patch_set":11,"id":"7449bb80_6203e237","line":382,"in_reply_to":"a6b1eafc_f7169874","updated":"2023-07-14 01:48:56.000000000","message":"Or do we just treat the request_logging_app with it\u0027s own unique (and correct) pipeline. In case we ever want to interrogate it?\n\n    request_logging_app._pipeline \u003d [request_logging_app, ultimate_app]","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4398470b0f7d1cb7fc1671d70a3570068119a15b","unresolved":false,"context_lines":[{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Do it for the logging app, too"},{"line_number":381,"context_line":"        request_logging_app._pipeline \u003d pipeline"},{"line_number":382,"context_line":"        request_logging_app._pipeline_request_logging_app \u003d request_logging_app"},{"line_number":383,"context_line":"        request_logging_app._pipeline_final_app \u003d ultimate_app"},{"line_number":384,"context_line":"        return pipeline[0]"},{"line_number":385,"context_line":"    return ctx.create()"}],"source_content_type":"text/x-python","patch_set":11,"id":"f08b4204_c3143c30","line":382,"in_reply_to":"c75ae7e8_b5084bb6","updated":"2023-07-31 19:51:16.000000000","message":"Yeah, I can get on board with that.","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6451e2bd9889daeafa81561e6ed5b1dfa89e1443","unresolved":true,"context_lines":[{"line_number":444,"context_line":"        app \u003d (app._pipeline[-2]"},{"line_number":445,"context_line":"               if isinstance(app._pipeline[-2],"},{"line_number":446,"context_line":"                             proxy_logging.ProxyLoggingMiddleware)"},{"line_number":447,"context_line":"               and app._pipeline[-2] is not app"},{"line_number":448,"context_line":"               else app._pipeline_final_app)"},{"line_number":449,"context_line":"    except AttributeError:"},{"line_number":450,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":1,"id":"f841ed09_23acd9d3","line":447,"updated":"2023-05-31 19:18:56.000000000","message":"I thought I was being defensive against recursion errors in case `ProxyLoggingMiddleware` ever started calling `get_account_info`/`get_container_info`, but that\u0027s not actually a thing I need to worry about -- it\u0027d pass `self.app`, not `self`.\n\nMeanwhile, as coded, we miss `get_account_info` logging/metrics when it\u0027s done as part of `get_container_info`.","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f91df175dae48f638b4e7bed2581ddacecad4ba4","unresolved":false,"context_lines":[{"line_number":444,"context_line":"        app \u003d (app._pipeline[-2]"},{"line_number":445,"context_line":"               if isinstance(app._pipeline[-2],"},{"line_number":446,"context_line":"                             proxy_logging.ProxyLoggingMiddleware)"},{"line_number":447,"context_line":"               and app._pipeline[-2] is not app"},{"line_number":448,"context_line":"               else app._pipeline_final_app)"},{"line_number":449,"context_line":"    except AttributeError:"},{"line_number":450,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":1,"id":"8408fa60_93451176","line":447,"in_reply_to":"f841ed09_23acd9d3","updated":"2023-06-01 17:26:37.000000000","message":"Done","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c74d8e8114e443afb4bdae82763ef434bd8f07e1","unresolved":true,"context_lines":[{"line_number":543,"context_line":"               if isinstance(app._pipeline[-2],"},{"line_number":544,"context_line":"                             proxy_logging.ProxyLoggingMiddleware)"},{"line_number":545,"context_line":"               and app._pipeline[-2] is not app"},{"line_number":546,"context_line":"               else app._pipeline_final_app)"},{"line_number":547,"context_line":"    except AttributeError:"},{"line_number":548,"context_line":"        pass"},{"line_number":549,"context_line":"    # Check in environment cache and in memcache (in that order)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a22baf80_0a44c530","line":546,"updated":"2023-05-31 20:19:22.000000000","message":"and I guess any app we send down the pipe from here (filter or otherwise) is going to have _pipeline_final_app attr set on it?","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f91df175dae48f638b4e7bed2581ddacecad4ba4","unresolved":true,"context_lines":[{"line_number":543,"context_line":"               if isinstance(app._pipeline[-2],"},{"line_number":544,"context_line":"                             proxy_logging.ProxyLoggingMiddleware)"},{"line_number":545,"context_line":"               and app._pipeline[-2] is not app"},{"line_number":546,"context_line":"               else app._pipeline_final_app)"},{"line_number":547,"context_line":"    except AttributeError:"},{"line_number":548,"context_line":"        pass"},{"line_number":549,"context_line":"    # Check in environment cache and in memcache (in that order)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9b83ddc8_4178afdd","line":546,"in_reply_to":"a22baf80_0a44c530","updated":"2023-06-01 17:26:37.000000000","message":"Not really -- hence the `except AttributeError`. If a 3rd party middleware has its paste entry-point composed of multiple filters (similar to what we do for s3api or encryption), one of those inner filters may get passed and it wouldn\u0027t have had our `loadapp` magic applied.","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"cc5dadb6cac8d5d67db500b00781de46d388f805","unresolved":true,"context_lines":[{"line_number":797,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":798,"context_line":"    if memcache:"},{"line_number":799,"context_line":"        try:"},{"line_number":800,"context_line":"            proxy_app \u003d app._pipeline_final_app"},{"line_number":801,"context_line":"        except AttributeError:"},{"line_number":802,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":803,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"}],"source_content_type":"text/x-python","patch_set":1,"id":"0532d0b6_420b6057","line":800,"updated":"2023-06-08 18:24:21.000000000","message":"how about\u0027_pipeline_final_app\u0027 here, does it also need to be changed to \u0027_pipeline_request_logging_app\u0027?","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e31fcea35951a8e03b3196fc7eaf0659aefd4dd4","unresolved":true,"context_lines":[{"line_number":797,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":798,"context_line":"    if memcache:"},{"line_number":799,"context_line":"        try:"},{"line_number":800,"context_line":"            proxy_app \u003d app._pipeline_final_app"},{"line_number":801,"context_line":"        except AttributeError:"},{"line_number":802,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":803,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"}],"source_content_type":"text/x-python","patch_set":1,"id":"b0f69279_fa3af699","line":800,"in_reply_to":"0532d0b6_420b6057","updated":"2023-06-08 21:19:15.000000000","message":"We definitely want `_pipeline_final_app` for getting `container_existence_skip_cache` / `account_existence_skip_cache`\n\nAs far as the logger used for stats... _maybe_ we want `_pipeline_request_logging_app`? We\u0027d have to grab `_pipeline_request_logging_app.access_logger`, though; `ProxyLoggingMiddleware` instances don\u0027t get logger attributes -- which I suppose is a sign that we\u0027re at least consistent about which app\u0027s logger we\u0027re using, since we\u0027d get `AttributeError`s otherwise.\n\nFWIW, in my SAIO, the parent sha gives me stats like\n```\nproxy-server.container.info.cache.miss:1|c\nproxy-server.account.info.cache.miss:1|c\nproxy-server.object.HEAD.404.timing:59.03267860412598|ms\nproxy-server.object.HEAD.404.xfer:0|c\nproxy-server.object.policy.0.HEAD.404.timing:59.03267860412598|ms\nproxy-server.object.policy.0.HEAD.404.xfer:0|c\n\nproxy-server.container.info.cache.miss:1|c\nproxy-server.account.info.cache.hit:1|c\nproxy-server.object.HEAD.404.timing:20.878076553344727|ms\nproxy-server.object.HEAD.404.xfer:0|c\nproxy-server.object.policy.0.HEAD.404.timing:20.878076553344727|ms\nproxy-server.object.policy.0.HEAD.404.xfer:0|c\n```\nwhile this one gets me\n```\nproxy-server.container.info.cache.miss:1|c\nproxy-server.account.info.cache.miss:1|c\nsubrequest.proxy-server.account.HEAD.204.timing:17.55976676940918|ms\nsubrequest.proxy-server.account.HEAD.204.xfer:0|c\nsubrequest.proxy-server.container.HEAD.404.timing:18.161773681640625|ms\nsubrequest.proxy-server.container.HEAD.404.xfer:0|c\nproxy-server.object.HEAD.404.timing:62.734127044677734|ms\nproxy-server.object.HEAD.404.xfer:0|c\nproxy-server.object.policy.0.HEAD.404.timing:62.734127044677734|ms\nproxy-server.object.policy.0.HEAD.404.xfer:0|c\n\n\nproxy-server.container.info.cache.hit:1|c\nproxy-server.account.info.cache.hit:1|c\nproxy-server.object.HEAD.404.timing:13.357877731323242|ms\nproxy-server.object.HEAD.404.xfer:0|c\nproxy-server.object.policy.0.HEAD.404.timing:13.357877731323242|ms\nproxy-server.object.policy.0.HEAD.404.xfer:0|c\n```\nso the `*.cache.info.*` stats are present both before and after. I also tried hacking up my `memcached_timing_stats` to have `dec_kwargs[\u0027sample_rate\u0027] \u003d 1`; both before and after had stats like `proxy-server.memcached.get.timing`; no `subrequest`","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":797,"context_line":"    memcache \u003d cache_from_env(env, True)"},{"line_number":798,"context_line":"    if memcache:"},{"line_number":799,"context_line":"        try:"},{"line_number":800,"context_line":"            proxy_app \u003d app._pipeline_final_app"},{"line_number":801,"context_line":"        except AttributeError:"},{"line_number":802,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":803,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"}],"source_content_type":"text/x-python","patch_set":1,"id":"d5718974_9884912c","line":800,"in_reply_to":"b0f69279_fa3af699","updated":"2023-06-09 14:01:27.000000000","message":"IIUC this use of proxy_app pre-dated the change that this patch is \"correcting\"","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c74d8e8114e443afb4bdae82763ef434bd8f07e1","unresolved":true,"context_lines":[{"line_number":801,"context_line":"        except AttributeError:"},{"line_number":802,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":803,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"},{"line_number":804,"context_line":"            # filters, we\u0027ll just have to choose a reasonable default"},{"line_number":805,"context_line":"            skip_chance \u003d 0.0"},{"line_number":806,"context_line":"            logger \u003d None"},{"line_number":807,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"34ba35b1_ca54475a","line":804,"updated":"2023-05-31 20:19:22.000000000","message":"does this only come up in tests - or is this going to be the common case now?","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f91df175dae48f638b4e7bed2581ddacecad4ba4","unresolved":true,"context_lines":[{"line_number":801,"context_line":"        except AttributeError:"},{"line_number":802,"context_line":"            # Only the middleware entry-points get a reference to the"},{"line_number":803,"context_line":"            # proxy-server app; if a middleware composes itself as multiple"},{"line_number":804,"context_line":"            # filters, we\u0027ll just have to choose a reasonable default"},{"line_number":805,"context_line":"            skip_chance \u003d 0.0"},{"line_number":806,"context_line":"            logger \u003d None"},{"line_number":807,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"89deaeb2_3c1d794c","line":804,"in_reply_to":"34ba35b1_ca54475a","updated":"2023-06-01 17:26:37.000000000","message":"No, this would still seem \"very unlikely\", though not entirely *unexpected*.","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c74d8e8114e443afb4bdae82763ef434bd8f07e1","unresolved":true,"context_lines":[{"line_number":819,"context_line":"            info \u003d memcache.get(cache_key)"},{"line_number":820,"context_line":"            if logger:"},{"line_number":821,"context_line":"                logger.increment(\u0027%s.info.cache.%s\u0027 % ("},{"line_number":822,"context_line":"                    info_type, \u0027hit\u0027 if info else \u0027miss\u0027))"},{"line_number":823,"context_line":"        if info and six.PY2:"},{"line_number":824,"context_line":"            # Get back to native strings"},{"line_number":825,"context_line":"            new_info \u003d {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"b606a783_58d2634b","line":822,"updated":"2023-05-31 20:19:22.000000000","message":"so if we have a logger we should be getting [account|container].info.cache.[hit|miss] already","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f91df175dae48f638b4e7bed2581ddacecad4ba4","unresolved":true,"context_lines":[{"line_number":819,"context_line":"            info \u003d memcache.get(cache_key)"},{"line_number":820,"context_line":"            if logger:"},{"line_number":821,"context_line":"                logger.increment(\u0027%s.info.cache.%s\u0027 % ("},{"line_number":822,"context_line":"                    info_type, \u0027hit\u0027 if info else \u0027miss\u0027))"},{"line_number":823,"context_line":"        if info and six.PY2:"},{"line_number":824,"context_line":"            # Get back to native strings"},{"line_number":825,"context_line":"            new_info \u003d {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"a8cbbdd3_a57291d2","line":822,"in_reply_to":"b606a783_58d2634b","updated":"2023-06-01 17:26:37.000000000","message":"Yes -- the trouble is that on a miss, we don\u0027t currently get the logging \u0026 metrics from proxy_logging for the subsequent HEAD (though we *used to*, a couple minor releases or so ago).","commit_id":"ecbf1b82c96bca2485276c5c7f54ee43af54ee04"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c74d8e8114e443afb4bdae82763ef434bd8f07e1","unresolved":true,"context_lines":[{"line_number":444,"context_line":"        app \u003d (app._pipeline[-2]"},{"line_number":445,"context_line":"               if isinstance(app._pipeline[-2],"},{"line_number":446,"context_line":"                             proxy_logging.ProxyLoggingMiddleware)"},{"line_number":447,"context_line":"               else app._pipeline_final_app)"},{"line_number":448,"context_line":"    except AttributeError:"},{"line_number":449,"context_line":"        pass"},{"line_number":450,"context_line":"    # Check in environment cache and in memcache (in that order)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ccb7dd5_d4cccaae","line":447,"updated":"2023-05-31 20:19:22.000000000","message":"i know it\u0027s \"only\" a ternary condition but the fact that it takes four lines, and calls another function and duplicates the \"magic\" -2 index makes it sufficiently ugly enough to stink having to spell it out in two places.\n\nDo you think it\u0027d be better as an extracted function with a unrolled if branch?","commit_id":"cbd8c41b273d92806f2757163b05ec6f8446ce59"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"539879aff0cac9df96c7d93d075e7b5b9f2fed4c","unresolved":false,"context_lines":[{"line_number":444,"context_line":"        app \u003d (app._pipeline[-2]"},{"line_number":445,"context_line":"               if isinstance(app._pipeline[-2],"},{"line_number":446,"context_line":"                             proxy_logging.ProxyLoggingMiddleware)"},{"line_number":447,"context_line":"               else app._pipeline_final_app)"},{"line_number":448,"context_line":"    except AttributeError:"},{"line_number":449,"context_line":"        pass"},{"line_number":450,"context_line":"    # Check in environment cache and in memcache (in that order)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0dadf88b_8030f20f","line":447,"in_reply_to":"2faed9ae_bfde8544","updated":"2023-06-12 18:37:02.000000000","message":"Done","commit_id":"cbd8c41b273d92806f2757163b05ec6f8446ce59"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f91df175dae48f638b4e7bed2581ddacecad4ba4","unresolved":true,"context_lines":[{"line_number":444,"context_line":"        app \u003d (app._pipeline[-2]"},{"line_number":445,"context_line":"               if isinstance(app._pipeline[-2],"},{"line_number":446,"context_line":"                             proxy_logging.ProxyLoggingMiddleware)"},{"line_number":447,"context_line":"               else app._pipeline_final_app)"},{"line_number":448,"context_line":"    except AttributeError:"},{"line_number":449,"context_line":"        pass"},{"line_number":450,"context_line":"    # Check in environment cache and in memcache (in that order)"}],"source_content_type":"text/x-python","patch_set":2,"id":"2faed9ae_bfde8544","line":447,"in_reply_to":"7ccb7dd5_d4cccaae","updated":"2023-06-01 17:26:37.000000000","message":"I\u0027m also debating about having `loadapp` set another attr -- maybe something like `_pipeline_logged_app`?","commit_id":"cbd8c41b273d92806f2757163b05ec6f8446ce59"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e31fcea35951a8e03b3196fc7eaf0659aefd4dd4","unresolved":true,"context_lines":[{"line_number":456,"context_line":"        # account is successful whether the account actually has .db files"},{"line_number":457,"context_line":"        # on disk or not."},{"line_number":458,"context_line":"        is_autocreate_account \u003d account.startswith("},{"line_number":459,"context_line":"            getattr(app, \u0027auto_create_account_prefix\u0027,"},{"line_number":460,"context_line":"                    constraints.AUTO_CREATE_ACCOUNT_PREFIX))"},{"line_number":461,"context_line":"        if not is_autocreate_account:"},{"line_number":462,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"}],"source_content_type":"text/x-python","patch_set":6,"id":"b95bcfae_0f78e066","line":459,"updated":"2023-06-08 21:19:15.000000000","message":"Oh, but _this_ bit should have the actual proxy app...","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"539879aff0cac9df96c7d93d075e7b5b9f2fed4c","unresolved":false,"context_lines":[{"line_number":456,"context_line":"        # account is successful whether the account actually has .db files"},{"line_number":457,"context_line":"        # on disk or not."},{"line_number":458,"context_line":"        is_autocreate_account \u003d account.startswith("},{"line_number":459,"context_line":"            getattr(app, \u0027auto_create_account_prefix\u0027,"},{"line_number":460,"context_line":"                    constraints.AUTO_CREATE_ACCOUNT_PREFIX))"},{"line_number":461,"context_line":"        if not is_autocreate_account:"},{"line_number":462,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"}],"source_content_type":"text/x-python","patch_set":6,"id":"659ddce1_e4cf64a5","line":459,"in_reply_to":"41c728d9_d8f9a297","updated":"2023-06-12 18:37:02.000000000","message":"Done","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"416422d6f3810778a16b30429b7df47d00233d5f","unresolved":true,"context_lines":[{"line_number":456,"context_line":"        # account is successful whether the account actually has .db files"},{"line_number":457,"context_line":"        # on disk or not."},{"line_number":458,"context_line":"        is_autocreate_account \u003d account.startswith("},{"line_number":459,"context_line":"            getattr(app, \u0027auto_create_account_prefix\u0027,"},{"line_number":460,"context_line":"                    constraints.AUTO_CREATE_ACCOUNT_PREFIX))"},{"line_number":461,"context_line":"        if not is_autocreate_account:"},{"line_number":462,"context_line":"            account_info \u003d get_account_info(env, app, swift_source)"}],"source_content_type":"text/x-python","patch_set":6,"id":"41c728d9_d8f9a297","line":459,"in_reply_to":"b95bcfae_0f78e066","updated":"2023-06-09 14:01:27.000000000","message":"OK, so this was broken historically, but works on master IIUC? So is this patch is now a regression?\n\nI was just thinking that we may help ourselves but using an alternative var name for the \"app\" to which we delegate info lookup! (like in _get_info_from_caches, we have proxy_app) \n\nIDK what a good name would be though? info_lookup_app, info_app","commit_id":"ff478f78f324691f8129f480f7df1e2a257ee7a0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"add6c447d97ac79115239f6a095c53185efdddf3","unresolved":true,"context_lines":[{"line_number":534,"context_line":""},{"line_number":535,"context_line":"    # Try to cut through all the layers to the proxy app"},{"line_number":536,"context_line":"    # (while also preserving logging)"},{"line_number":537,"context_line":"    try:"},{"line_number":538,"context_line":"        app \u003d app._pipeline_request_logging_app"},{"line_number":539,"context_line":"    except AttributeError:"},{"line_number":540,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":15,"id":"a4f00648_0c954172","line":537,"updated":"2023-08-01 12:13:19.000000000","message":"could we make this mirror line 442...\n\ni.e. `logged_app \u003d app._pipeline_request_logging_app`","commit_id":"6d82511e3fc81577bf7ef3eb812ee0ace820b2d0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"add6c447d97ac79115239f6a095c53185efdddf3","unresolved":true,"context_lines":[{"line_number":539,"context_line":"    except AttributeError:"},{"line_number":540,"context_line":"        pass"},{"line_number":541,"context_line":"    # Check in environment cache and in memcache (in that order)"},{"line_number":542,"context_line":"    info \u003d _get_info_from_caches(app, env, account)"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"    # Cache miss; go HEAD the account and populate the caches"},{"line_number":545,"context_line":"    if not info:"}],"source_content_type":"text/x-python","patch_set":15,"id":"40bb2469_67a6d9ce","line":542,"range":{"start_line":542,"start_character":32,"end_line":542,"end_character":36},"updated":"2023-08-01 12:13:19.000000000","message":"for container info we now use proxy_app, but here we use app._pipeline_request_logging_app ?? It makes no fucntional difference because _get_info_from_caches ends up using app._pipeline_final_app, but it would be less confusing for the two methods to be similar","commit_id":"6d82511e3fc81577bf7ef3eb812ee0ace820b2d0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"add6c447d97ac79115239f6a095c53185efdddf3","unresolved":true,"context_lines":[{"line_number":550,"context_line":"        # *Always* allow reserved names for get-info requests -- it\u0027s on the"},{"line_number":551,"context_line":"        # caller to keep the result private-ish"},{"line_number":552,"context_line":"        req.headers[\u0027X-Backend-Allow-Reserved-Names\u0027] \u003d \u0027true\u0027"},{"line_number":553,"context_line":"        resp \u003d req.get_response(app)"},{"line_number":554,"context_line":"        drain_and_close(resp)"},{"line_number":555,"context_line":"        # Check in infocache to see if the proxy (or anyone else) already"},{"line_number":556,"context_line":"        # populated the cache for us. If they did, just use what\u0027s there."}],"source_content_type":"text/x-python","patch_set":15,"id":"d30039ed_87ef8c68","line":553,"range":{"start_line":553,"start_character":32,"end_line":553,"end_character":35},"updated":"2023-08-01 12:13:19.000000000","message":"ok, this is app._pipeline_request_logging_app but might be clearer to use a \u0027logged_app\u0027 var","commit_id":"6d82511e3fc81577bf7ef3eb812ee0ace820b2d0"}],"test/unit/common/test_wsgi.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"472ab9be9e1a1c5e716deb741756af6ef5c0dfad","unresolved":true,"context_lines":[{"line_number":1889,"context_line":"            # filters to left of proxy-logging point to proxy-logging"},{"line_number":1890,"context_line":"            self.assertIs(app.app.app, app._pipeline_request_logging_app)"},{"line_number":1891,"context_line":"            self.assertIs(app.app.app, app.app._pipeline_request_logging_app)"},{"line_number":1892,"context_line":"            # other filters point to proxy-server"},{"line_number":1893,"context_line":"            self.assertIs(app._pipeline_final_app,"},{"line_number":1894,"context_line":"                          app.app.app._pipeline_request_logging_app)"},{"line_number":1895,"context_line":"            self.assertIs(app._pipeline_final_app,"}],"source_content_type":"text/x-python","patch_set":8,"id":"41605984_b5bea76b","line":1892,"updated":"2023-06-14 11:52:29.000000000","message":"proxy-logging doesn\u0027t use itself ?","commit_id":"6481cb4ce9e18be7bd279ab50eadfa76863283ac"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"55d9d1ee84cca954d5c9c8f745e09c22f12709a8","unresolved":false,"context_lines":[{"line_number":1889,"context_line":"            # filters to left of proxy-logging point to proxy-logging"},{"line_number":1890,"context_line":"            self.assertIs(app.app.app, app._pipeline_request_logging_app)"},{"line_number":1891,"context_line":"            self.assertIs(app.app.app, app.app._pipeline_request_logging_app)"},{"line_number":1892,"context_line":"            # other filters point to proxy-server"},{"line_number":1893,"context_line":"            self.assertIs(app._pipeline_final_app,"},{"line_number":1894,"context_line":"                          app.app.app._pipeline_request_logging_app)"},{"line_number":1895,"context_line":"            self.assertIs(app._pipeline_final_app,"}],"source_content_type":"text/x-python","patch_set":8,"id":"48c5d527_a0b6674a","line":1892,"in_reply_to":"10cb9ffd_8735c2ac","updated":"2023-07-14 09:00:25.000000000","message":"Done","commit_id":"6481cb4ce9e18be7bd279ab50eadfa76863283ac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"57fb031e78b0c9d4d37e7cff5ee99acbacf8508d","unresolved":true,"context_lines":[{"line_number":1889,"context_line":"            # filters to left of proxy-logging point to proxy-logging"},{"line_number":1890,"context_line":"            self.assertIs(app.app.app, app._pipeline_request_logging_app)"},{"line_number":1891,"context_line":"            self.assertIs(app.app.app, app.app._pipeline_request_logging_app)"},{"line_number":1892,"context_line":"            # other filters point to proxy-server"},{"line_number":1893,"context_line":"            self.assertIs(app._pipeline_final_app,"},{"line_number":1894,"context_line":"                          app.app.app._pipeline_request_logging_app)"},{"line_number":1895,"context_line":"            self.assertIs(app._pipeline_final_app,"}],"source_content_type":"text/x-python","patch_set":8,"id":"10cb9ffd_8735c2ac","line":1892,"in_reply_to":"41605984_b5bea76b","updated":"2023-06-14 18:42:00.000000000","message":"Pretty sure it\u0027s moot since proxy-logging doesn\u0027t use `get_account_info` / `get_container_info` -- but if it ever *did*, having it use itself would likely lead to a `RecursionError`.","commit_id":"6481cb4ce9e18be7bd279ab50eadfa76863283ac"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"472ab9be9e1a1c5e716deb741756af6ef5c0dfad","unresolved":true,"context_lines":[{"line_number":1902,"context_line":"            self.assertIs("},{"line_number":1903,"context_line":"                app._pipeline_final_app,"},{"line_number":1904,"context_line":"                app.app.app.app.app.app.app._pipeline_request_logging_app)"},{"line_number":1905,"context_line":"            # utlimate app does not have _pipeline_request_logging_app"},{"line_number":1906,"context_line":""},{"line_number":1907,"context_line":"    def test_proxy_unmodified_wsgi_pipeline(self):"},{"line_number":1908,"context_line":"        # Make sure things are sane even when we modify nothing"}],"source_content_type":"text/x-python","patch_set":8,"id":"12058020_e139bf1f","line":1905,"updated":"2023-06-14 11:52:29.000000000","message":"but ultimate app *does* point to _pipeline_final_app (itself)?","commit_id":"6481cb4ce9e18be7bd279ab50eadfa76863283ac"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"57fb031e78b0c9d4d37e7cff5ee99acbacf8508d","unresolved":true,"context_lines":[{"line_number":1902,"context_line":"            self.assertIs("},{"line_number":1903,"context_line":"                app._pipeline_final_app,"},{"line_number":1904,"context_line":"                app.app.app.app.app.app.app._pipeline_request_logging_app)"},{"line_number":1905,"context_line":"            # utlimate app does not have _pipeline_request_logging_app"},{"line_number":1906,"context_line":""},{"line_number":1907,"context_line":"    def test_proxy_unmodified_wsgi_pipeline(self):"},{"line_number":1908,"context_line":"        # Make sure things are sane even when we modify nothing"}],"source_content_type":"text/x-python","patch_set":8,"id":"1bac092e_ae65ad91","line":1905,"in_reply_to":"12058020_e139bf1f","updated":"2023-06-14 18:42:00.000000000","message":"Yeah, not having a `_pipeline_request_logging_app` at all seems like a bug.\n\nThough, if you\u0027ve got a minimal pipeline or for some other reason have the first `get_container_info` call not trip until you get into the proxy app, having it point to itself will still have us miss the logging and metrics :-/\n\nSee, this was part of why I wanted to have the little stub logging-enabled pipeline ;-)","commit_id":"6481cb4ce9e18be7bd279ab50eadfa76863283ac"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"55d9d1ee84cca954d5c9c8f745e09c22f12709a8","unresolved":false,"context_lines":[{"line_number":1902,"context_line":"            self.assertIs("},{"line_number":1903,"context_line":"                app._pipeline_final_app,"},{"line_number":1904,"context_line":"                app.app.app.app.app.app.app._pipeline_request_logging_app)"},{"line_number":1905,"context_line":"            # utlimate app does not have _pipeline_request_logging_app"},{"line_number":1906,"context_line":""},{"line_number":1907,"context_line":"    def test_proxy_unmodified_wsgi_pipeline(self):"},{"line_number":1908,"context_line":"        # Make sure things are sane even when we modify nothing"}],"source_content_type":"text/x-python","patch_set":8,"id":"17dcba57_7ebfddaa","line":1905,"in_reply_to":"1bac092e_ae65ad91","updated":"2023-07-14 09:00:25.000000000","message":"Done","commit_id":"6481cb4ce9e18be7bd279ab50eadfa76863283ac"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"78e743eb25ebcb2451e975c6fe53a42482359d99","unresolved":true,"context_lines":[{"line_number":1896,"context_line":"            self.assertEqual(self.pipeline_modules(logging_app), ["},{"line_number":1897,"context_line":"                \u0027swift.common.middleware.proxy_logging\u0027,"},{"line_number":1898,"context_line":"                \u0027swift.proxy.server\u0027])"},{"line_number":1899,"context_line":"            self.assertNotIn(logging_app, pipeline)"},{"line_number":1900,"context_line":"            self.assertIs(logging_app.app, final_app)"},{"line_number":1901,"context_line":""},{"line_number":1902,"context_line":"            # All the apps in the main pipeline got decorated identically"}],"source_content_type":"text/x-python","patch_set":11,"id":"a4d6d52e_4dafd7bb","line":1899,"updated":"2023-06-23 14:31:17.000000000","message":"ok, this checks it\u0027s a different instance","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"55d9d1ee84cca954d5c9c8f745e09c22f12709a8","unresolved":false,"context_lines":[{"line_number":1896,"context_line":"            self.assertEqual(self.pipeline_modules(logging_app), ["},{"line_number":1897,"context_line":"                \u0027swift.common.middleware.proxy_logging\u0027,"},{"line_number":1898,"context_line":"                \u0027swift.proxy.server\u0027])"},{"line_number":1899,"context_line":"            self.assertNotIn(logging_app, pipeline)"},{"line_number":1900,"context_line":"            self.assertIs(logging_app.app, final_app)"},{"line_number":1901,"context_line":""},{"line_number":1902,"context_line":"            # All the apps in the main pipeline got decorated identically"}],"source_content_type":"text/x-python","patch_set":11,"id":"909a63bc_8365a85c","line":1899,"in_reply_to":"a4d6d52e_4dafd7bb","updated":"2023-07-14 09:00:25.000000000","message":"Ack","commit_id":"1e9b477eadd87eebc6ef87e9f6fa86ee92821d1d"}]}
