)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Matthew Oliver \u003cmatt@oliver.net.au\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-12-08 12:24:19 +1100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactored errorlimited out of proxy + global cache"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch pulls the proxy\u0027s error limited code into a class that can be"},{"line_number":10,"context_line":"used in any class to provide error_limiting."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"f81a8e20_7d98d97b","line":7,"updated":"2021-12-08 15:49:26.000000000","message":"why errorlimitED - almost all our other modules end in an R?\n\n    from errorlimiter import ErrorLimiter\n\nreminds me of\n\n     from replicator import Replicator","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3803c465df1ccd4422c911f2fdd095ce32083198","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Matthew Oliver \u003cmatt@oliver.net.au\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-12-08 12:24:19 +1100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactored errorlimited out of proxy + global cache"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch pulls the proxy\u0027s error limited code into a class that can be"},{"line_number":10,"context_line":"used in any class to provide error_limiting."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"0b116260_4eb19911","line":7,"in_reply_to":"f81a8e20_7d98d97b","updated":"2021-12-09 04:27:34.000000000","message":"Done","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"51535137_8077a494","updated":"2021-12-08 15:49:26.000000000","message":"I think it would be a good thing proxy workers to share error limiting stats","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7cf7b27b9aea9e3b7d4e434f1f26befe4e6b2e49","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"6a41aa7b_323b1c01","updated":"2022-09-22 17:18:36.000000000","message":"ok, I see where this is going","commit_id":"fc171d60220ea75a7f92d9aefb0b58d029387f44"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6bf1ef22265d528584bca4f7a0409870905a8370","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"b6c85c0f_03465f24","updated":"2022-09-23 15:21:09.000000000","message":"I\u0027m pushing often to this patch chain, there\u0027s likely more work yet to do to polish things off.","commit_id":"e9eab0b24ccdc49e03e4b31325b07f6780346a87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c2b420afbd6bcd235d0f5a590fcaa711fb8c9bf6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"cb147bc7_42636815","updated":"2022-09-27 21:47:11.000000000","message":"love all these concrete classes in error_limiter!","commit_id":"e0b76bcd6357d86565dbfa8ca0956bfee4ace821"}],"etc/proxy-server.conf-sample":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":175,"context_line":"#"},{"line_number":176,"context_line":"# Instead of a workers memory store the error limited nodes in cache. This allows"},{"line_number":177,"context_line":"# other error_limited services, like other proxies, to share error_limited information."},{"line_number":178,"context_line":"# error_limited_cache_enabled \u003d false"},{"line_number":179,"context_line":"#"},{"line_number":180,"context_line":"# When the error_limited cache is enabled one might want to have control of what services"},{"line_number":181,"context_line":"# share the same error_limited nodes. For example, if you only want those in a zone to share"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"79726dfe_c7e99fc6","line":178,"updated":"2021-12-08 15:49:26.000000000","message":"I think \"enabled\" here is misleading; you can\u0027t *disbale* the error cache; you can only decide if it\u0027s per-worker or in memcache.\n\n    error_limiting_use_memcache \u003d false","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3803c465df1ccd4422c911f2fdd095ce32083198","unresolved":false,"context_lines":[{"line_number":175,"context_line":"#"},{"line_number":176,"context_line":"# Instead of a workers memory store the error limited nodes in cache. This allows"},{"line_number":177,"context_line":"# other error_limited services, like other proxies, to share error_limited information."},{"line_number":178,"context_line":"# error_limited_cache_enabled \u003d false"},{"line_number":179,"context_line":"#"},{"line_number":180,"context_line":"# When the error_limited cache is enabled one might want to have control of what services"},{"line_number":181,"context_line":"# share the same error_limited nodes. For example, if you only want those in a zone to share"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"a2897b64_562a3696","line":178,"in_reply_to":"79726dfe_c7e99fc6","updated":"2021-12-09 04:27:34.000000000","message":"yeah good point. I had cache to mean memcache, but the in worker dict is also a cache. Will rework.","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"944743627e991084289c5509b9ec0e5299c59605","unresolved":true,"context_lines":[{"line_number":190,"context_line":"# Instead of a workers memory store the error limited nodes in memcache. This"},{"line_number":191,"context_line":"# allows other error_limited services, like other proxies, to share"},{"line_number":192,"context_line":"# error_limited information."},{"line_number":193,"context_line":"# error_limiting_use_memcache \u003d false"},{"line_number":194,"context_line":"#"},{"line_number":195,"context_line":"# When the error_limited cache is enabled one might want to have control of"},{"line_number":196,"context_line":"# what services share the same error_limited nodes. For example, if you only"}],"source_content_type":"application/octet-stream","patch_set":23,"id":"edcab064_6d18783b","line":193,"updated":"2022-10-12 13:07:53.000000000","message":"@Matt it\u0027s not immediately obvious to me that the proxy server app conf will have the memcache server config - that looks like it lives in the memcache middleware section. Do we need more in the sample config about that? Do we need to add conf to the saio setup etc. to have memcahce conf available to the proxy?","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"}],"swift/common/error_limited.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"8c401d76299fe95378354a8294e04a83aec9b34e","unresolved":true,"context_lines":[{"line_number":26,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":27,"context_line":"        self.error_limiting_pool \u003d conf.get(\u0027error_limiting_pool\u0027, \u0027\u0027)"},{"line_number":28,"context_line":"        self.error_limited_cache_disabled \u003d config_true_value("},{"line_number":29,"context_line":"            conf.get(\u0027error_limited_cache_disabled\u0027, True))"},{"line_number":30,"context_line":"        if self.error_limited_cache_disabled:"},{"line_number":31,"context_line":"            self.memcache \u003d None"},{"line_number":32,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5d137d8f_23032a0a","line":29,"updated":"2021-12-07 02:45:26.000000000","message":"Need to add doc and sample config for these 2 new config options. (error_limiting_pool and error_limited_cache_disabled).","commit_id":"acad44c8e8d77542c136c8c79fa1dc427b2de8c4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":26,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":27,"context_line":"        self.error_limiting_pool \u003d conf.get(\u0027error_limiting_pool\u0027, \u0027\u0027)"},{"line_number":28,"context_line":"        self.error_limited_cache_enabled \u003d config_true_value("},{"line_number":29,"context_line":"            conf.get(\u0027error_limited_cache_enabled\u0027, False))"},{"line_number":30,"context_line":"        if self.error_limited_cache_enabled:"},{"line_number":31,"context_line":"            self.memcache \u003d load_memcache(self, conf)"},{"line_number":32,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"80c4d408_d7d58885","line":29,"updated":"2021-12-08 15:49:26.000000000","message":"so default is still per-worker stats 👍","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":27,"context_line":"        self.error_limiting_pool \u003d conf.get(\u0027error_limiting_pool\u0027, \u0027\u0027)"},{"line_number":28,"context_line":"        self.error_limited_cache_enabled \u003d config_true_value("},{"line_number":29,"context_line":"            conf.get(\u0027error_limited_cache_enabled\u0027, False))"},{"line_number":30,"context_line":"        if self.error_limited_cache_enabled:"},{"line_number":31,"context_line":"            self.memcache \u003d load_memcache(self, conf)"},{"line_number":32,"context_line":"        else:"},{"line_number":33,"context_line":"            self.memcache \u003d None"}],"source_content_type":"text/x-python","patch_set":5,"id":"01ab095c_7c962d7c","line":30,"updated":"2021-12-08 15:49:26.000000000","message":"this is a pretty big option that fundementally changes what this class does\n\nI\u0027d like to consider if a BaseErrorLimiter, InMemoryErrorLimiter and MemcacheErrorLimter might be easier to maintain","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def node_key(self, node):"},{"line_number":37,"context_line":"        node_key \u003d \"{ip}:{port}/{device}\".format(**node)"},{"line_number":38,"context_line":"        return \"{}/{}\".format(self.error_limiting_pool, node_key)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def error_limited(self, node):"},{"line_number":41,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"697ce29e_c1f3987f","line":38,"updated":"2021-12-08 15:49:26.000000000","message":"so it\u0027s either \u0027pool/node_key\u0027 or just \u0027/node_key\u0027","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3803c465df1ccd4422c911f2fdd095ce32083198","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def node_key(self, node):"},{"line_number":37,"context_line":"        node_key \u003d \"{ip}:{port}/{device}\".format(**node)"},{"line_number":38,"context_line":"        return \"{}/{}\".format(self.error_limiting_pool, node_key)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def error_limited(self, node):"},{"line_number":41,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"35dcbcd9_cc4a52df","line":38,"in_reply_to":"697ce29e_c1f3987f","updated":"2021-12-09 04:27:34.000000000","message":"yeah, I figured an additional \u0027/\u0027 at the start by default wont be a problem","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":67,"context_line":"        if self.memcache:"},{"line_number":68,"context_line":"            resp \u003d self.memcache.get(node_key)"},{"line_number":69,"context_line":"            return resp or {}"},{"line_number":70,"context_line":"        return self._error_limiting.setdefault(node_key, {})"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def error_limit(self, node, msg):"},{"line_number":73,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"12096b05_9767dfe8","line":70,"updated":"2021-12-08 15:49:26.000000000","message":"this might be the only method in a BaseErrorLimiter that requires implementation","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"}],"swift/common/error_limiter.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7cf7b27b9aea9e3b7d4e434f1f26befe4e6b2e49","unresolved":true,"context_lines":[{"line_number":22,"context_line":"            conf.get(\u0027error_limiting_use_memcache\u0027, False)):"},{"line_number":23,"context_line":"        return MemcacheErrorLimiter(conf, logger)"},{"line_number":24,"context_line":"    else:"},{"line_number":25,"context_line":"        return InMemoryErrorLimiter(conf, logger)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class BaseErrorLimiter(object):"}],"source_content_type":"text/x-python","patch_set":14,"id":"54f33185_39fbd22e","line":25,"updated":"2022-09-22 17:18:36.000000000","message":"I wonder if instead of *is a* style inheritance we would prefer to maintain a single concreate SwiftErrorLimiter class that takes a config has *has a* error_counter impl that it creates based on a config option or plugin.","commit_id":"fc171d60220ea75a7f92d9aefb0b58d029387f44"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6bf1ef22265d528584bca4f7a0409870905a8370","unresolved":true,"context_lines":[{"line_number":22,"context_line":"            conf.get(\u0027error_limiting_use_memcache\u0027, False)):"},{"line_number":23,"context_line":"        return MemcacheErrorLimiter(conf, logger)"},{"line_number":24,"context_line":"    else:"},{"line_number":25,"context_line":"        return InMemoryErrorLimiter(conf, logger)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class BaseErrorLimiter(object):"}],"source_content_type":"text/x-python","patch_set":14,"id":"f6938505_0e7e8ccf","line":25,"in_reply_to":"54f33185_39fbd22e","updated":"2022-09-23 15:21:09.000000000","message":"+1 I was heading in same line of thinking. Done.","commit_id":"fc171d60220ea75a7f92d9aefb0b58d029387f44"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"be1326f1e72f671c9891fb884fafaf704337ea23","unresolved":false,"context_lines":[{"line_number":22,"context_line":"            conf.get(\u0027error_limiting_use_memcache\u0027, False)):"},{"line_number":23,"context_line":"        return MemcacheErrorLimiter(conf, logger)"},{"line_number":24,"context_line":"    else:"},{"line_number":25,"context_line":"        return InMemoryErrorLimiter(conf, logger)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class BaseErrorLimiter(object):"}],"source_content_type":"text/x-python","patch_set":14,"id":"4c4ea56e_0ee78e88","line":25,"in_reply_to":"f6938505_0e7e8ccf","updated":"2022-10-12 12:45:10.000000000","message":"Done","commit_id":"fc171d60220ea75a7f92d9aefb0b58d029387f44"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7cf7b27b9aea9e3b7d4e434f1f26befe4e6b2e49","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        if self.memcache is None:"},{"line_number":139,"context_line":"            msg \u003d \u0027Failed to create the MemcacheRing client\u0027"},{"line_number":140,"context_line":"            self.logger.warn(msg)"},{"line_number":141,"context_line":"            raise AttributeError(msg)"},{"line_number":142,"context_line":"        self.keys \u003d ((\u0027errors\u0027, int), (\u0027last_error\u0027, float))"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    def _get_current_error_limit_stats(self, node_key):"}],"source_content_type":"text/x-python","patch_set":14,"id":"1f7c64c4_f293ad9d","line":141,"updated":"2022-09-22 17:18:36.000000000","message":"why is this an AttributeError I wonder","commit_id":"fc171d60220ea75a7f92d9aefb0b58d029387f44"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"be1326f1e72f671c9891fb884fafaf704337ea23","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        if self.memcache is None:"},{"line_number":139,"context_line":"            msg \u003d \u0027Failed to create the MemcacheRing client\u0027"},{"line_number":140,"context_line":"            self.logger.warn(msg)"},{"line_number":141,"context_line":"            raise AttributeError(msg)"},{"line_number":142,"context_line":"        self.keys \u003d ((\u0027errors\u0027, int), (\u0027last_error\u0027, float))"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    def _get_current_error_limit_stats(self, node_key):"}],"source_content_type":"text/x-python","patch_set":14,"id":"536f9978_0bf47657","line":141,"in_reply_to":"1f7c64c4_f293ad9d","updated":"2022-10-12 12:45:10.000000000","message":"Done","commit_id":"fc171d60220ea75a7f92d9aefb0b58d029387f44"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6bf1ef22265d528584bca4f7a0409870905a8370","unresolved":true,"context_lines":[{"line_number":79,"context_line":"        Mark a node as error limited. This immediately pretends the"},{"line_number":80,"context_line":"        node received enough errors to trigger error suppression. Use"},{"line_number":81,"context_line":"        this for errors like Insufficient Storage. For other errors"},{"line_number":82,"context_line":"        use :func:`increment`."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        :param node: dictionary of node to error limit"},{"line_number":85,"context_line":"        :param msg: error message"}],"source_content_type":"text/x-python","patch_set":17,"id":"757ae53e_ad781f09","line":82,"updated":"2022-09-23 15:21:09.000000000","message":"bah! global replace error :(","commit_id":"e9eab0b24ccdc49e03e4b31325b07f6780346a87"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"be1326f1e72f671c9891fb884fafaf704337ea23","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        Mark a node as error limited. This immediately pretends the"},{"line_number":80,"context_line":"        node received enough errors to trigger error suppression. Use"},{"line_number":81,"context_line":"        this for errors like Insufficient Storage. For other errors"},{"line_number":82,"context_line":"        use :func:`increment`."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        :param node: dictionary of node to error limit"},{"line_number":85,"context_line":"        :param msg: error message"}],"source_content_type":"text/x-python","patch_set":17,"id":"a6c1cdb6_7e08a851","line":82,"in_reply_to":"757ae53e_ad781f09","updated":"2022-10-12 12:45:10.000000000","message":"Done","commit_id":"e9eab0b24ccdc49e03e4b31325b07f6780346a87"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"be1326f1e72f671c9891fb884fafaf704337ea23","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        self._error_limited \u003d {}"},{"line_number":27,"context_line":"        if config_true_value("},{"line_number":28,"context_line":"                conf.get(\u0027error_limiting_use_memcache\u0027, False)):"},{"line_number":29,"context_line":"            self.error_limiting_pool \u003d conf.get(\u0027error_limiting_pool\u0027, \u0027\u0027)"},{"line_number":30,"context_line":"            self.stats \u003d MemcacheErrorStats(conf, logger)"},{"line_number":31,"context_line":"        else:"},{"line_number":32,"context_line":"            self.error_limiting_pool \u003d None"}],"source_content_type":"text/x-python","patch_set":22,"id":"80bb89b1_29461dc3","line":29,"range":{"start_line":29,"start_character":12,"end_line":29,"end_character":37},"updated":"2022-10-12 12:45:10.000000000","message":"I don\u0027t like that error_limiting_pool bleeds into ErrorLimiter - it is specific to memcache stats","commit_id":"bc03999844456356514734c7b498f5a0803646b6"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8f845954659c6ae63f58a432bed75a33e8cb6d4d","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        self._error_limited \u003d {}"},{"line_number":27,"context_line":"        if config_true_value("},{"line_number":28,"context_line":"                conf.get(\u0027error_limiting_use_memcache\u0027, False)):"},{"line_number":29,"context_line":"            self.error_limiting_pool \u003d conf.get(\u0027error_limiting_pool\u0027, \u0027\u0027)"},{"line_number":30,"context_line":"            self.stats \u003d MemcacheErrorStats(conf, logger)"},{"line_number":31,"context_line":"        else:"},{"line_number":32,"context_line":"            self.error_limiting_pool \u003d None"}],"source_content_type":"text/x-python","patch_set":22,"id":"5ec587c0_8a67ed51","line":29,"range":{"start_line":29,"start_character":12,"end_line":29,"end_character":37},"in_reply_to":"80bb89b1_29461dc3","updated":"2022-10-26 13:24:49.000000000","message":"Done","commit_id":"bc03999844456356514734c7b498f5a0803646b6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"966a9abab6f6c2449062f3ce30a1361c264b2f52","unresolved":true,"context_lines":[{"line_number":86,"context_line":"            return False"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        if \u0027last_error\u0027 in error_stats and error_stats[\u0027last_error\u0027] \u003c \\"},{"line_number":89,"context_line":"                now - self.suppression_interval:"},{"line_number":90,"context_line":"            self.stats.pop(node_key)"},{"line_number":91,"context_line":"            self._limited_nodes.pop(node_key, None)"},{"line_number":92,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":23,"id":"15d2c607_1425cf77","line":89,"updated":"2022-10-19 22:18:06.000000000","message":"Now that time.time() and last_error may have come from different nodes (ie, there will be *some* clock skew), we could drop in here even with suppression_interval\u003d0...\n\nIs there a way to turn off error limiting at that point? I guess, just make sure you don\u0027t use memcache 😂","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b61da8aaf8c6d70210cfb2014e9ad6486deb0632","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        self.limiting_pool \u003d limiting_pool"},{"line_number":161,"context_line":"        self.keys \u003d ((\u0027errors\u0027, int), (\u0027last_error\u0027, float))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def _prefix_node_key(self, node_key):"},{"line_number":164,"context_line":"        if self.limiting_pool:"},{"line_number":165,"context_line":"            return \"{}/{}\".format(self.limiting_pool, node_key)"},{"line_number":166,"context_line":"        return node_key"}],"source_content_type":"text/x-python","patch_set":23,"id":"0d53120d_b9d8a714","line":163,"updated":"2022-10-19 18:26:24.000000000","message":"We always add either \"errors\" or \"last_error\" on the end later -- should we just accept that as an extra arg here (and maybe rename the method) so we can just call format() once?","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"22427634e513f54380ab25078a6dd118c887047e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        self.limiting_pool \u003d limiting_pool"},{"line_number":161,"context_line":"        self.keys \u003d ((\u0027errors\u0027, int), (\u0027last_error\u0027, float))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def _prefix_node_key(self, node_key):"},{"line_number":164,"context_line":"        if self.limiting_pool:"},{"line_number":165,"context_line":"            return \"{}/{}\".format(self.limiting_pool, node_key)"},{"line_number":166,"context_line":"        return node_key"}],"source_content_type":"text/x-python","patch_set":23,"id":"8c237cdb_d27f045b","line":163,"in_reply_to":"0d53120d_b9d8a714","updated":"2022-10-20 16:03:21.000000000","message":"Done","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b61da8aaf8c6d70210cfb2014e9ad6486deb0632","unresolved":true,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def _prefix_node_key(self, node_key):"},{"line_number":164,"context_line":"        if self.limiting_pool:"},{"line_number":165,"context_line":"            return \"{}/{}\".format(self.limiting_pool, node_key)"},{"line_number":166,"context_line":"        return node_key"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def get(self, node_key):"}],"source_content_type":"text/x-python","patch_set":23,"id":"2f99347e_b5275856","line":165,"updated":"2022-10-19 18:26:24.000000000","message":"I worry that we can get into a weird place if someone makes the mistake of configuring\n\n error_limiting_pool \u003d account\n\nor\n\n error_limiting_pool \u003d container\n\nsay... see https://github.com/openstack/swift/blob/2.30.0/swift/proxy/controllers/base.py#L618-L620\n\nThat is to say, I think there ought to be some hardcoded, as-yet-unused prefix *in addition* to whatever we let operators define.","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"22427634e513f54380ab25078a6dd118c887047e","unresolved":true,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def _prefix_node_key(self, node_key):"},{"line_number":164,"context_line":"        if self.limiting_pool:"},{"line_number":165,"context_line":"            return \"{}/{}\".format(self.limiting_pool, node_key)"},{"line_number":166,"context_line":"        return node_key"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def get(self, node_key):"}],"source_content_type":"text/x-python","patch_set":23,"id":"00c795f9_68665b97","line":165,"in_reply_to":"2f99347e_b5275856","updated":"2022-10-20 16:03:21.000000000","message":"+1\n\nand/or make limiting_pool be required","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"29cd83279c2ff3d417db82d69cfa25ea9a00bfc6","unresolved":true,"context_lines":[{"line_number":170,"context_line":"        resp \u003d {}"},{"line_number":171,"context_line":"        for key, v_type in self.keys:"},{"line_number":172,"context_line":"            res \u003d self.memcache.get(\"{}/{}\".format(node_key, key))"},{"line_number":173,"context_line":"            if res:"},{"line_number":174,"context_line":"                resp[key] \u003d v_type(res)"},{"line_number":175,"context_line":"        return resp"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"e540bdc0_82eb72ee","line":173,"updated":"2022-10-19 19:22:41.000000000","message":":-/ So neither the \"errors\" nor \"last_error\" key is guaranteed to be in the dict that\u0027s returned? And we always return a dict (unlike in InMemoryErrorStats, which can return None) -- but as long as it\u0027s empty, we\u0027ll still return the default that was passed in up in ErrorLimiter.get_node_stats()...","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"22427634e513f54380ab25078a6dd118c887047e","unresolved":true,"context_lines":[{"line_number":170,"context_line":"        resp \u003d {}"},{"line_number":171,"context_line":"        for key, v_type in self.keys:"},{"line_number":172,"context_line":"            res \u003d self.memcache.get(\"{}/{}\".format(node_key, key))"},{"line_number":173,"context_line":"            if res:"},{"line_number":174,"context_line":"                resp[key] \u003d v_type(res)"},{"line_number":175,"context_line":"        return resp"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"292c96ea_c04d44d1","line":173,"in_reply_to":"e540bdc0_82eb72ee","updated":"2022-10-20 16:03:21.000000000","message":"InMemoryErrorStats changed to always return a dict","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8f845954659c6ae63f58a432bed75a33e8cb6d4d","unresolved":true,"context_lines":[{"line_number":87,"context_line":"        error_stats[\u0027errors\u0027] \u003d self.suppression_limit + 1"},{"line_number":88,"context_line":"        error_stats[\u0027last_error\u0027] \u003d time()"},{"line_number":89,"context_line":"        if self.use_memcache:"},{"line_number":90,"context_line":"            self.stats[node_key] \u003d error_stats"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def increment(self, node):"},{"line_number":93,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"f91b01c4_028a5732","line":90,"updated":"2022-10-26 13:24:49.000000000","message":"This conditional is a bit unfortunate. The only alternatives i can think of are:\n\n1. always call self.stats[node_key] \u003d error_stats, but that is not necessary when stats is a dict\n\n2. Have MemcacheStats return a dict-like object that updates memcache dynamically in its setter method.","commit_id":"cff3abf214c3d0a5a36ed9f362c8047447297c2a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8f845954659c6ae63f58a432bed75a33e8cb6d4d","unresolved":true,"context_lines":[{"line_number":173,"context_line":"        for schema_key, _ in self.schema.items():"},{"line_number":174,"context_line":"            if schema_key in error_stats:"},{"line_number":175,"context_line":"                self.memcache.set(self._make_key(key, schema_key),"},{"line_number":176,"context_line":"                                  error_stats[schema_key])"}],"source_content_type":"text/x-python","patch_set":28,"id":"b3619f7c_11245585","line":176,"updated":"2022-10-26 13:24:49.000000000","message":"why do the items get stored separately in memcache, rather than storing the entire dict under the node key?\n\nConversely, if we\u0027re storing the items separately then can we disable json (de)serialization on the way to(from) memcache?","commit_id":"cff3abf214c3d0a5a36ed9f362c8047447297c2a"}],"swift/proxy/server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":658,"context_line":"                \u0027Node error limited %(ip)s:%(port)s (%(device)s)\u0027, node)"},{"line_number":659,"context_line":"        return limited"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"    def error_limit(self, node, msg):"},{"line_number":662,"context_line":"        \"\"\""},{"line_number":663,"context_line":"        Mark a node as error limited. This immediately pretends the"},{"line_number":664,"context_line":"        node received enough errors to trigger error suppression. Use"}],"source_content_type":"text/x-python","patch_set":5,"id":"6cc4ae36_7116fe0a","side":"PARENT","line":661,"updated":"2021-12-08 15:49:26.000000000","message":"maybe leave these public methods on the app as a shim to self.error_limter.error_limit (less churn)","commit_id":"6fd8d6f11da615bc8d7f51a58548d32c89c6f1e5"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9929f44c80f98c0a6f5d3e48157b6572494f80b9","unresolved":false,"context_lines":[{"line_number":658,"context_line":"                \u0027Node error limited %(ip)s:%(port)s (%(device)s)\u0027, node)"},{"line_number":659,"context_line":"        return limited"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"    def error_limit(self, node, msg):"},{"line_number":662,"context_line":"        \"\"\""},{"line_number":663,"context_line":"        Mark a node as error limited. This immediately pretends the"},{"line_number":664,"context_line":"        node received enough errors to trigger error suppression. Use"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf730274_364a074e","side":"PARENT","line":661,"in_reply_to":"6cc4ae36_7116fe0a","updated":"2022-10-12 12:48:37.000000000","message":"Done","commit_id":"6fd8d6f11da615bc8d7f51a58548d32c89c6f1e5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"662db73b33ffb27cdd1bc1725b85656ecdaa44b6","unresolved":true,"context_lines":[{"line_number":636,"context_line":"        :param node: dictionary of node to handle errors for"},{"line_number":637,"context_line":"        :param msg: error message"},{"line_number":638,"context_line":"        \"\"\""},{"line_number":639,"context_line":"        self.error_limiter.incr_node_errors(node)"},{"line_number":640,"context_line":"        if isinstance(msg, bytes):"},{"line_number":641,"context_line":"            msg \u003d msg.decode(\u0027utf-8\u0027)"},{"line_number":642,"context_line":"        self.logger.error(\u0027%(msg)s %(ip)s:%(port)s/%(device)s\u0027, {"}],"source_content_type":"text/x-python","patch_set":5,"id":"f6e0c9a1_6cb49a07","line":639,"updated":"2021-12-08 15:49:26.000000000","message":"could error_limiter keep this incr_node_errors method private if it had a public error_occurred method?","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9929f44c80f98c0a6f5d3e48157b6572494f80b9","unresolved":false,"context_lines":[{"line_number":636,"context_line":"        :param node: dictionary of node to handle errors for"},{"line_number":637,"context_line":"        :param msg: error message"},{"line_number":638,"context_line":"        \"\"\""},{"line_number":639,"context_line":"        self.error_limiter.incr_node_errors(node)"},{"line_number":640,"context_line":"        if isinstance(msg, bytes):"},{"line_number":641,"context_line":"            msg \u003d msg.decode(\u0027utf-8\u0027)"},{"line_number":642,"context_line":"        self.logger.error(\u0027%(msg)s %(ip)s:%(port)s/%(device)s\u0027, {"}],"source_content_type":"text/x-python","patch_set":5,"id":"a1a42620_426f9d67","line":639,"in_reply_to":"81ba8129_bafa9c68","updated":"2022-10-12 12:48:37.000000000","message":"Done","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3803c465df1ccd4422c911f2fdd095ce32083198","unresolved":true,"context_lines":[{"line_number":636,"context_line":"        :param node: dictionary of node to handle errors for"},{"line_number":637,"context_line":"        :param msg: error message"},{"line_number":638,"context_line":"        \"\"\""},{"line_number":639,"context_line":"        self.error_limiter.incr_node_errors(node)"},{"line_number":640,"context_line":"        if isinstance(msg, bytes):"},{"line_number":641,"context_line":"            msg \u003d msg.decode(\u0027utf-8\u0027)"},{"line_number":642,"context_line":"        self.logger.error(\u0027%(msg)s %(ip)s:%(port)s/%(device)s\u0027, {"}],"source_content_type":"text/x-python","patch_set":5,"id":"81ba8129_bafa9c68","line":639,"in_reply_to":"f6e0c9a1_6cb49a07","updated":"2021-12-09 04:27:34.000000000","message":"What about exception_occured below. I worry we take something fundemental as dealing with errors and exceptions for the proxy and move it to an error limiter where it might not make logical sense.\n\nMaybe incre_node_errors isn\u0027t the best name for it though, it could be renamed error_occured as in an ErrorLimiter I\u0027d expect an icrement on an error_occured more then calling something called increment.","commit_id":"5a8becad59f61574be82152f081a06bb23cb55a1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9929f44c80f98c0a6f5d3e48157b6572494f80b9","unresolved":true,"context_lines":[{"line_number":222,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60)),"},{"line_number":223,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10)),"},{"line_number":224,"context_line":"            use_memcache\u003dconfig_true_value("},{"line_number":225,"context_line":"                conf.get(\u0027error_limiting_use_memcache\u0027, False)),"},{"line_number":226,"context_line":"            limiting_pool\u003dconf.get(\u0027error_limiting_pool\u0027)"},{"line_number":227,"context_line":"        )"},{"line_number":228,"context_line":"        self.recheck_container_existence \u003d \\"}],"source_content_type":"text/x-python","patch_set":23,"id":"e78ba178_01bfe730","line":225,"range":{"start_line":225,"start_character":16,"end_line":225,"end_character":24},"updated":"2022-10-12 12:48:37.000000000","message":"note: we\u0027re not just passing conf through because we need to strip off the \u0027error_\u0027 prefix from the conf options","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9929f44c80f98c0a6f5d3e48157b6572494f80b9","unresolved":true,"context_lines":[{"line_number":223,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10)),"},{"line_number":224,"context_line":"            use_memcache\u003dconfig_true_value("},{"line_number":225,"context_line":"                conf.get(\u0027error_limiting_use_memcache\u0027, False)),"},{"line_number":226,"context_line":"            limiting_pool\u003dconf.get(\u0027error_limiting_pool\u0027)"},{"line_number":227,"context_line":"        )"},{"line_number":228,"context_line":"        self.recheck_container_existence \u003d \\"},{"line_number":229,"context_line":"            int(conf.get(\u0027recheck_container_existence\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"e93b7cfe_a4ce5d1b","line":226,"range":{"start_line":226,"start_character":12,"end_line":226,"end_character":25},"updated":"2022-10-12 12:48:37.000000000","message":"I wonder if this might be better named \u0027memcache_prefix\u0027?? (memcache_pool probably has other connotations)","commit_id":"051323c10f9ecfb7ad0fc0381200e6ec3407fe02"}],"test/unit/common/test_error_limiter.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7cf7b27b9aea9e3b7d4e434f1f26befe4e6b2e49","unresolved":true,"context_lines":[{"line_number":251,"context_line":"                    error_limiter.InMemoryErrorLimiter({}, self.logger),"},{"line_number":252,"context_line":"                    error_limiter.MemcacheErrorLimiter({}, self.logger)):"},{"line_number":253,"context_line":"                self.logger.clear()"},{"line_number":254,"context_line":"                do_test(node, limiter)"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    def test_get_recent_error_limited_nodes(self):"},{"line_number":257,"context_line":"        now \u003d time()"}],"source_content_type":"text/x-python","patch_set":14,"id":"fdebafba_3145e776","line":254,"updated":"2022-09-22 17:18:36.000000000","message":"I could imagine this test module would benifit from a BaseTestErrorLimiter class that requires a setUp that creates a different self.error_limiter\n\nobviously some of the tests are specific to the impl - but some tests should be able to run on either","commit_id":"fc171d60220ea75a7f92d9aefb0b58d029387f44"}]}