)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64a2ac29730d83887aff02cf72ee9ba0c35daa38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"da6a8cc1_3b0624d6","updated":"2022-09-22 17:28:56.000000000","message":"ok, i went to look and can now see where the memcache comes from in the end - but in the intim it still seems like there\u0027s something wonky going on here.\n\nI think I like splitting the patches up ... there\u0027s LOTS of things useful to consider just in this re-factor and how we want to evolve the per-worker in memory error limiting and it\u0027s configuration that have nothing to do with global cluster error limiting using memcache.  But if we\u0027re going that route we probably have to go all in - if the goal is really just to get the memcache impl maybe it\u0027s faster to work on it all together.  Dunno.","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7d9965ae8140ae8783033f6b9c8aeddfe036e252","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3b2b20af_6beb6331","updated":"2022-09-23 14:14:02.000000000","message":"with this patchset I only fixed the memcache references in test_error_limiter.py, other issues are WIP","commit_id":"618fbd769f84b01c0a687f500fde3d65d522d742"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"6a696b46ac95ddbadbca6ec570be4876e10b71a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"0d0913ea_1683f44c","updated":"2022-10-12 09:52:04.000000000","message":"thanks for reviews!\n\nI\u0027m trying to keep the ErrorLimiter \u0027generic\u0027 hence keeping logging in the proxy app. IF (?) we manage to re-use ErrorLimiter to handle 529s we may not want the same logging (or any - there could be many 529s).","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"752b37e4_259595e9","updated":"2022-10-12 09:23:56.000000000","message":"thanks for the reviews!\n\nBy way of explanation, in addition to making the existing code a little more modular and (I think?) increasing test coverage - mostly Matt\u0027s original contribution here - I am also hoping that we may be able to re-use the ErrorLimiter class. Hence some of my choices about the separation of concerns: the ErrorLimiter manages the counts vs the proxy app still does the logging.\n\n","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2f0519447520ff99bb45144b049a2d10fdd548f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"85e28cab_a20b600e","updated":"2022-10-11 15:37:33.000000000","message":"this all looks pretty good!  curious if we want to maintain/re-use the existing interface on the server/app - or if we want to use the app.error_limiter instance directly (and move logging into the error limiter?!)","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2e79a3d248f49c8d0002831977de0dddc9ab1fd6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"f624ea49_9adb5d4f","updated":"2022-10-19 03:23:49.000000000","message":"I think there may be some things here that aren\u0027t really used until later in the chain -- that\u0027s OK, as long as we expect the rest to land \"soon\" -- I\u0027ll keep looking at the rest to get a sense of it.","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c0760a64212de4166781d329cfe6c7676e27ffe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"99688b71_207eb3c5","updated":"2022-10-12 11:36:24.000000000","message":"I\u0027ve moved the conf option parsing and defaults back to the proxy server app, because they are specific to the prosy error handling (507/timeouts) and would be different if we re-use the ErrorLimiter class for other \u0027error\u0027 types.","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"635af0d21f13703f5510a6d77698222ba28e346f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"f22e38a0_03391ae4","updated":"2022-10-18 13:30:42.000000000","message":"recheck","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5d82b02f54918cc7b9c584c2cb841cf254f58d37","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"bfe61c1a_433b5535","updated":"2022-10-25 18:27:09.000000000","message":"Sorry, I\u0027m probably bikeshedding...","commit_id":"1a1d215a09107d2d0e5b79e1dc45f8c8d6998fe3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d310899a84dcfce282dd536592e342957666ddb1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"fe323324_a0986028","updated":"2022-10-27 17:39:41.000000000","message":"recheck\n\nprobe test failure should be fixed by https://review.opendev.org/c/openstack/swift/+/862758","commit_id":"51730f127304071f51cf95cf8fed3e75539265ed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c7089da4bee05b9bc519587bfcec428e8ab72aaa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"7f172d6b_463b122f","updated":"2022-10-28 06:18:54.000000000","message":"recheck\n\npy3 probe test failure seems like it could just be eventual consistency?\n\nDefinitely still better to have it pulled out to a class like this -- I\u0027m still not sure about what the ErrorLimiter \u003c-\u003e Stats api should look like, but maybe it\u0027s better for us to land and iterate on it as we get further out the chain.","commit_id":"51730f127304071f51cf95cf8fed3e75539265ed"}],"swift/common/error_limiter.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64a2ac29730d83887aff02cf72ee9ba0c35daa38","unresolved":true,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class BaseErrorLimiter(object):"},{"line_number":23,"context_line":"    def __init__(self, conf, logger):"},{"line_number":24,"context_line":"        self.logger \u003d logger"},{"line_number":25,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":26,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":2,"id":"6f67aba0_93e52865","line":23,"updated":"2022-09-22 17:28:56.000000000","message":"is logger required, I wonder if you could get_logger off the conf if you don\u0027t have one.","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class BaseErrorLimiter(object):"},{"line_number":23,"context_line":"    def __init__(self, conf, logger):"},{"line_number":24,"context_line":"        self.logger \u003d logger"},{"line_number":25,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":26,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":2,"id":"fc910f7a_55b02aff","line":23,"in_reply_to":"45e2fbe6_1b11c536","updated":"2022-10-12 09:23:56.000000000","message":"Done","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5a4e1dc9d107f2a89732b1172626a0b43a2e450b","unresolved":true,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class BaseErrorLimiter(object):"},{"line_number":23,"context_line":"    def __init__(self, conf, logger):"},{"line_number":24,"context_line":"        self.logger \u003d logger"},{"line_number":25,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":26,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":2,"id":"f4e0cad9_39b507cc","line":23,"in_reply_to":"6f67aba0_93e52865","updated":"2022-09-23 10:30:54.000000000","message":"I\u0027ve have some WIP on logger passing. This patchset was initial cut/paste split up of Matt\u0027s original patch.","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29bb7a9a40d9f60ba1ea405b85942dcd4d7b7885","unresolved":true,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class BaseErrorLimiter(object):"},{"line_number":23,"context_line":"    def __init__(self, conf, logger):"},{"line_number":24,"context_line":"        self.logger \u003d logger"},{"line_number":25,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":26,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"}],"source_content_type":"text/x-python","patch_set":2,"id":"45e2fbe6_1b11c536","line":23,"in_reply_to":"f4e0cad9_39b507cc","updated":"2022-09-23 15:17:55.000000000","message":"logger handling is still WIP","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64a2ac29730d83887aff02cf72ee9ba0c35daa38","unresolved":true,"context_lines":[{"line_number":103,"context_line":"        raise NotImplementedError()"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"class InMemoryErrorLimiter(BaseErrorLimiter):"},{"line_number":107,"context_line":"    def __init__(self, conf, logger):"},{"line_number":108,"context_line":"        super(InMemoryErrorLimiter, self).__init__(conf, logger)"},{"line_number":109,"context_line":"        self._error_limiting \u003d {}"}],"source_content_type":"text/x-python","patch_set":2,"id":"674fc983_f6a82a32","line":106,"updated":"2022-09-22 17:28:56.000000000","message":"I\u0027m skeptical about the base class here - unless another concreate implementation shows up in the next patch I\u0027d want to push to leave it out.","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5a4e1dc9d107f2a89732b1172626a0b43a2e450b","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        raise NotImplementedError()"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"class InMemoryErrorLimiter(BaseErrorLimiter):"},{"line_number":107,"context_line":"    def __init__(self, conf, logger):"},{"line_number":108,"context_line":"        super(InMemoryErrorLimiter, self).__init__(conf, logger)"},{"line_number":109,"context_line":"        self._error_limiting \u003d {}"}],"source_content_type":"text/x-python","patch_set":2,"id":"4b035cf7_c2a2f588","line":106,"in_reply_to":"674fc983_f6a82a32","updated":"2022-09-23 10:30:54.000000000","message":"Ack. this will be because I lifted the file from Matt\u0027s original patch which came after memcache error limiter was a thing, then deleted the memcache error limiter.\n\n#willfix","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29bb7a9a40d9f60ba1ea405b85942dcd4d7b7885","unresolved":true,"context_lines":[{"line_number":15,"context_line":"from time import time"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"class BaseErrorLimiter(object):"},{"line_number":19,"context_line":"    def __init__(self, conf, logger):"},{"line_number":20,"context_line":"        self.logger \u003d logger"},{"line_number":21,"context_line":"        self.error_suppression_interval \u003d \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"79d74d9a_728ebb39","line":18,"range":{"start_line":18,"start_character":6,"end_line":18,"end_character":10},"updated":"2022-09-23 15:17:55.000000000","message":"maybe Base is no longer appropriate, but I bet we *are* going to extend this class at some point ;-)","commit_id":"e024e45003ced9f19dccab2165a690f68b81e810"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[{"line_number":15,"context_line":"from time import time"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"class BaseErrorLimiter(object):"},{"line_number":19,"context_line":"    def __init__(self, conf, logger):"},{"line_number":20,"context_line":"        self.logger \u003d logger"},{"line_number":21,"context_line":"        self.error_suppression_interval \u003d \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"d24eee12_46106815","line":18,"range":{"start_line":18,"start_character":6,"end_line":18,"end_character":10},"in_reply_to":"79d74d9a_728ebb39","updated":"2022-10-12 09:23:56.000000000","message":"Done","commit_id":"e024e45003ced9f19dccab2165a690f68b81e810"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29bb7a9a40d9f60ba1ea405b85942dcd4d7b7885","unresolved":true,"context_lines":[{"line_number":24,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":25,"context_line":"        self.error_limiting_prefix \u003d \u0027\u0027"},{"line_number":26,"context_line":"        self._error_limited \u003d {}"},{"line_number":27,"context_line":"        self.stats \u003d InMemoryErrorStats()"},{"line_number":28,"context_line":"        self.error_limiting_prefix \u003d conf.get(\u0027error_limiting_prefix\u0027, \u0027\u0027)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def node_key(self, node):"}],"source_content_type":"text/x-python","patch_set":4,"id":"d7d0ab6d_472cf0ae","line":27,"range":{"start_line":27,"start_character":20,"end_line":27,"end_character":41},"updated":"2022-09-23 15:17:55.000000000","message":"using a separate class to wrap a dict only makes sense later in the patch chain when the MemcacheErrorStats class is added","commit_id":"e024e45003ced9f19dccab2165a690f68b81e810"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[{"line_number":24,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":25,"context_line":"        self.error_limiting_prefix \u003d \u0027\u0027"},{"line_number":26,"context_line":"        self._error_limited \u003d {}"},{"line_number":27,"context_line":"        self.stats \u003d InMemoryErrorStats()"},{"line_number":28,"context_line":"        self.error_limiting_prefix \u003d conf.get(\u0027error_limiting_prefix\u0027, \u0027\u0027)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def node_key(self, node):"}],"source_content_type":"text/x-python","patch_set":4,"id":"b7372b5e_c9076279","line":27,"range":{"start_line":27,"start_character":20,"end_line":27,"end_character":41},"in_reply_to":"d7d0ab6d_472cf0ae","updated":"2022-10-12 09:23:56.000000000","message":"Ack","commit_id":"e024e45003ced9f19dccab2165a690f68b81e810"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":true,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"class ErrorLimiter(object):"},{"line_number":19,"context_line":"    def __init__(self, conf):"},{"line_number":20,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":21,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"},{"line_number":22,"context_line":"        self.error_suppression_limit \u003d \\"},{"line_number":23,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":24,"context_line":"        self._error_limited \u003d {}"},{"line_number":25,"context_line":"        self.stats \u003d InMemoryErrorStats()"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"6a569610_a4b478af","line":23,"range":{"start_line":20,"start_character":8,"end_line":23,"end_character":56},"updated":"2022-10-12 09:23:56.000000000","message":"this is not generic: these conf options are specific to 507/timeout handling","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c0760a64212de4166781d329cfe6c7676e27ffe","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"class ErrorLimiter(object):"},{"line_number":19,"context_line":"    def __init__(self, conf):"},{"line_number":20,"context_line":"        self.error_suppression_interval \u003d \\"},{"line_number":21,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"},{"line_number":22,"context_line":"        self.error_suppression_limit \u003d \\"},{"line_number":23,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":24,"context_line":"        self._error_limited \u003d {}"},{"line_number":25,"context_line":"        self.stats \u003d InMemoryErrorStats()"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"6ed9f440_9264c416","line":23,"range":{"start_line":20,"start_character":8,"end_line":23,"end_character":56},"in_reply_to":"6a569610_a4b478af","updated":"2022-10-12 11:36:24.000000000","message":"Done","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c0760a64212de4166781d329cfe6c7676e27ffe","unresolved":true,"context_lines":[{"line_number":27,"context_line":"    :param suppression_limit: The number of errors that a node must accumulate"},{"line_number":28,"context_line":"        before it is considered to be error-limited. Should be an int value."},{"line_number":29,"context_line":"    \"\"\""},{"line_number":30,"context_line":"    def __init__(self, suppression_interval, suppression_limit):"},{"line_number":31,"context_line":"        self.suppression_interval \u003d float(suppression_interval)"},{"line_number":32,"context_line":"        self.suppression_limit \u003d int(suppression_limit)"},{"line_number":33,"context_line":"        self._limited_nodes \u003d {}"}],"source_content_type":"text/x-python","patch_set":10,"id":"3d670d20_b274504e","line":30,"updated":"2022-10-12 11:36:24.000000000","message":"changed signature from last patchset","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    :param suppression_limit: The number of errors that a node must accumulate"},{"line_number":28,"context_line":"        before it is considered to be error-limited. Should be an int value."},{"line_number":29,"context_line":"    \"\"\""},{"line_number":30,"context_line":"    def __init__(self, suppression_interval, suppression_limit):"},{"line_number":31,"context_line":"        self.suppression_interval \u003d float(suppression_interval)"},{"line_number":32,"context_line":"        self.suppression_limit \u003d int(suppression_limit)"},{"line_number":33,"context_line":"        self._limited_nodes \u003d {}"}],"source_content_type":"text/x-python","patch_set":10,"id":"66587bab_b1b2f7ac","line":30,"in_reply_to":"3d670d20_b274504e","updated":"2022-10-20 15:14:24.000000000","message":"Ack","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2e79a3d248f49c8d0002831977de0dddc9ab1fd6","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    def __init__(self, suppression_interval, suppression_limit):"},{"line_number":31,"context_line":"        self.suppression_interval \u003d float(suppression_interval)"},{"line_number":32,"context_line":"        self.suppression_limit \u003d int(suppression_limit)"},{"line_number":33,"context_line":"        self._limited_nodes \u003d {}"},{"line_number":34,"context_line":"        self.stats \u003d InMemoryErrorStats()"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def node_key(self, node):"}],"source_content_type":"text/x-python","patch_set":10,"id":"eb55316d_81c662e0","line":33,"updated":"2022-10-19 03:23:49.000000000","message":"I see us setting/popping keys in this -- but when do we ever read them? I was expecting it to be used for some sort of memoization in error_limited(), but I\u0027m not seeing it...","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def __init__(self, suppression_interval, suppression_limit):"},{"line_number":31,"context_line":"        self.suppression_interval \u003d float(suppression_interval)"},{"line_number":32,"context_line":"        self.suppression_limit \u003d int(suppression_limit)"},{"line_number":33,"context_line":"        self._limited_nodes \u003d {}"},{"line_number":34,"context_line":"        self.stats \u003d InMemoryErrorStats()"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def node_key(self, node):"}],"source_content_type":"text/x-python","patch_set":10,"id":"0f17d097_57c98fc8","line":33,"in_reply_to":"bf019f9f_eb76c627","updated":"2022-10-20 15:14:24.000000000","message":"looking back, I think this crept in as part of the first refactor patchset, but you\u0027re right it is new and is not used later in patch chain, so I\u0027m going to rip it out.","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf72e9e605823cc7c5a8c4ba3b5ab43f22e905a","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    def __init__(self, suppression_interval, suppression_limit):"},{"line_number":31,"context_line":"        self.suppression_interval \u003d float(suppression_interval)"},{"line_number":32,"context_line":"        self.suppression_limit \u003d int(suppression_limit)"},{"line_number":33,"context_line":"        self._limited_nodes \u003d {}"},{"line_number":34,"context_line":"        self.stats \u003d InMemoryErrorStats()"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def node_key(self, node):"}],"source_content_type":"text/x-python","patch_set":10,"id":"bf019f9f_eb76c627","line":33,"in_reply_to":"eb55316d_81c662e0","updated":"2022-10-19 19:19:24.000000000","message":"I expected to find it by the time we got to using memcache in https://review.opendev.org/c/openstack/swift/+/820313... but nope, not yet :-/\n\nI guess we check it in tests? It\u0027s not clear to me why we don\u0027t just have\n\n def get_recent_error_limited_nodes():\n      return sorted(limiter.node_key(n) for n in self.ring.devs\n                    if limiter.error_limited(n))\n\nthough.","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2e79a3d248f49c8d0002831977de0dddc9ab1fd6","unresolved":true,"context_lines":[{"line_number":40,"context_line":"        :param node: dictionary describing a node."},{"line_number":41,"context_line":"        :return: string key."},{"line_number":42,"context_line":"        \"\"\""},{"line_number":43,"context_line":"        return \"{ip}:{port}/{device}\".format(**node)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def get_node_stats(self, node, default\u003dNone):"},{"line_number":46,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"73a7f96c_a9930acd","line":43,"updated":"2022-10-19 03:23:49.000000000","message":"This reminds me of https://review.opendev.org/c/openstack/swift/+/768668/3/swift/common/utils.py ... I should refresh that patch...","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        :param node: dictionary describing a node."},{"line_number":41,"context_line":"        :return: string key."},{"line_number":42,"context_line":"        \"\"\""},{"line_number":43,"context_line":"        return \"{ip}:{port}/{device}\".format(**node)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def get_node_stats(self, node, default\u003dNone):"},{"line_number":46,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"2911bdb2_9f768f72","line":43,"in_reply_to":"73a7f96c_a9930acd","updated":"2022-10-20 15:14:24.000000000","message":"let\u0027s make a Node class!","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ccf72e9e605823cc7c5a8c4ba3b5ab43f22e905a","unresolved":true,"context_lines":[{"line_number":54,"context_line":"        return self.stats.get(self.node_key(node)) or default"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def set_node_stats(self, node, error_stats):"},{"line_number":57,"context_line":"        \"\"\""},{"line_number":58,"context_line":"        Set the error stats for given ``node``."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        :param node: dictionary describing a node."}],"source_content_type":"text/x-python","patch_set":10,"id":"c0e2a599_8ab48797","line":57,"updated":"2022-10-19 19:19:24.000000000","message":"This is only used in tests, yeah? I\u0027m wondering whether we really want to expose an API for it at all...","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        return self.stats.get(self.node_key(node)) or default"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def set_node_stats(self, node, error_stats):"},{"line_number":57,"context_line":"        \"\"\""},{"line_number":58,"context_line":"        Set the error stats for given ``node``."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        :param node: dictionary describing a node."}],"source_content_type":"text/x-python","patch_set":10,"id":"b959cf43_84164d6a","line":57,"in_reply_to":"c0e2a599_8ab48797","updated":"2022-10-20 15:14:24.000000000","message":"agree. it turned out to be unnecessary. same for get_node_stats()","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2e79a3d248f49c8d0002831977de0dddc9ab1fd6","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        \"\"\""},{"line_number":63,"context_line":"        self.stats.set(self.node_key(node), error_stats)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def error_limited(self, node):"},{"line_number":66,"context_line":"        \"\"\""},{"line_number":67,"context_line":"        Check if the node is currently error limited."},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"90050b5b_5caa56d1","line":65,"updated":"2022-10-19 03:23:49.000000000","message":"The distinction between\n\n ErrorLimiter.error_limited(node)\n\nand\n\n ErrorLimiter.error_limit(node)\n\nseems pretty small -- maybe this would be better as\n\n ErrorLimiter.is_error_limited(node)\n\n? *shrug*","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        \"\"\""},{"line_number":63,"context_line":"        self.stats.set(self.node_key(node), error_stats)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def error_limited(self, node):"},{"line_number":66,"context_line":"        \"\"\""},{"line_number":67,"context_line":"        Check if the node is currently error limited."},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"470799af_04075a72","line":65,"in_reply_to":"90050b5b_5caa56d1","updated":"2022-10-20 15:14:24.000000000","message":"agree. I\u0027m also going to drop the error_ prefix in this class to further distinguish from the proxy app methods.","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2e79a3d248f49c8d0002831977de0dddc9ab1fd6","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":99,"context_line":"        error_stats \u003d self.get_node_stats(node, default\u003d{})"},{"line_number":100,"context_line":"        error_stats[\u0027errors\u0027] \u003d self.suppression_limit + 1"},{"line_number":101,"context_line":"        error_stats[\u0027last_error\u0027] \u003d time()"},{"line_number":102,"context_line":"        self._limited_nodes[node_key] \u003d error_stats[\u0027last_error\u0027]"},{"line_number":103,"context_line":"        self.stats.set(node_key, error_stats)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"7606acb9_ca98b29e","line":101,"updated":"2022-10-19 03:23:49.000000000","message":"Feels like we\u0027re leaking a whole lot of the ErrorStats implementation in here... would it be better for us to add a step arg to InMemoryErrorStats.increment (defaulting to 1)?","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":99,"context_line":"        error_stats \u003d self.get_node_stats(node, default\u003d{})"},{"line_number":100,"context_line":"        error_stats[\u0027errors\u0027] \u003d self.suppression_limit + 1"},{"line_number":101,"context_line":"        error_stats[\u0027last_error\u0027] \u003d time()"},{"line_number":102,"context_line":"        self._limited_nodes[node_key] \u003d error_stats[\u0027last_error\u0027]"},{"line_number":103,"context_line":"        self.stats.set(node_key, error_stats)"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"65874b6b_f41bf6fa","line":101,"in_reply_to":"7606acb9_ca98b29e","updated":"2022-10-20 15:14:24.000000000","message":"So I think the leak is actually the other way, in that InMemoryErrorStats.increment() needs to know about the keys. I\u0027m going to move the increment implementation to error_occurred()","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"64456c827fb58ed259ad002407f29589d6c670cc","unresolved":true,"context_lines":[{"line_number":32,"context_line":"    def __init__(self, suppression_interval, suppression_limit):"},{"line_number":33,"context_line":"        self.suppression_interval \u003d float(suppression_interval)"},{"line_number":34,"context_line":"        self.suppression_limit \u003d int(suppression_limit)"},{"line_number":35,"context_line":"        self.stats \u003d InMemoryStats()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def node_key(self, node):"},{"line_number":38,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"04dd7e35_94f15047","line":35,"updated":"2022-10-25 17:11:16.000000000","message":"Do we need our own class, or should we just use a defaultdict(dict)?","commit_id":"ad3f1bd93e2cb464abbf8e5ff2f5b0d2a2fe3c8b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"47cd57b5fa6e46babb7844762ec73571d2944c82","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    def __init__(self, suppression_interval, suppression_limit):"},{"line_number":33,"context_line":"        self.suppression_interval \u003d float(suppression_interval)"},{"line_number":34,"context_line":"        self.suppression_limit \u003d int(suppression_limit)"},{"line_number":35,"context_line":"        self.stats \u003d InMemoryStats()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def node_key(self, node):"},{"line_number":38,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"e2e84db2_56ca4461","line":35,"in_reply_to":"04dd7e35_94f15047","updated":"2022-10-25 18:18:34.000000000","message":"great idea","commit_id":"ad3f1bd93e2cb464abbf8e5ff2f5b0d2a2fe3c8b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5d82b02f54918cc7b9c584c2cb841cf254f58d37","unresolved":true,"context_lines":[{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        now \u003d time()"},{"line_number":55,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":56,"context_line":"        error_stats \u003d self.stats.get(node_key, {})"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        if \u0027errors\u0027 not in error_stats:"},{"line_number":59,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":14,"id":"69a56c50_ea507b6b","line":56,"updated":"2022-10-25 18:27:09.000000000","message":"Can just be\n\n error_stats \u003d self.stats[node_key]\n\nnow. (Here and elsewhere.) Or maybe we want just a plain old dict?","commit_id":"1a1d215a09107d2d0e5b79e1dc45f8c8d6998fe3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cefb15bebc340a100d18360f55304192c90f6255","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        now \u003d time()"},{"line_number":55,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":56,"context_line":"        error_stats \u003d self.stats.get(node_key, {})"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        if \u0027errors\u0027 not in error_stats:"},{"line_number":59,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":14,"id":"95d3e14d_055d23d2","line":56,"in_reply_to":"69a56c50_ea507b6b","updated":"2022-10-26 10:12:14.000000000","message":"Here we do *not* want to automatically create an item in stats if it does not already exist, since this is lookup only. Hence, we use get().","commit_id":"1a1d215a09107d2d0e5b79e1dc45f8c8d6998fe3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"cefb15bebc340a100d18360f55304192c90f6255","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        :param node: dictionary of node to error limit"},{"line_number":75,"context_line":"        \"\"\""},{"line_number":76,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":77,"context_line":"        error_stats \u003d self.stats.get(node_key, {})"},{"line_number":78,"context_line":"        error_stats[\u0027errors\u0027] \u003d self.suppression_limit + 1"},{"line_number":79,"context_line":"        error_stats[\u0027last_error\u0027] \u003d time()"},{"line_number":80,"context_line":"        self.stats[node_key] \u003d error_stats"}],"source_content_type":"text/x-python","patch_set":14,"id":"2ac1d879_962bd25b","line":77,"updated":"2022-10-26 10:12:14.000000000","message":"here we could use \n\n  self.stats.get[node_key]\n  \nwhich will add a default entry to stats, but I\u0027ve been distracted from that approach by the fact that we line set anyway 80:\n\n  self.stats[node_key] \u003d error_stats\n  \nThe set is actually only necessary when we use something other than a dict (i.e. memcache) in a later patch. I think I am going to remove the set here and add it when required in the later patch.","commit_id":"1a1d215a09107d2d0e5b79e1dc45f8c8d6998fe3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d69c93085e36e0fd5a899d2f32fc6ff580959daf","unresolved":true,"context_lines":[{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        now \u003d time()"},{"line_number":55,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":56,"context_line":"        error_stats \u003d self.stats.get(node_key)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        if error_stats is None or \u0027errors\u0027 not in error_stats:"},{"line_number":59,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":15,"id":"021e6849_7f7360c8","line":56,"updated":"2022-10-28 08:21:09.000000000","message":"I think maybe I prefer using a dict so that it is more obvious when we set a default as a side effect of get and when we don\u0027t.","commit_id":"51730f127304071f51cf95cf8fed3e75539265ed"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"086fbad8113981ccd218d8ef860b8d8729ed9983","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        now \u003d time()"},{"line_number":55,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":56,"context_line":"        error_stats \u003d self.stats.get(node_key)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        if error_stats is None or \u0027errors\u0027 not in error_stats:"},{"line_number":59,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":15,"id":"958e7bf8_79ebf9cf","line":56,"in_reply_to":"021e6849_7f7360c8","updated":"2022-10-31 11:28:53.000000000","message":"scratch that - using a dict would take us back to having a call to setdefault() here and then a future memcache stats class would need to implement that, which would not make a lot of sense given that the memcache client isn\u0027t dynamically updating the stored stats (what does setting a default mean in context of memcache? writing an empty dict to memcache??).\n\nBut if we switch to a dict here and use get() followed by set() then the obvious criticism is that we should use setdefault()!\n\nFor now, I\u0027m happy to merge this and revisit if necessary once we are focussed on the memcache stats class.","commit_id":"51730f127304071f51cf95cf8fed3e75539265ed"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c7089da4bee05b9bc519587bfcec428e8ab72aaa","unresolved":true,"context_lines":[{"line_number":76,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":77,"context_line":"        error_stats \u003d self.stats[node_key]"},{"line_number":78,"context_line":"        error_stats[\u0027errors\u0027] \u003d self.suppression_limit + 1"},{"line_number":79,"context_line":"        error_stats[\u0027last_error\u0027] \u003d time()"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def increment(self, node):"},{"line_number":82,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"15f4412a_05cd15c5","line":79,"updated":"2022-10-28 06:18:54.000000000","message":"Feels a little spooky -- I kinda liked the explicit set... IDK, I keep going back and forth.","commit_id":"51730f127304071f51cf95cf8fed3e75539265ed"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d69c93085e36e0fd5a899d2f32fc6ff580959daf","unresolved":true,"context_lines":[{"line_number":76,"context_line":"        node_key \u003d self.node_key(node)"},{"line_number":77,"context_line":"        error_stats \u003d self.stats[node_key]"},{"line_number":78,"context_line":"        error_stats[\u0027errors\u0027] \u003d self.suppression_limit + 1"},{"line_number":79,"context_line":"        error_stats[\u0027last_error\u0027] \u003d time()"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def increment(self, node):"},{"line_number":82,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"a3da9184_bd43f55a","line":79,"in_reply_to":"15f4412a_05cd15c5","updated":"2022-10-28 08:21:09.000000000","message":"It\u0027s unfortunate that dict has no set or add method. I considered using update, but that just looks silly. So we either revert to another dict-wrapping class here (which I now feel is unnecessary) or accept that IF the memcache patch merges then the memcache dict wrapper will need a __setitem__ method, or we\u0027ll add an in memory dict-wrapping class then.","commit_id":"51730f127304071f51cf95cf8fed3e75539265ed"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2f0519447520ff99bb45144b049a2d10fdd548f6","unresolved":true,"context_lines":[{"line_number":1920,"context_line":"                            resp.read()"},{"line_number":1921,"context_line":"                    elif resp.status \u003d\u003d HTTP_INSUFFICIENT_STORAGE:"},{"line_number":1922,"context_line":"                        self.app.error_limit(node,"},{"line_number":1923,"context_line":"                                             \u0027ERROR Insufficient Storage\u0027)"},{"line_number":1924,"context_line":"                    elif is_server_error(resp.status):"},{"line_number":1925,"context_line":"                        msg \u003d (\u0027ERROR %(status)d Trying to \u0027"},{"line_number":1926,"context_line":"                               \u0027%(method)s %(path)s From %(type)s Server\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5be42a44_8cef9e4d","side":"PARENT","line":1923,"updated":"2022-10-11 15:37:33.000000000","message":"i think there\u0027s a couple places we could ditch this chrun and keep our keep blame a little cleaner","commit_id":"fa19868703f3f145f40468328fbaafa15ba51402"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":true,"context_lines":[{"line_number":1920,"context_line":"                            resp.read()"},{"line_number":1921,"context_line":"                    elif resp.status \u003d\u003d HTTP_INSUFFICIENT_STORAGE:"},{"line_number":1922,"context_line":"                        self.app.error_limit(node,"},{"line_number":1923,"context_line":"                                             \u0027ERROR Insufficient Storage\u0027)"},{"line_number":1924,"context_line":"                    elif is_server_error(resp.status):"},{"line_number":1925,"context_line":"                        msg \u003d (\u0027ERROR %(status)d Trying to \u0027"},{"line_number":1926,"context_line":"                               \u0027%(method)s %(path)s From %(type)s Server\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"8a9ca0ba_aa4dc56d","side":"PARENT","line":1923,"in_reply_to":"5be42a44_8cef9e4d","updated":"2022-10-12 09:23:56.000000000","message":"will fix. These are a hangover from me going to and fro with calling ErrorLimiter directly and then not.","commit_id":"fa19868703f3f145f40468328fbaafa15ba51402"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"73b835f2ca9bd5dc1640fb727246cb9be7ade887","unresolved":false,"context_lines":[{"line_number":1689,"context_line":"    def _node_gen(self):"},{"line_number":1690,"context_line":"        while self.primary_nodes:"},{"line_number":1691,"context_line":"            node \u003d self.primary_nodes.pop(0)"},{"line_number":1692,"context_line":"            if not self.app.error_limiter.error_limited(node):"},{"line_number":1693,"context_line":"                yield node"},{"line_number":1694,"context_line":"                if not self.app.error_limiter.error_limited(node):"},{"line_number":1695,"context_line":"                    self.nodes_left -\u003d 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"2418c77a_35309466","line":1692,"updated":"2022-10-12 06:02:46.000000000","message":"That\u0027s true, we have the self.app.error_limited helper method, we might as well use it. Either that or be consistent and not use the helpers and use the limiter directly.\n\nIt could even grow a logger, if we always want to encapsulate the DEBUG calls.","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2f0519447520ff99bb45144b049a2d10fdd548f6","unresolved":true,"context_lines":[{"line_number":1689,"context_line":"    def _node_gen(self):"},{"line_number":1690,"context_line":"        while self.primary_nodes:"},{"line_number":1691,"context_line":"            node \u003d self.primary_nodes.pop(0)"},{"line_number":1692,"context_line":"            if not self.app.error_limiter.error_limited(node):"},{"line_number":1693,"context_line":"                yield node"},{"line_number":1694,"context_line":"                if not self.app.error_limiter.error_limited(node):"},{"line_number":1695,"context_line":"                    self.nodes_left -\u003d 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"c8503030_606be374","line":1692,"updated":"2022-10-11 15:37:33.000000000","message":"i think we loose some debug logging bypassing the app helper method","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[{"line_number":1689,"context_line":"    def _node_gen(self):"},{"line_number":1690,"context_line":"        while self.primary_nodes:"},{"line_number":1691,"context_line":"            node \u003d self.primary_nodes.pop(0)"},{"line_number":1692,"context_line":"            if not self.app.error_limiter.error_limited(node):"},{"line_number":1693,"context_line":"                yield node"},{"line_number":1694,"context_line":"                if not self.app.error_limiter.error_limited(node):"},{"line_number":1695,"context_line":"                    self.nodes_left -\u003d 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"eb13daf2_eeaef399","line":1692,"in_reply_to":"2418c77a_35309466","updated":"2022-10-12 09:23:56.000000000","message":"argh, that\u0027s an oversight, I mean to use the app methods to get the logging.\n\nI have (for now at least) deliberately kept logging in the app so that the ErrorLimiter class remains more generic. I may be on a futile endeavour but if I can re-use the ErrorLimiter class to manage 529 responses then I probably don\u0027t want it logging on every 529 because (i) a 529 is not an error, and (ii) there could be a lot of 529s","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[{"line_number":1689,"context_line":"    def _node_gen(self):"},{"line_number":1690,"context_line":"        while self.primary_nodes:"},{"line_number":1691,"context_line":"            node \u003d self.primary_nodes.pop(0)"},{"line_number":1692,"context_line":"            if not self.app.error_limiter.error_limited(node):"},{"line_number":1693,"context_line":"                yield node"},{"line_number":1694,"context_line":"                if not self.app.error_limiter.error_limited(node):"},{"line_number":1695,"context_line":"                    self.nodes_left -\u003d 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"6cb470d2_ea1c8822","line":1692,"in_reply_to":"c8503030_606be374","updated":"2022-10-12 09:23:56.000000000","message":"Done","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2f0519447520ff99bb45144b049a2d10fdd548f6","unresolved":true,"context_lines":[{"line_number":481,"context_line":"            if response.status \u003d\u003d HTTP_INSUFFICIENT_STORAGE:"},{"line_number":482,"context_line":"                putter.failed \u003d True"},{"line_number":483,"context_line":"                self.app.error_limit(putter.node,"},{"line_number":484,"context_line":"                                     \u0027ERROR Insufficient Storage\u0027)"},{"line_number":485,"context_line":"            elif response.status \u003e\u003d HTTP_INTERNAL_SERVER_ERROR:"},{"line_number":486,"context_line":"                putter.failed \u003d True"},{"line_number":487,"context_line":"                self.app.error_occurred("}],"source_content_type":"text/x-python","patch_set":8,"id":"a280a149_fd6fedca","side":"PARENT","line":484,"updated":"2022-10-11 15:37:33.000000000","message":"if we\u0027re commited to keeping the interface, we probably don\u0027t even need to get into this file","commit_id":"fa19868703f3f145f40468328fbaafa15ba51402"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[{"line_number":481,"context_line":"            if response.status \u003d\u003d HTTP_INSUFFICIENT_STORAGE:"},{"line_number":482,"context_line":"                putter.failed \u003d True"},{"line_number":483,"context_line":"                self.app.error_limit(putter.node,"},{"line_number":484,"context_line":"                                     \u0027ERROR Insufficient Storage\u0027)"},{"line_number":485,"context_line":"            elif response.status \u003e\u003d HTTP_INTERNAL_SERVER_ERROR:"},{"line_number":486,"context_line":"                putter.failed \u003d True"},{"line_number":487,"context_line":"                self.app.error_occurred("}],"source_content_type":"text/x-python","patch_set":8,"id":"e0b6d00b_a9b951c2","side":"PARENT","line":484,"in_reply_to":"a280a149_fd6fedca","updated":"2022-10-12 09:23:56.000000000","message":"Done","commit_id":"fa19868703f3f145f40468328fbaafa15ba51402"}],"swift/proxy/server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c0760a64212de4166781d329cfe6c7676e27ffe","unresolved":true,"context_lines":[{"line_number":218,"context_line":"        self.client_chunk_size \u003d int(conf.get(\u0027client_chunk_size\u0027, 65536))"},{"line_number":219,"context_line":"        self.trans_id_suffix \u003d conf.get(\u0027trans_id_suffix\u0027, \u0027\u0027)"},{"line_number":220,"context_line":"        self.post_quorum_timeout \u003d float(conf.get(\u0027post_quorum_timeout\u0027, 0.5))"},{"line_number":221,"context_line":"        error_suppression_interval \u003d \\"},{"line_number":222,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"},{"line_number":223,"context_line":"        error_suppression_limit \u003d \\"},{"line_number":224,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":225,"context_line":"        self.error_limiter \u003d ErrorLimiter(error_suppression_interval,"},{"line_number":226,"context_line":"                                          error_suppression_limit)"},{"line_number":227,"context_line":"        self.recheck_container_existence \u003d \\"},{"line_number":228,"context_line":"            int(conf.get(\u0027recheck_container_existence\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"4a2a39b0_45dacb0c","line":225,"range":{"start_line":221,"start_character":0,"end_line":225,"end_character":1},"updated":"2022-10-12 11:36:24.000000000","message":"changed from last patchset: conf options are now parsed in the proxy app since the options, and their defaults, are specific to proxy server error handling","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        self.client_chunk_size \u003d int(conf.get(\u0027client_chunk_size\u0027, 65536))"},{"line_number":219,"context_line":"        self.trans_id_suffix \u003d conf.get(\u0027trans_id_suffix\u0027, \u0027\u0027)"},{"line_number":220,"context_line":"        self.post_quorum_timeout \u003d float(conf.get(\u0027post_quorum_timeout\u0027, 0.5))"},{"line_number":221,"context_line":"        error_suppression_interval \u003d \\"},{"line_number":222,"context_line":"            float(conf.get(\u0027error_suppression_interval\u0027, 60))"},{"line_number":223,"context_line":"        error_suppression_limit \u003d \\"},{"line_number":224,"context_line":"            int(conf.get(\u0027error_suppression_limit\u0027, 10))"},{"line_number":225,"context_line":"        self.error_limiter \u003d ErrorLimiter(error_suppression_interval,"},{"line_number":226,"context_line":"                                          error_suppression_limit)"},{"line_number":227,"context_line":"        self.recheck_container_existence \u003d \\"},{"line_number":228,"context_line":"            int(conf.get(\u0027recheck_container_existence\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"ac96a2ed_36489121","line":225,"range":{"start_line":221,"start_character":0,"end_line":225,"end_character":1},"in_reply_to":"4a2a39b0_45dacb0c","updated":"2022-10-20 15:14:24.000000000","message":"Ack","commit_id":"5908c23df4d7b44da478a805ad0e7f3d5b070ccf"}],"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":"64a2ac29730d83887aff02cf72ee9ba0c35daa38","unresolved":true,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def test_create_error_limiter(self):"},{"line_number":31,"context_line":"        with mock.patch(\u0027swift.common.memcached.MemcacheRing\u0027,"},{"line_number":32,"context_line":"                        return_value\u003dself.memcache):"},{"line_number":33,"context_line":"            el \u003d error_limiter.create_error_limiter({}, self.logger)"},{"line_number":34,"context_line":"            self.assertTrue(isinstance(el, error_limiter.InMemoryErrorLimiter))"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"cbc22239_74efbd14","line":32,"updated":"2022-09-22 17:28:56.000000000","message":"I *so* do not understand why Memcache is involved here?","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7d9965ae8140ae8783033f6b9c8aeddfe036e252","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def test_create_error_limiter(self):"},{"line_number":31,"context_line":"        with mock.patch(\u0027swift.common.memcached.MemcacheRing\u0027,"},{"line_number":32,"context_line":"                        return_value\u003dself.memcache):"},{"line_number":33,"context_line":"            el \u003d error_limiter.create_error_limiter({}, self.logger)"},{"line_number":34,"context_line":"            self.assertTrue(isinstance(el, error_limiter.InMemoryErrorLimiter))"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"39d691a8_41203d98","line":32,"in_reply_to":"9a77438f_f3b66969","updated":"2022-09-23 14:14:02.000000000","message":"Done","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5a4e1dc9d107f2a89732b1172626a0b43a2e450b","unresolved":true,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def test_create_error_limiter(self):"},{"line_number":31,"context_line":"        with mock.patch(\u0027swift.common.memcached.MemcacheRing\u0027,"},{"line_number":32,"context_line":"                        return_value\u003dself.memcache):"},{"line_number":33,"context_line":"            el \u003d error_limiter.create_error_limiter({}, self.logger)"},{"line_number":34,"context_line":"            self.assertTrue(isinstance(el, error_limiter.InMemoryErrorLimiter))"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a77438f_f3b66969","line":32,"in_reply_to":"cbc22239_74efbd14","updated":"2022-09-23 10:30:54.000000000","message":"this will be because I lifted the test file from Matt\u0027s original patch which came after memcache error limiter was a thing","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64a2ac29730d83887aff02cf72ee9ba0c35daa38","unresolved":true,"context_lines":[{"line_number":87,"context_line":"            self.assertFalse(app.error_limited(node))"},{"line_number":88,"context_line":"            app.error_limit(node, \u0027naughty naughty node!\u0027)"},{"line_number":89,"context_line":"            node_key \u003d app.node_key(node)"},{"line_number":90,"context_line":"            self.assertFalse(self.memcache.get(node_key))"},{"line_number":91,"context_line":"            self.assertIn(node_key, app._error_limiting)"},{"line_number":92,"context_line":"            self.assertEqual(app._error_limiting[node_key],"},{"line_number":93,"context_line":"                             {\u0027errors\u0027: app.error_suppression_limit + 1,"}],"source_content_type":"text/x-python","patch_set":2,"id":"55be3208_cf71f2b9","line":90,"updated":"2022-09-22 17:28:56.000000000","message":"memcache what now?","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7d9965ae8140ae8783033f6b9c8aeddfe036e252","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            self.assertFalse(app.error_limited(node))"},{"line_number":88,"context_line":"            app.error_limit(node, \u0027naughty naughty node!\u0027)"},{"line_number":89,"context_line":"            node_key \u003d app.node_key(node)"},{"line_number":90,"context_line":"            self.assertFalse(self.memcache.get(node_key))"},{"line_number":91,"context_line":"            self.assertIn(node_key, app._error_limiting)"},{"line_number":92,"context_line":"            self.assertEqual(app._error_limiting[node_key],"},{"line_number":93,"context_line":"                             {\u0027errors\u0027: app.error_suppression_limit + 1,"}],"source_content_type":"text/x-python","patch_set":2,"id":"43fedaba_5338a12b","line":90,"in_reply_to":"55be3208_cf71f2b9","updated":"2022-09-23 14:14:02.000000000","message":"Done","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2f0519447520ff99bb45144b049a2d10fdd548f6","unresolved":true,"context_lines":[{"line_number":155,"context_line":"                \u0027swift.common.error_limiter.time\u0027,"},{"line_number":156,"context_line":"                return_value\u003dnow + limiter.error_suppression_interval + 0.1):"},{"line_number":157,"context_line":"            self.assertFalse(limiter.error_limited(increment_node))"},{"line_number":158,"context_line":"            self.assertFalse(get_recent_error_limited_nodes())"}],"source_content_type":"text/x-python","patch_set":8,"id":"0e4bcab7_642838d0","line":158,"updated":"2022-10-11 15:37:33.000000000","message":"i see NO memcache!  👍","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"188e8b7f9a9dab7805071b6c2bf093180d3a5060","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                \u0027swift.common.error_limiter.time\u0027,"},{"line_number":156,"context_line":"                return_value\u003dnow + limiter.error_suppression_interval + 0.1):"},{"line_number":157,"context_line":"            self.assertFalse(limiter.error_limited(increment_node))"},{"line_number":158,"context_line":"            self.assertFalse(get_recent_error_limited_nodes())"}],"source_content_type":"text/x-python","patch_set":8,"id":"4066605c_969bf96b","line":158,"in_reply_to":"0e4bcab7_642838d0","updated":"2022-10-20 15:14:24.000000000","message":"Ack","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"}],"test/unit/proxy/controllers/test_container.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"2f0519447520ff99bb45144b049a2d10fdd548f6","unresolved":true,"context_lines":[{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        for method in (\u0027PUT\u0027, \u0027DELETE\u0027, \u0027POST\u0027):"},{"line_number":320,"context_line":"            def test_status_map(statuses, expected):"},{"line_number":321,"context_line":"                self.app.error_limiter.stats.clear()"},{"line_number":322,"context_line":"                req \u003d Request.blank(\u0027/v1/a/c\u0027, method\u003dmethod)"},{"line_number":323,"context_line":"                with mocked_http_conn(*statuses) as fake_conn:"},{"line_number":324,"context_line":"                    resp \u003d req.get_response(self.app)"}],"source_content_type":"text/x-python","patch_set":8,"id":"03d34436_59ff9b13","line":321,"updated":"2022-10-11 15:37:33.000000000","message":"oh that\u0027s nice","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0c0760a64212de4166781d329cfe6c7676e27ffe","unresolved":false,"context_lines":[{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        for method in (\u0027PUT\u0027, \u0027DELETE\u0027, \u0027POST\u0027):"},{"line_number":320,"context_line":"            def test_status_map(statuses, expected):"},{"line_number":321,"context_line":"                self.app.error_limiter.stats.clear()"},{"line_number":322,"context_line":"                req \u003d Request.blank(\u0027/v1/a/c\u0027, method\u003dmethod)"},{"line_number":323,"context_line":"                with mocked_http_conn(*statuses) as fake_conn:"},{"line_number":324,"context_line":"                    resp \u003d req.get_response(self.app)"}],"source_content_type":"text/x-python","patch_set":8,"id":"8da8a98f_3ef10990","line":321,"in_reply_to":"03d34436_59ff9b13","updated":"2022-10-12 11:36:24.000000000","message":"Ack","commit_id":"686cef497fcfa1a16090ef97253fa6abd86a9c54"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"64a2ac29730d83887aff02cf72ee9ba0c35daa38","unresolved":true,"context_lines":[{"line_number":155,"context_line":"    # particular node"},{"line_number":156,"context_line":"    node_key \u003d proxy_app.error_limiter.node_key(ring_node)"},{"line_number":157,"context_line":"    return proxy_app.error_limiter._error_limiting.get("},{"line_number":158,"context_line":"        node_key, {}).get(\u0027errors\u0027, 0)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def node_last_error(proxy_app, ring_node):"}],"source_content_type":"text/x-python","patch_set":2,"id":"091a45d8_fa8ef0bf","line":158,"updated":"2022-09-22 17:28:56.000000000","message":"I could imagine this function could potentailly become a method on error_limiter itself - and that might have some benifits if the implementation changes to use some shared storage under the hood or something...","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5a4e1dc9d107f2a89732b1172626a0b43a2e450b","unresolved":true,"context_lines":[{"line_number":155,"context_line":"    # particular node"},{"line_number":156,"context_line":"    node_key \u003d proxy_app.error_limiter.node_key(ring_node)"},{"line_number":157,"context_line":"    return proxy_app.error_limiter._error_limiting.get("},{"line_number":158,"context_line":"        node_key, {}).get(\u0027errors\u0027, 0)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def node_last_error(proxy_app, ring_node):"}],"source_content_type":"text/x-python","patch_set":2,"id":"6f2ee70b_3be5a03e","line":158,"in_reply_to":"091a45d8_fa8ef0bf","updated":"2022-09-23 10:30:54.000000000","message":"+1 similarly there\u0027s a lot of reaching into internals to reset error limiters which could maybe be supported by a method (although that is likely to be only used by tests)","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a35801b4b2bf5ab69b62bfae3e9e9d00cc579e4b","unresolved":false,"context_lines":[{"line_number":155,"context_line":"    # particular node"},{"line_number":156,"context_line":"    node_key \u003d proxy_app.error_limiter.node_key(ring_node)"},{"line_number":157,"context_line":"    return proxy_app.error_limiter._error_limiting.get("},{"line_number":158,"context_line":"        node_key, {}).get(\u0027errors\u0027, 0)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"def node_last_error(proxy_app, ring_node):"}],"source_content_type":"text/x-python","patch_set":2,"id":"915020de_1d74914d","line":158,"in_reply_to":"6f2ee70b_3be5a03e","updated":"2022-10-12 09:23:56.000000000","message":"Done","commit_id":"3bd4ab8821b6c490ee2cc87ae41cdd8cf7e18d51"}]}