)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b0a325bcb5109dc7bab6924bf87770cb092ea32e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"77b9079f_61bc23d1","updated":"2021-11-29 17:31:50.000000000","message":"I have been reminded of some of the problems with pipeline property\n\nhttps://review.opendev.org/c/openstack/swift/+/104926\n\nwe should reconsider \"just\" using an `x-backend-user-agent` header set on all requests in the internal_client which the proxy app will look for and copy over in it\u0027s backend request helper.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49cbb3e39ced407095835e422842a5697a888de4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2225d121_ef48fbc7","updated":"2021-11-29 16:44:07.000000000","message":"I like how the proxy app has defined a \"backend_user_agent\" attribute that can be modified which it will use instead of forcing it to the getpid value\n\nI think it would be cleaner to have the app default to the getpid value during worker init and then always use whatever backend_user_agent is set to at request time\n\nI see two options how the internal client should override the app\u0027s backend_user_agent\n\n1) use something like a pipeline property to find and set the proxy-app\u0027s backend_user_agent attribute\n2) allow backend_user_agent to be set via config\n3) something else?\n\nmaybe instead of pipeline property we should admit internal_client should always have direct explicit access to the instance of the proxy-app at the bottom of the pipeline and re-work swift.common.wsgi so that can bubble up out of something like:\n\nself.app, self.proxy \u003d loadapp_and_find_proxy()","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1496be4653db664cbd536942dc384dd360b43e6f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"139ebdbb_62af53bb","updated":"2021-11-29 08:51:30.000000000","message":"thanks Matt\n\n-1 for the lost user-agent in original client request headers, as discussed","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"d6924bc29f5be3390c2448e9f231213c137dc03d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"69996f88_39740f36","in_reply_to":"77b9079f_61bc23d1","updated":"2021-11-29 22:50:25.000000000","message":"Maybe, but 1. We are already using pipeline_property for InternalClient, so it\u0027s something we already are living with, so not changing any current risk levels.\n\nAnd 2. always adding another backend-header seems like adding more to an internal rest api, sounds like an easy solution. But we\u0027re passing data in a request in the form of a header, that never indends to be sent out.. seems dirty.\n\nBut we have a mechanism that we already use 3 other times in the same class.. that sure has some brittleness (that we can fix in the future), that does exactly what we want.. talk/use a function further down the pipeline chain.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f4524575_cf0bbd2a","updated":"2022-01-11 01:21:31.000000000","message":"Thank you for the rebase!  I think the _final_pipeline_app is a good option compared to pipeline_property\n\nYou\u0027ve made me reconsider my approach in unifying FakeInternalClient [1] - having a *real* InternalClient.__init__ that is passed in an app that\u0027s an instance of middleware_helpers.FakeSwift might be awesome!\n\nI think the FakeApp and the user_agent property vs. app[._final_pipeline_app].backend_user_agent attribute both need some more justification - all other things equal less code (or more obvious code) is perferred.\n\n1. https://review.opendev.org/c/openstack/swift/+/800701/8/test/unit/__init__.py#1016\n\n","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d89eb4a47b9789188c9b40bd7a8d89321b0e5557","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"c3556556_976275ef","updated":"2022-01-21 07:27:12.000000000","message":"I am not a big fan of screwing with members post-factum. But sadly, there aren\u0027t any arguments we can easily pass to proxy\u0027s Application(). The conf is loaded from the file, and that\u0027s it.\n\nSo, how about this alternative:\nhttps://review.opendev.org/c/openstack/swift/+/825713","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1fed6629_72f60069","updated":"2022-01-24 15:58:57.000000000","message":"I\u0027m really happy with this change overall - I don\u0027t think I understand some of the reservations that have been presented, and I don\u0027t want to minimize anyone\u0027s legitimate concerns...\n\n... but I think the only thing we need to do before we merge is shore up the unittests.  We\u0027re connecting two modules (proxy\u0027s backend make-request, and internal client __init__) through an interface and we need to check BOTH sides (even if the proxy side is simple and obvious)\n\nFor bonus points we could try and find one of the tests in proxy.test_server that goes all the way from real proxy app to in-memory backend processes; but I\u0027d be happy to just unittest generate headers uses the app\u0027s backend_user_agent attribute instead of the fixed os.getpid value","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"c33384dac1082f79f71ae83562e44b8e45717785","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"eb1c9211_70994ad9","updated":"2022-01-21 16:20:09.000000000","message":"Oops, I forgot that we now have global_conf. Of course it\u0027s much better to use that. Add something like \"user_agent\" in there.","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ecba7097df32a8a38d6b77dc437a89b2e71c04e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"35842b56_5cc672fa","updated":"2022-01-21 01:21:37.000000000","message":"recehck","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"efd21e14763fa51ae69065f5a2e4f765f4a3b9eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"82d05ae0_d0e53851","updated":"2022-01-28 15:41:45.000000000","message":"\u003e test/unit/common/test_internal_client.py:1515:80: E501 line too long (89 \u003e 79 characters)\n","commit_id":"bebf46c7115027fcba8be07c68bebc0409093d2c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9e9ba75eac10158ad8b4694ba457bb49bcaf9536","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"2f2310c6_3be13a19","updated":"2022-02-16 01:03:57.000000000","message":"LGTM. I think I heard Alistair voice a concern at one point about this seeming at odds with the recent change for us to stop mucking with statsd prefixes -- but I think it\u0027s probably fine. The _property seems warning enough to keep us from mutating the backend user-agent mid-request-handling like we were doing with the statsd prefix. *Maybe* it\u0027d be even better if we were twiddling self.app._pipeline_final_app._backend_user_agent?\n\nI could imagine a reasonable way to get this plumbed in via global_conf, similar to what we did for log_name, but I *can\u0027t* think of a good reason that an *operator* would ever want to configure it, so it feels like a bit of an abuse.","commit_id":"b0245f4eb5e625a0528e8ce607b1d604201bc6da"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9438c9291fa2b85c9061ff76276a774a5a73667e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"9ad2a640_78c42fa4","updated":"2022-02-14 18:27:35.000000000","message":"everybody loves it!","commit_id":"b0245f4eb5e625a0528e8ce607b1d604201bc6da"}],"swift/common/internal_client.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1496be4653db664cbd536942dc384dd360b43e6f","unresolved":true,"context_lines":[{"line_number":186,"context_line":"        \"\"\""},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        headers \u003d dict(headers)"},{"line_number":189,"context_line":"        headers[\u0027user-agent\u0027] \u003d self.user_agent"},{"line_number":190,"context_line":"        headers.setdefault(\u0027x-backend-allow-reserved-names\u0027, \u0027true\u0027)"},{"line_number":191,"context_line":"        if self.use_replication_network:"},{"line_number":192,"context_line":"            headers.setdefault(USE_REPLICATION_NETWORK_HEADER, \u0027true\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"468d2e80_b33dcfd8","side":"PARENT","line":189,"updated":"2021-11-29 08:51:30.000000000","message":"this should probably remain, otherwise any middleware between client and proxy no longer has access to the user-agent value","commit_id":"092d409c4bbe542f6b9f3c217aa05ffd8669c815"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49cbb3e39ced407095835e422842a5697a888de4","unresolved":true,"context_lines":[{"line_number":154,"context_line":"        self._user_agent \u003d None"},{"line_number":155,"context_line":"        self.app \u003d loadapp(conf_path,"},{"line_number":156,"context_line":"                           allow_modify_pipeline\u003dallow_modify_pipeline)"},{"line_number":157,"context_line":"        self.user_agent \u003d user_agent"},{"line_number":158,"context_line":"        self.request_tries \u003d request_tries"},{"line_number":159,"context_line":"        self.use_replication_network \u003d use_replication_network"},{"line_number":160,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bca697c1_a262cb98","line":157,"updated":"2021-11-29 16:44:07.000000000","message":"to me the difference between _user_agent and user_agent attributes is not obvious","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a1108a823ebadcef4befbae920fe2ac00d5c2d9","unresolved":true,"context_lines":[{"line_number":154,"context_line":"        self._user_agent \u003d None"},{"line_number":155,"context_line":"        self.app \u003d loadapp(conf_path,"},{"line_number":156,"context_line":"                           allow_modify_pipeline\u003dallow_modify_pipeline)"},{"line_number":157,"context_line":"        self.user_agent \u003d user_agent"},{"line_number":158,"context_line":"        self.request_tries \u003d request_tries"},{"line_number":159,"context_line":"        self.use_replication_network \u003d use_replication_network"},{"line_number":160,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d04bfa2c_9c6d0450","line":157,"in_reply_to":"bca697c1_a262cb98","updated":"2021-11-29 22:31:16.000000000","message":"_user_agent is the private instance variable storing the value. And self.user_agent is a propety on the class (so uses of the InternalClient have the same API). using the property to set `self.user_agent \u003d user_agent` will go set the private instance variable _and_ go and set the value down in the proxy.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49cbb3e39ced407095835e422842a5697a888de4","unresolved":true,"context_lines":[{"line_number":163,"context_line":"    account_ring \u003d pipeline_property(\u0027account_ring\u0027)"},{"line_number":164,"context_line":"    auto_create_account_prefix \u003d pipeline_property("},{"line_number":165,"context_line":"        \u0027auto_create_account_prefix\u0027, default\u003dAUTO_CREATE_ACCOUNT_PREFIX)"},{"line_number":166,"context_line":"    set_backend_user_agent \u003d pipeline_property(\u0027set_backend_user_agent\u0027)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    @property"},{"line_number":169,"context_line":"    def user_agent(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3275a6a8_cd708be8","line":166,"updated":"2021-11-29 16:44:07.000000000","message":"I think we\u0027ve mostly used pipeline_property (which I think Tim has pointed out is kind of gross and brittle) mostly for accessing attributes - not methods.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a1108a823ebadcef4befbae920fe2ac00d5c2d9","unresolved":true,"context_lines":[{"line_number":163,"context_line":"    account_ring \u003d pipeline_property(\u0027account_ring\u0027)"},{"line_number":164,"context_line":"    auto_create_account_prefix \u003d pipeline_property("},{"line_number":165,"context_line":"        \u0027auto_create_account_prefix\u0027, default\u003dAUTO_CREATE_ACCOUNT_PREFIX)"},{"line_number":166,"context_line":"    set_backend_user_agent \u003d pipeline_property(\u0027set_backend_user_agent\u0027)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    @property"},{"line_number":169,"context_line":"    def user_agent(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8eb9540f_a6ee04e9","line":166,"in_reply_to":"3275a6a8_cd708be8","updated":"2021-11-29 22:31:16.000000000","message":"What about get_object_ring on line 161? That\u0027s a method. It just ends up being a local method that proxies down to the application/proxy sever class.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49cbb3e39ced407095835e422842a5697a888de4","unresolved":true,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    @property"},{"line_number":169,"context_line":"    def user_agent(self):"},{"line_number":170,"context_line":"        return self._user_agent"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    @user_agent.setter"},{"line_number":173,"context_line":"    def user_agent(self, value):"}],"source_content_type":"text/x-python","patch_set":2,"id":"105d654c_2f41df96","line":170,"updated":"2021-11-29 16:44:07.000000000","message":"it\u0027s confusing to define a property who\u0027s name shadows and existing attribute","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a1108a823ebadcef4befbae920fe2ac00d5c2d9","unresolved":true,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    @property"},{"line_number":169,"context_line":"    def user_agent(self):"},{"line_number":170,"context_line":"        return self._user_agent"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    @user_agent.setter"},{"line_number":173,"context_line":"    def user_agent(self, value):"}],"source_content_type":"text/x-python","patch_set":2,"id":"30b9cead_c3f6e87f","line":170,"in_reply_to":"105d654c_2f41df96","updated":"2021-11-29 22:31:16.000000000","message":"It doesn\u0027t shadow it anymore, the existing attribute is now self._user_agent.\n\nIt\u0027s using the same name, as it\u0027s a property. So accessing it is the same as accessing an attribute as before:\n\n self.user_agent \u003d \"fred\"\n\nwill set self._user_agent to equal fred. But also allows us to call set_backend_user_agent at the same time. Meaning we don\u0027t change any of the InteralClient API for anything using it.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":163,"context_line":"            raise ValueError(\u0027request_tries must be positive\u0027)"},{"line_number":164,"context_line":"        self.app \u003d loadapp(conf_path, global_conf\u003dglobal_conf,"},{"line_number":165,"context_line":"                           allow_modify_pipeline\u003dallow_modify_pipeline,)"},{"line_number":166,"context_line":"        self.user_agent \u003d user_agent"},{"line_number":167,"context_line":"        self.request_tries \u003d request_tries"},{"line_number":168,"context_line":"        self.use_replication_network \u003d use_replication_network"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"025b263c_f5110ebb","line":166,"updated":"2022-01-11 01:21:31.000000000","message":"I think I\u0027d find this change way more obvious if instead of the property stuff we just\n\n   self.user_agent \u003d self.app._pipeline_final_app.backend_user_agent \u003d user_agent","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f892f3b97a57f8ef2001879b7b695970541d1d63","unresolved":true,"context_lines":[{"line_number":163,"context_line":"            raise ValueError(\u0027request_tries must be positive\u0027)"},{"line_number":164,"context_line":"        self.app \u003d loadapp(conf_path, global_conf\u003dglobal_conf,"},{"line_number":165,"context_line":"                           allow_modify_pipeline\u003dallow_modify_pipeline,)"},{"line_number":166,"context_line":"        self.user_agent \u003d user_agent"},{"line_number":167,"context_line":"        self.request_tries \u003d request_tries"},{"line_number":168,"context_line":"        self.use_replication_network \u003d use_replication_network"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"e8fa901d_9118109a","line":166,"in_reply_to":"025b263c_f5110ebb","updated":"2022-01-12 06:00:30.000000000","message":"I did have this for a while, but keeping it as a property meant same old internal client API and so didn\u0027t change the tests so much. Will change it though.\n\nDone","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":156,"context_line":"        if request_tries \u003c 1:"},{"line_number":157,"context_line":"            raise ValueError(\u0027request_tries must be positive\u0027)"},{"line_number":158,"context_line":"        self.app \u003d app or loadapp(conf_path, global_conf\u003dglobal_conf,"},{"line_number":159,"context_line":"                                  allow_modify_pipeline\u003dallow_modify_pipeline,)"},{"line_number":160,"context_line":"        self.user_agent \u003d \\"},{"line_number":161,"context_line":"            self.app._pipeline_final_app.backend_user_agent \u003d user_agent"},{"line_number":162,"context_line":"        self.request_tries \u003d request_tries"}],"source_content_type":"text/x-python","patch_set":8,"id":"e33db564_ee4e54bf","line":159,"updated":"2022-01-24 15:58:57.000000000","message":"i didn\u0027t realized we\u0027d merged this `app or loadapp` change into ic already - this is great!","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a486540eedc0ad33f9936ddb309f29577713a079","unresolved":true,"context_lines":[{"line_number":158,"context_line":"        self.app \u003d app or loadapp(conf_path, global_conf\u003dglobal_conf,"},{"line_number":159,"context_line":"                                  allow_modify_pipeline\u003dallow_modify_pipeline,)"},{"line_number":160,"context_line":"        self.user_agent \u003d \\"},{"line_number":161,"context_line":"            self.app._pipeline_final_app.backend_user_agent \u003d user_agent"},{"line_number":162,"context_line":"        self.request_tries \u003d request_tries"},{"line_number":163,"context_line":"        self.use_replication_network \u003d use_replication_network"},{"line_number":164,"context_line":"        self.get_object_ring \u003d self.app._pipeline_final_app.get_object_ring"}],"source_content_type":"text/x-python","patch_set":8,"id":"2b4c4153_3afae527","line":161,"updated":"2022-01-21 13:27:14.000000000","message":"I wonder if we will regret that having taken the trouble to make the internal client user-agent distinct from the proxy, we now have no distinction between the client and backend user-agents\n\ne.g. we won\u0027t be able to use only user-agent search for request from an internal client to backends, because the service-\u003einternal client request would have same user-agent.","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":158,"context_line":"        self.app \u003d app or loadapp(conf_path, global_conf\u003dglobal_conf,"},{"line_number":159,"context_line":"                                  allow_modify_pipeline\u003dallow_modify_pipeline,)"},{"line_number":160,"context_line":"        self.user_agent \u003d \\"},{"line_number":161,"context_line":"            self.app._pipeline_final_app.backend_user_agent \u003d user_agent"},{"line_number":162,"context_line":"        self.request_tries \u003d request_tries"},{"line_number":163,"context_line":"        self.use_replication_network \u003d use_replication_network"},{"line_number":164,"context_line":"        self.get_object_ring \u003d self.app._pipeline_final_app.get_object_ring"}],"source_content_type":"text/x-python","patch_set":8,"id":"9ece7368_3fc4ff88","line":161,"in_reply_to":"2b4c4153_3afae527","updated":"2022-01-24 15:58:57.000000000","message":"i don\u0027t understand this distinction.  IIRC we had noted that proxy logging messages from ic daemons already included the \"correct\" user agent string; I originally thought it was just a bug that this user-agent wasn\u0027t carried forward to the backend requests!\n\nBut of course, I realized we don\u0027t expect client facing requests to the proxy to carry *their* user-agent to the backend storage servce (instead we get proxy-pid user-agent to a 1.conf server)\n\nI still want to be able to look at a 2.conf log line and know which consistency system made the request - correlating back to the ic proxy logging message txid is a PITA.","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49cbb3e39ced407095835e422842a5697a888de4","unresolved":true,"context_lines":[{"line_number":1778,"context_line":"        if self.app.backend_user_agent:"},{"line_number":1779,"context_line":"            user_agent \u003d self.app.backend_user_agent"},{"line_number":1780,"context_line":"        else:"},{"line_number":1781,"context_line":"            user_agent \u003d \u0027proxy-server %s\u0027 % os.getpid()"},{"line_number":1782,"context_line":"        headers[\u0027user-agent\u0027] \u003d user_agent"},{"line_number":1783,"context_line":"        headers[\u0027referer\u0027] \u003d referer"},{"line_number":1784,"context_line":"        return headers"}],"source_content_type":"text/x-python","patch_set":2,"id":"675f50d8_1bf465f9","line":1781,"updated":"2021-11-29 16:44:07.000000000","message":"i would have expected the default value for `backend_user_agent` to be the getpid value - unless internal_client overrides it","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a1108a823ebadcef4befbae920fe2ac00d5c2d9","unresolved":true,"context_lines":[{"line_number":1778,"context_line":"        if self.app.backend_user_agent:"},{"line_number":1779,"context_line":"            user_agent \u003d self.app.backend_user_agent"},{"line_number":1780,"context_line":"        else:"},{"line_number":1781,"context_line":"            user_agent \u003d \u0027proxy-server %s\u0027 % os.getpid()"},{"line_number":1782,"context_line":"        headers[\u0027user-agent\u0027] \u003d user_agent"},{"line_number":1783,"context_line":"        headers[\u0027referer\u0027] \u003d referer"},{"line_number":1784,"context_line":"        return headers"}],"source_content_type":"text/x-python","patch_set":2,"id":"0c5a6a45_b82965a0","line":1781,"in_reply_to":"675f50d8_1bf465f9","updated":"2021-11-29 22:31:16.000000000","message":"Yeah, good idea, makes alot of sense.\n\nI went this way becauce I hadn\u0027t double checked the worker code yet and at this point I knew the pid was the correct pid. (didn\u0027t want to report the wrong pid).\n\nLooking now, the fork happens before the loadapp. So yeah I\u0027ll default it to the PID version in the constructor.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":1774,"context_line":"            referer \u003d \u0027\u0027"},{"line_number":1775,"context_line":"        headers[\u0027x-trans-id\u0027] \u003d self.trans_id"},{"line_number":1776,"context_line":"        headers[\u0027connection\u0027] \u003d \u0027close\u0027"},{"line_number":1777,"context_line":"        headers[\u0027user-agent\u0027] \u003d self.app.backend_user_agent"},{"line_number":1778,"context_line":"        headers[\u0027referer\u0027] \u003d referer"},{"line_number":1779,"context_line":"        return headers"},{"line_number":1780,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"4bca417b_ff3f0ba1","line":1777,"updated":"2022-01-11 01:21:31.000000000","message":"this makes sense with the change in proxy.server","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":1775,"context_line":"            referer \u003d \u0027\u0027"},{"line_number":1776,"context_line":"        headers[\u0027x-trans-id\u0027] \u003d self.trans_id"},{"line_number":1777,"context_line":"        headers[\u0027connection\u0027] \u003d \u0027close\u0027"},{"line_number":1778,"context_line":"        headers[\u0027user-agent\u0027] \u003d self.app.backend_user_agent"},{"line_number":1779,"context_line":"        headers[\u0027referer\u0027] \u003d referer"},{"line_number":1780,"context_line":"        return headers"},{"line_number":1781,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"061360db_926dd3f6","line":1778,"updated":"2022-01-24 15:58:57.000000000","message":"this is my favorite part of this change; it\u0027s lifting some magic override we were doing at the last second before we make a backend request up to a string we initialize on the app driving the request at configuration time - brilliant!\n\ni\u0027m disappointed to notice reverting this change doesn\u0027t cause any tests to fail - please update (or copy and modify) test/unit/proxy/controllers/test_base.py:TestFuncs.test_generate_request_headers","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1b43698d54d1acefbd0aedc3dbb58145902d8965","unresolved":false,"context_lines":[{"line_number":1775,"context_line":"            referer \u003d \u0027\u0027"},{"line_number":1776,"context_line":"        headers[\u0027x-trans-id\u0027] \u003d self.trans_id"},{"line_number":1777,"context_line":"        headers[\u0027connection\u0027] \u003d \u0027close\u0027"},{"line_number":1778,"context_line":"        headers[\u0027user-agent\u0027] \u003d self.app.backend_user_agent"},{"line_number":1779,"context_line":"        headers[\u0027referer\u0027] \u003d referer"},{"line_number":1780,"context_line":"        return headers"},{"line_number":1781,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"a3e9278e_e8331162","line":1778,"in_reply_to":"061360db_926dd3f6","updated":"2022-01-28 03:36:21.000000000","message":"Done","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"}],"swift/proxy/server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"49cbb3e39ced407095835e422842a5697a888de4","unresolved":true,"context_lines":[{"line_number":627,"context_line":"        return nodes"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":"    def set_backend_user_agent(self, value):"},{"line_number":630,"context_line":"        self.backend_user_agent \u003d value"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"    def set_node_timing(self, node, timing):"},{"line_number":633,"context_line":"        if not self.sorts_by_timing:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c2c3da3_7b2dda4f","line":630,"updated":"2021-11-29 16:44:07.000000000","message":"it\u0027s quite unusual style to have a setter method for a \"public\" class attribute like this","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4a1108a823ebadcef4befbae920fe2ac00d5c2d9","unresolved":true,"context_lines":[{"line_number":627,"context_line":"        return nodes"},{"line_number":628,"context_line":""},{"line_number":629,"context_line":"    def set_backend_user_agent(self, value):"},{"line_number":630,"context_line":"        self.backend_user_agent \u003d value"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"    def set_node_timing(self, node, timing):"},{"line_number":633,"context_line":"        if not self.sorts_by_timing:"}],"source_content_type":"text/x-python","patch_set":2,"id":"e54a0584_02cf1713","line":630,"in_reply_to":"5c2c3da3_7b2dda4f","updated":"2021-11-29 22:31:16.000000000","message":"yeah it is. But calling getattr on the attribute will send up a copy of the string, so doesn\u0027t work. So it needs to be something that can move by reference so a new class or a list [\u0027some_backend_user_agent_value\u0027] or a function itself, as it moves by reference.\n\nOriginally I made this a property, thinking it\u0027ll send up the property object... but it doesn\u0027t. So ended up going with a set_* method.","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":210,"context_line":"        self.object_chunk_size \u003d int(conf.get(\u0027object_chunk_size\u0027, 65536))"},{"line_number":211,"context_line":"        self.client_chunk_size \u003d int(conf.get(\u0027client_chunk_size\u0027, 65536))"},{"line_number":212,"context_line":"        self.trans_id_suffix \u003d conf.get(\u0027trans_id_suffix\u0027, \u0027\u0027)"},{"line_number":213,"context_line":"        self.backend_user_agent \u003d \u0027proxy-server %s\u0027 % os.getpid()"},{"line_number":214,"context_line":"        self.post_quorum_timeout \u003d float(conf.get(\u0027post_quorum_timeout\u0027, 0.5))"},{"line_number":215,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":216,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":4,"id":"b8020a6a_37a895a1","line":213,"updated":"2022-01-11 01:21:31.000000000","message":"this makes sense given the change in controllers/base.py\n\n... but it\u0027s not configurable or parameterized","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f892f3b97a57f8ef2001879b7b695970541d1d63","unresolved":true,"context_lines":[{"line_number":210,"context_line":"        self.object_chunk_size \u003d int(conf.get(\u0027object_chunk_size\u0027, 65536))"},{"line_number":211,"context_line":"        self.client_chunk_size \u003d int(conf.get(\u0027client_chunk_size\u0027, 65536))"},{"line_number":212,"context_line":"        self.trans_id_suffix \u003d conf.get(\u0027trans_id_suffix\u0027, \u0027\u0027)"},{"line_number":213,"context_line":"        self.backend_user_agent \u003d \u0027proxy-server %s\u0027 % os.getpid()"},{"line_number":214,"context_line":"        self.post_quorum_timeout \u003d float(conf.get(\u0027post_quorum_timeout\u0027, 0.5))"},{"line_number":215,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":216,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":4,"id":"054dd3ff_7cdcd595","line":213,"in_reply_to":"b8020a6a_37a895a1","updated":"2022-01-12 06:00:30.000000000","message":"Well it wasn\u0027t before either. If we want to have it as an option, I\u0027m OK with that.\nJust wasn\u0027t adding any extra functionality to the scope, just making it work :)","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a486540eedc0ad33f9936ddb309f29577713a079","unresolved":true,"context_lines":[{"line_number":210,"context_line":"        self.object_chunk_size \u003d int(conf.get(\u0027object_chunk_size\u0027, 65536))"},{"line_number":211,"context_line":"        self.client_chunk_size \u003d int(conf.get(\u0027client_chunk_size\u0027, 65536))"},{"line_number":212,"context_line":"        self.trans_id_suffix \u003d conf.get(\u0027trans_id_suffix\u0027, \u0027\u0027)"},{"line_number":213,"context_line":"        self.backend_user_agent \u003d \u0027proxy-server %s\u0027 % os.getpid()"},{"line_number":214,"context_line":"        self.post_quorum_timeout \u003d float(conf.get(\u0027post_quorum_timeout\u0027, 0.5))"},{"line_number":215,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":216,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":8,"id":"197b9d0c_e3f06722","line":213,"updated":"2022-01-21 13:27:14.000000000","message":"nit: this non-configurable is assigned in the middle of variables that are loaded from conf\n\n...but, then again, it would be nice if it were configurable","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":210,"context_line":"        self.object_chunk_size \u003d int(conf.get(\u0027object_chunk_size\u0027, 65536))"},{"line_number":211,"context_line":"        self.client_chunk_size \u003d int(conf.get(\u0027client_chunk_size\u0027, 65536))"},{"line_number":212,"context_line":"        self.trans_id_suffix \u003d conf.get(\u0027trans_id_suffix\u0027, \u0027\u0027)"},{"line_number":213,"context_line":"        self.backend_user_agent \u003d \u0027proxy-server %s\u0027 % os.getpid()"},{"line_number":214,"context_line":"        self.post_quorum_timeout \u003d float(conf.get(\u0027post_quorum_timeout\u0027, 0.5))"},{"line_number":215,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":216,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":8,"id":"b4e25c21_9cbce440","line":213,"in_reply_to":"197b9d0c_e3f06722","updated":"2022-01-24 15:58:57.000000000","message":"as a follow-up, if needed, a template system that allows interpolation might let us make this configurable and still have backwards compatibility.  But this change definately wasn\u0027t about changing what 1.conf storage servers log for client requests.\n\nhaving templates that could interpolate swift.source sub-request codes or client user-agent or workflow-id could be powerful","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1b43698d54d1acefbd0aedc3dbb58145902d8965","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        self.object_chunk_size \u003d int(conf.get(\u0027object_chunk_size\u0027, 65536))"},{"line_number":211,"context_line":"        self.client_chunk_size \u003d int(conf.get(\u0027client_chunk_size\u0027, 65536))"},{"line_number":212,"context_line":"        self.trans_id_suffix \u003d conf.get(\u0027trans_id_suffix\u0027, \u0027\u0027)"},{"line_number":213,"context_line":"        self.backend_user_agent \u003d \u0027proxy-server %s\u0027 % os.getpid()"},{"line_number":214,"context_line":"        self.post_quorum_timeout \u003d float(conf.get(\u0027post_quorum_timeout\u0027, 0.5))"},{"line_number":215,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":216,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":8,"id":"eefcc65a_38e33f63","line":213,"in_reply_to":"b4e25c21_9cbce440","updated":"2022-01-28 03:36:21.000000000","message":"I\u0027ll move it up a bit, and leave making it configurable as a follow up.","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"}],"test/unit/common/middleware/helpers.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1496be4653db664cbd536942dc384dd360b43e6f","unresolved":true,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"class FakeApp(object):"},{"line_number":278,"context_line":"    \"\"\""},{"line_number":279,"context_line":"    I simple FakeApp that also has the backend_user_agent plumbed in used for"},{"line_number":280,"context_line":"    Internal clients and expirers."},{"line_number":281,"context_line":"    \"\"\""},{"line_number":282,"context_line":"    def __init__(self, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"6ed0ad22_8aa01bda","line":279,"updated":"2021-11-29 08:51:30.000000000","message":"s/I/A/","commit_id":"23bb05ad6277c4564e9df48b1d78a5bf5d7d50e0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":94,"context_line":"        self.container_ring \u003d FakeRing()"},{"line_number":95,"context_line":"        self.get_object_ring \u003d lambda policy_index: FakeRing()"},{"line_number":96,"context_line":"        self.backend_user_agent \u003d \"fake_swift\""},{"line_number":97,"context_line":"        self._pipeline_final_app \u003d self"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def _find_response(self, method, path):"},{"line_number":100,"context_line":"        path \u003d normalize_path(path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"0c4d33bd_bee0e162","line":97,"updated":"2022-01-11 01:21:31.000000000","message":"i was wondering about _pipeline_final_app when we\u0027re faking out loadapp in tests","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":280,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":281,"context_line":"        self.backend_user_agent \u003d None"},{"line_number":282,"context_line":"        if kwargs.get(\u0027call_method\u0027):"},{"line_number":283,"context_line":"            self.call_method \u003d kwargs[\u0027call_method\u0027]"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            self.call_method \u003d lambda env, start_response: None"},{"line_number":286,"context_line":"        self._pipeline_final_app \u003d self"}],"source_content_type":"text/x-python","patch_set":4,"id":"2a8c3235_103a1284","line":283,"updated":"2022-01-11 01:21:31.000000000","message":"if someone wanted this override __call__ I feel like it\u0027d be a more familiar OOO pattern to just subclass this and override __call__","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f892f3b97a57f8ef2001879b7b695970541d1d63","unresolved":true,"context_lines":[{"line_number":280,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":281,"context_line":"        self.backend_user_agent \u003d None"},{"line_number":282,"context_line":"        if kwargs.get(\u0027call_method\u0027):"},{"line_number":283,"context_line":"            self.call_method \u003d kwargs[\u0027call_method\u0027]"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            self.call_method \u003d lambda env, start_response: None"},{"line_number":286,"context_line":"        self._pipeline_final_app \u003d self"}],"source_content_type":"text/x-python","patch_set":4,"id":"4ba0427a_7c8ab912","line":283,"in_reply_to":"2a8c3235_103a1284","updated":"2022-01-12 06:00:30.000000000","message":"Yeah this probably isn\u0027t the most OO way. It was more we were using functions (def) and apps in the expirer tests, so this was a quick way to reuse them without having to keep inheriting new apps for each test.. ie. the lazy way out.\n\nBut your right I should probably just to it properly :)","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":283,"context_line":"            self.call_method \u003d kwargs[\u0027call_method\u0027]"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            self.call_method \u003d lambda env, start_response: None"},{"line_number":286,"context_line":"        self._pipeline_final_app \u003d self"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    def __call__(self, env, start_response, **kwargs):"},{"line_number":289,"context_line":"        return self.call_method(env, start_response)"}],"source_content_type":"text/x-python","patch_set":4,"id":"e7d8562c_4bb2f5e7","line":286,"updated":"2022-01-11 01:21:31.000000000","message":"so if you use this fake it\u0027s impossible to test something is setting an attribute on self.app or self.app._pipeline_final_app because they\u0027re both the same instance","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f892f3b97a57f8ef2001879b7b695970541d1d63","unresolved":true,"context_lines":[{"line_number":283,"context_line":"            self.call_method \u003d kwargs[\u0027call_method\u0027]"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            self.call_method \u003d lambda env, start_response: None"},{"line_number":286,"context_line":"        self._pipeline_final_app \u003d self"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    def __call__(self, env, start_response, **kwargs):"},{"line_number":289,"context_line":"        return self.call_method(env, start_response)"}],"source_content_type":"text/x-python","patch_set":4,"id":"e81120c5_f20379c1","line":286,"in_reply_to":"e7d8562c_4bb2f5e7","updated":"2022-01-12 06:00:30.000000000","message":"If the FakeApp is the last app in the middleware (which I assume its faking) shouldn\u0027t _pipeline_final_app _always_ point to itself?","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"}],"test/unit/common/test_internal_client.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":341,"context_line":"            def __init__(self, test):"},{"line_number":342,"context_line":"                self.test \u003d test"},{"line_number":343,"context_line":"                self.app \u003d FakeApp(call_method\u003dself.fake_app)"},{"line_number":344,"context_line":"                self.user_agent \u003d \u0027some_agent\u0027"},{"line_number":345,"context_line":"                self.request_tries \u003d 1"},{"line_number":346,"context_line":"                self.use_replication_network \u003d False"},{"line_number":347,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"06a10d66_24ccb43f","line":344,"updated":"2022-01-11 01:21:31.000000000","message":"it\u0027s a little frustrating that these Fake\u0027s inhert from a real InternalClient and then override so much of __init__ just so they can get a different stub response out of their app\n\nI think InternalClient should accept an app kwarg and do \n\n   self.app \u003d app or loadapp(...)\n\nso that it easier to setup","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":422,"context_line":"                                       keep_blank_values\u003dTrue,"},{"line_number":423,"context_line":"                                       strict_parsing\u003dTrue))"},{"line_number":424,"context_line":"        self.assertEqual(params, actual_params)"},{"line_number":425,"context_line":"        self.assertEqual(client.app.backend_user_agent, \u0027some_agent\u0027)"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    def test_make_request_retries(self):"},{"line_number":428,"context_line":"        class InternalClient(internal_client.InternalClient):"}],"source_content_type":"text/x-python","patch_set":4,"id":"170a5ef4_25c4f1f6","line":425,"updated":"2022-01-11 01:21:31.000000000","message":"honestly I half expected this to be client.app._final_app.backend_user_agent","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1b43698d54d1acefbd0aedc3dbb58145902d8965","unresolved":false,"context_lines":[{"line_number":422,"context_line":"                                       keep_blank_values\u003dTrue,"},{"line_number":423,"context_line":"                                       strict_parsing\u003dTrue))"},{"line_number":424,"context_line":"        self.assertEqual(params, actual_params)"},{"line_number":425,"context_line":"        self.assertEqual(client.app.backend_user_agent, \u0027some_agent\u0027)"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    def test_make_request_retries(self):"},{"line_number":428,"context_line":"        class InternalClient(internal_client.InternalClient):"}],"source_content_type":"text/x-python","patch_set":4,"id":"02e5693c_a291a231","line":425,"in_reply_to":"170a5ef4_25c4f1f6","updated":"2022-01-28 03:36:21.000000000","message":"yeah that is probably better, seeing as the app in the client should really be a full pipeline. Will change it.","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":334,"context_line":"        self.assertTrue(client.use_replication_network)"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"    def test_make_request_sets_user_agent(self):"},{"line_number":337,"context_line":"        class InternalClient(internal_client.InternalClient):"},{"line_number":338,"context_line":"            def __init__(self, test):"},{"line_number":339,"context_line":"                self.test \u003d test"},{"line_number":340,"context_line":"                self.app \u003d self.fake_app"}],"source_content_type":"text/x-python","patch_set":8,"id":"b069595b_37df558c","side":"PARENT","line":337,"updated":"2022-01-24 15:58:57.000000000","message":"this is actually kind of gross - this is *test_internal_client* and we\u0027re actually testing a *subclass*","commit_id":"26c0f31379ba4b47f84fe13c1f7f09a47abdb15a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        client \u003d internal_client.InternalClient("},{"line_number":351,"context_line":"            None, \u0027some_agent\u0027, 1, use_replication_network\u003dFalse,"},{"line_number":352,"context_line":"            app\u003dFakeApp(self))"},{"line_number":353,"context_line":"        client.make_request(\u0027GET\u0027, \u0027/\u0027, {}, (200,))"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"    def test_make_request_defaults_replication_network_header(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"46522def_eb9efdd8","line":352,"updated":"2022-01-24 15:58:57.000000000","message":"this FakeApp injection is much better IMHO","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":1418,"context_line":"        for line in client.iter_object_lines(\u0027account\u0027, \u0027container\u0027, \u0027object\u0027):"},{"line_number":1419,"context_line":"            ret_lines.append(line)"},{"line_number":1420,"context_line":"        self.assertEqual(lines, ret_lines)"},{"line_number":1421,"context_line":"        self.assertEqual(client.app.backend_user_agent, \u0027some_agent\u0027)"},{"line_number":1422,"context_line":""},{"line_number":1423,"context_line":"    def test_iter_object_lines_compressed_object(self):"},{"line_number":1424,"context_line":"        class FakeApp(FakeSwift):"}],"source_content_type":"text/x-python","patch_set":8,"id":"766bb53c_ef7ad2f9","line":1421,"updated":"2022-01-24 15:58:57.000000000","message":"I\u0027m seeing this assertion a lot and don\u0027t find it terribly valuable without some tests showing that the app\u0027s backend_user_agent value gets set on backend requests, but maybe FakeSwift mostly cuts us off form that?","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":1426,"context_line":"                super(FakeApp, self).__init__()"},{"line_number":1427,"context_line":"                self.lines \u003d lines"},{"line_number":1428,"context_line":""},{"line_number":1429,"context_line":"            def __call__(self, env, start_response):"},{"line_number":1430,"context_line":"                start_response(\u0027200 Ok\u0027, [(\u0027Content-Length\u0027, \u00270\u0027)])"},{"line_number":1431,"context_line":"                return internal_client.CompressingFileReader("},{"line_number":1432,"context_line":"                    BytesIO(b\u0027\\n\u0027.join(self.lines)))"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf1311c4_6ffecf21","line":1429,"updated":"2022-01-24 15:58:57.000000000","message":"I bet this env has an HTTP_USER_AGENT key with a value of \u0027some_agent\u0027 in there - but I guess that doesn\u0027t prove the real proxy base controller doesn\u0027t overwrite the user-agent string in the external request before sending it to the backend servers.","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":1468,"context_line":"            \u0027X-Backend-Allow-Reserved-Names\u0027: \u0027true\u0027,"},{"line_number":1469,"context_line":"            \u0027Host\u0027: \u0027localhost:80\u0027,"},{"line_number":1470,"context_line":"            \u0027X-Object-Meta-Color\u0027: \u0027Blue\u0027,"},{"line_number":1471,"context_line":"            \u0027User-Agent\u0027: \u0027test\u0027"},{"line_number":1472,"context_line":"        })], app._calls)"},{"line_number":1473,"context_line":"        self.assertEqual({}, app.unread_requests)"},{"line_number":1474,"context_line":"        self.assertEqual({}, app.unclosed_requests)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a0cb0db_18fc9d69","line":1471,"updated":"2022-01-24 15:58:57.000000000","message":"*finally* is this an assertion on the actual User-Agent value, but I guess this is just the *proxy* request; the fakeapp doesn\u0027t simulate *backend* requests","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"}],"test/unit/obj/test_expirer.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":17,"context_line":"from unittest import main, TestCase"},{"line_number":18,"context_line":"from test.debug_logger import debug_logger"},{"line_number":19,"context_line":"from test.unit import FakeRing, mocked_http_conn, make_timestamp_iter"},{"line_number":20,"context_line":"from test.unit.common.middleware.helpers import FakeApp"},{"line_number":21,"context_line":"from tempfile import mkdtemp"},{"line_number":22,"context_line":"from shutil import rmtree"},{"line_number":23,"context_line":"from collections import defaultdict"}],"source_content_type":"text/x-python","patch_set":4,"id":"4fe6b5e3_0850a54a","line":20,"updated":"2022-01-11 01:21:31.000000000","message":"who, what now?","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4847ee510e475771fc781a76a7acd99ff9192ad8","unresolved":true,"context_lines":[{"line_number":65,"context_line":"        \"\"\""},{"line_number":66,"context_line":"        self.aco_dict \u003d defaultdict(dict)"},{"line_number":67,"context_line":"        self.aco_dict.update(aco_dict)"},{"line_number":68,"context_line":"        self.user_agent \u003d None"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def get_account_info(self, account):"},{"line_number":71,"context_line":"        acc_dict \u003d self.aco_dict[account]"}],"source_content_type":"text/x-python","patch_set":4,"id":"90b022d9_21516859","line":68,"updated":"2022-01-11 01:21:31.000000000","message":"this doesn\u0027t appear to be needed","commit_id":"5d4ac2760844a42e3a9896f9f44bdebfb3622067"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"de2c6c17e40a33c5b43593d17b6a30f1e9d56b16","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        \"\"\""},{"line_number":65,"context_line":"        self.aco_dict \u003d defaultdict(dict)"},{"line_number":66,"context_line":"        self.aco_dict.update(aco_dict)"},{"line_number":67,"context_line":"        self.user_agent \u003d None"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def get_account_info(self, account):"},{"line_number":70,"context_line":"        acc_dict \u003d self.aco_dict[account]"}],"source_content_type":"text/x-python","patch_set":8,"id":"aa2e1f00_44aa0e61","line":67,"updated":"2022-01-24 15:58:57.000000000","message":"i don\u0027t think this is needed","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"1b43698d54d1acefbd0aedc3dbb58145902d8965","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        \"\"\""},{"line_number":65,"context_line":"        self.aco_dict \u003d defaultdict(dict)"},{"line_number":66,"context_line":"        self.aco_dict.update(aco_dict)"},{"line_number":67,"context_line":"        self.user_agent \u003d None"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def get_account_info(self, account):"},{"line_number":70,"context_line":"        acc_dict \u003d self.aco_dict[account]"}],"source_content_type":"text/x-python","patch_set":8,"id":"4d8c8f52_6850c2b2","line":67,"in_reply_to":"aa2e1f00_44aa0e61","updated":"2022-01-28 03:36:21.000000000","message":"Nope it isn\u0027t, removed.","commit_id":"d8d0a414a0be3a63cdb8e870a2f69d741b95d235"}]}