)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2026cf4c_8521d5ea","updated":"2023-08-28 16:00:53.000000000","message":"There\u0027s some things I like about this direction, and some I don\u0027t\n\nI\u0027m not sure how useful the interface is at this point - I\u0027m guessing since most of the lookup/normalize behavior is INJECTED into the helper that it will be fairly adaptable; but I\u0027d like to try it out with the a/c info lookup/skip/fallback code paths as well.\n\nThe closures/nonlocal might be a bit trixy for some folks aesthetics; but I think consolidating the cache strategy into a re-usable pattern has some merit that might be worth some compromise.\n\nNeeds more polish, and the new utils function needs some targeted tests (once we agree on the interface)","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":2581,"context_line":"        if value is not None:"},{"line_number":2582,"context_line":"            infocache[cache_key] \u003d value"},{"line_number":2583,"context_line":"            memcache_set_f(memcache, cache_key, value)"},{"line_number":2584,"context_line":"    return value, cache_state"},{"line_number":2585,"context_line":""},{"line_number":2586,"context_line":""},{"line_number":2587,"context_line":"def read_conf_dir(parser, conf_dir):"}],"source_content_type":"text/x-python","patch_set":1,"id":"165cd182_d2127e09","line":2584,"updated":"2023-08-28 16:00:53.000000000","message":"I like that the infocache -\u003e memcache (skip) -\u003e backend fallback log fits in 30 lines and seems somewhat generic\n\nI don\u0027t LOVE that the memcache get/set functions need to be injected (mainly for extra logging and normalization)\n\nI\u0027m not sure if the \"cache_state\" string-enum should be part of the interface","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":290,"context_line":"                         :class:`swift.common.memcached.MemcacheRing`."},{"line_number":291,"context_line":"        :param cache_key: the cache key for both infocache and memcache."},{"line_number":292,"context_line":"        :return: a tuple of (an instance of NamespaceBoundList, cache state)"},{"line_number":293,"context_line":"        \"\"\""},{"line_number":294,"context_line":"        # try get namespaces from infocache first"},{"line_number":295,"context_line":"        namespace_list \u003d infocache.get(cache_key)"},{"line_number":296,"context_line":"        if namespace_list:"}],"source_content_type":"text/x-python","patch_set":1,"id":"72d81990_58b6bdca","side":"PARENT","line":293,"updated":"2023-08-28 16:00:53.000000000","message":"I think this method might be un-used now","commit_id":"f06e5369579599648cc78e4b556887bc6d978c2b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":392,"context_line":"                    memcache.set("},{"line_number":393,"context_line":"                        cache_key, cached_namespaces.bounds,"},{"line_number":394,"context_line":"                        time\u003dself.app.recheck_updating_shard_ranges)"},{"line_number":395,"context_line":"            update_shard \u003d find_namespace(obj, shard_ranges or [])"},{"line_number":396,"context_line":"        record_cache_op_metrics("},{"line_number":397,"context_line":"            self.logger, \u0027shard_updating\u0027, cache_state, response)"},{"line_number":398,"context_line":"        return update_shard"}],"source_content_type":"text/x-python","patch_set":1,"id":"099e117e_e72224d8","side":"PARENT","line":395,"updated":"2023-08-28 16:00:53.000000000","message":"i find it akward that we call this method with a potentially empty list","commit_id":"f06e5369579599648cc78e4b556887bc6d978c2b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            or None if the update should go back to the root"},{"line_number":358,"context_line":"        \"\"\""},{"line_number":359,"context_line":"        memcache \u003d cache_from_env(req.environ, True)"},{"line_number":360,"context_line":"        if not (self.app.recheck_updating_shard_ranges and memcache):"},{"line_number":361,"context_line":"            # caching is disabled"},{"line_number":362,"context_line":"            return self._get_update_shard_caching_disabled("},{"line_number":363,"context_line":"                req, account, container, obj)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d1eba4f1_86d512bc","line":360,"updated":"2023-08-28 16:00:53.000000000","message":"this is a significant change, I\u0027d suggest that we don\u0027t need to fetch the full shard range list just to stash in infocache if memcache is disabled","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":385,"context_line":"                    cached_namespaces \u003d NamespaceBoundList(value)"},{"line_number":386,"context_line":"                else:"},{"line_number":387,"context_line":"                    cache_state \u003d \u0027miss\u0027"},{"line_number":388,"context_line":"                    cached_namespaces \u003d None"},{"line_number":389,"context_line":"            return cached_namespaces, cache_state"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        def set_in_memcache(memcache, cache_key, cached_namespaces):"}],"source_content_type":"text/x-python","patch_set":1,"id":"4568a396_c9022e65","line":388,"updated":"2023-08-28 16:00:53.000000000","message":"I like that the error and miss case clearly show the returned \"cached_namespaces\" value is None","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":398,"context_line":"                time\u003dself.app.recheck_updating_shard_ranges)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        def get_from_backend():"},{"line_number":401,"context_line":"            nonlocal response"},{"line_number":402,"context_line":"            shard_ranges, response \u003d self._get_shard_ranges("},{"line_number":403,"context_line":"                req, account, container, states\u003d\u0027updating\u0027)"},{"line_number":404,"context_line":"            return NamespaceBoundList.parse(shard_ranges)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2dd43c60_594ff246","line":401,"updated":"2023-08-28 16:00:53.000000000","message":"the nonlocal to capture response doesn\u0027t bother me as much as I thought it would","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"}],"test/unit/proxy/test_server.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":4624,"context_line":"            req \u003d Request.blank(\u0027/v1/a/c/o\u0027, {"},{"line_number":4625,"context_line":"                    \u0027swift.infocache\u0027: infocache,"},{"line_number":4626,"context_line":"                    # w/o memcache target shard uses direct includes query"},{"line_number":4627,"context_line":"                    \u0027swift.cache\u0027: FakeMemcache(),"},{"line_number":4628,"context_line":"                },"},{"line_number":4629,"context_line":"                method\u003dmethod, body\u003d\u0027\u0027,"},{"line_number":4630,"context_line":"                headers\u003d{\u0027Content-Type\u0027: \u0027text/plain\u0027})"}],"source_content_type":"text/x-python","patch_set":1,"id":"f4e5eaa3_3614f7b6","line":4627,"updated":"2023-08-28 16:00:53.000000000","message":"I think using infocache w/o memcache is somewhat suspect, but could probably make it work with sufficient \"if memcache\" guards - assuming we agree it\u0027s a needed use-case","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":4648,"context_line":"                \u0027account.info.cache.miss.200\u0027: 1,"},{"line_number":4649,"context_line":"                \u0027account.info.infocache.hit\u0027: 1,"},{"line_number":4650,"context_line":"                \u0027container.info.cache.miss.200\u0027: 1,"},{"line_number":4651,"context_line":"                \u0027container.info.infocache.hit\u0027: 1,"},{"line_number":4652,"context_line":"                \u0027object.shard_updating.infocache.hit\u0027: 1,"},{"line_number":4653,"context_line":"            }, stats)"},{"line_number":4654,"context_line":"            # verify statsd prefix is not mutated"}],"source_content_type":"text/x-python","patch_set":1,"id":"7b19edcb_c548dcbd","line":4651,"updated":"2023-08-28 16:00:53.000000000","message":"it\u0027s unlcear why we get the cache.miss stats AND the infocache.hit stat","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0b71ea7673a975ae42f1e3f8a0fd816a0639ea8f","unresolved":true,"context_lines":[{"line_number":4855,"context_line":"                \u0027/v1/a/c/o\u0027, {\u0027swift.cache\u0027: cache},"},{"line_number":4856,"context_line":"                method\u003dmethod, body\u003d\u0027\u0027, headers\u003d{\u0027Content-Type\u0027: \u0027text/plain\u0027})"},{"line_number":4857,"context_line":"            cache.error_on_get \u003d [False, True]"},{"line_number":4858,"context_line":"            with mock.patch(\u0027swift.common.utils.random\u0027, return_value\u003d1.0), \\"},{"line_number":4859,"context_line":"                    mocked_http_conn(*status_codes, headers\u003dresp_headers,"},{"line_number":4860,"context_line":"                                     body\u003dbody):"},{"line_number":4861,"context_line":"                resp \u003d req.get_response(self.app)"}],"source_content_type":"text/x-python","patch_set":1,"id":"48a02bf4_bc0122b2","line":4858,"updated":"2023-08-28 16:00:53.000000000","message":"the utils module imports random by name, it might be reasonable to fix that by importing the module in utils; but this is actually more targeted.","commit_id":"430114f85f201349beb9c8bada2f6f750c811394"}]}