)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5b937a064ae551dd9c87877e64c6eb883349ed5e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"439166c5_28c4488f","updated":"2023-11-18 01:18:23.000000000","message":"LGTM. Only need to add test cases and print out error messages.","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9f17876a6dd96a9b30d97b498990d7c4a0194e00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"21975850_efea0e49","updated":"2023-11-20 23:31:34.000000000","message":"I think this is a solid DRYing of cache handling for container-listing/object-updating of shard-range cache objects.  KUDOS!\n\nIn fact, if it wasn\u0027t for the NEW tests you added on the helper methods in test_base.py it would be a negative line count:\n\n     swift/proxy/controllers/base.py               | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n     swift/proxy/controllers/container.py          | 66 +++++++++++++++---------------------------------------------------\n     swift/proxy/controllers/obj.py                | 96 ++++++++++++++++++++----------------------------------------------------------------------------\n     test/unit/__init__.py                         |  4 ++++\n     test/unit/proxy/controllers/test_container.py | 19 +++++++++----------\n     5 files changed, 117 insertions(+), 138 deletions(-)\n\nwhich is a always a good sign for a \"pure refactor; no behavior change\"\n\nI also think all of the existing behaviors we care about were well covered by existing tests; so I think we have a strong signal this does what it says and nothing else on accident.","commit_id":"4952f91e8b4200e3f265a82f46a9196b7624ce89"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a5c695f4aebbb1c297faca40e98220f46ae9dbf4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"92c0f224_96e4ac44","updated":"2023-11-21 10:27:47.000000000","message":"I\u0027m going to move this to the start of the patch chain and maybe we can get it merged","commit_id":"4952f91e8b4200e3f265a82f46a9196b7624ce89"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"18f309c098f2cf5ba63b680f02ade662466ed02a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0087a6b6_338d9e1c","updated":"2023-11-21 16:21:46.000000000","message":"LGTM. We can merge this one first.","commit_id":"72ac5b3be0c8ac4c3465c9b7935d5daf443640a6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b4ad1af9e81b8d3cdbb028a5331747a522b6c5d8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1b15170f_25e31f0c","updated":"2023-11-21 16:59:45.000000000","message":"yay progress!","commit_id":"72ac5b3be0c8ac4c3465c9b7935d5daf443640a6"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5b937a064ae551dd9c87877e64c6eb883349ed5e","unresolved":true,"context_lines":[{"line_number":890,"context_line":"    return info, cache_state"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":""},{"line_number":893,"context_line":"def get_namespaces_from_cache(req, cache_key, skip_chance):"},{"line_number":894,"context_line":"    \"\"\""},{"line_number":895,"context_line":"    Get cached namespaces from infocache or memcache."},{"line_number":896,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"adb763bb_ac591c30","line":893,"updated":"2023-11-18 01:18:23.000000000","message":"Nice abstraction! listing and updating can now reuse the same functions. I feel we would need test cases for new functions, let me know if you need help.","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"95da5dab1e48777af6cfcaf71eaba01add5ac39b","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    return info, cache_state"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":""},{"line_number":893,"context_line":"def get_namespaces_from_cache(req, cache_key, skip_chance):"},{"line_number":894,"context_line":"    \"\"\""},{"line_number":895,"context_line":"    Get cached namespaces from infocache or memcache."},{"line_number":896,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf2a8ead_e97ffd61","line":893,"in_reply_to":"adb763bb_ac591c30","updated":"2023-11-20 12:11:45.000000000","message":"Done","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"95da5dab1e48777af6cfcaf71eaba01add5ac39b","unresolved":true,"context_lines":[{"line_number":927,"context_line":"                [lower.encode(\u0027utf-8\u0027), name.encode(\u0027utf-8\u0027)]"},{"line_number":928,"context_line":"                for lower, name in bounds]"},{"line_number":929,"context_line":"        ns_bound_list \u003d NamespaceBoundList(bounds)"},{"line_number":930,"context_line":"        infocache[cache_key] \u003d ns_bound_list"},{"line_number":931,"context_line":"    else:"},{"line_number":932,"context_line":"        ns_bound_list \u003d None"},{"line_number":933,"context_line":"    return ns_bound_list, cache_state"}],"source_content_type":"text/x-python","patch_set":1,"id":"d6bd4419_b1a6be60","line":930,"updated":"2023-11-20 12:11:45.000000000","message":"@Jianjian - note: stored in infocache here","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1629d1b483b438d9016ce44cc2cd4bd4a9c576f7","unresolved":false,"context_lines":[{"line_number":927,"context_line":"                [lower.encode(\u0027utf-8\u0027), name.encode(\u0027utf-8\u0027)]"},{"line_number":928,"context_line":"                for lower, name in bounds]"},{"line_number":929,"context_line":"        ns_bound_list \u003d NamespaceBoundList(bounds)"},{"line_number":930,"context_line":"        infocache[cache_key] \u003d ns_bound_list"},{"line_number":931,"context_line":"    else:"},{"line_number":932,"context_line":"        ns_bound_list \u003d None"},{"line_number":933,"context_line":"    return ns_bound_list, cache_state"}],"source_content_type":"text/x-python","patch_set":1,"id":"041937ff_0574bcf5","line":930,"in_reply_to":"49a3e820_35d1cfc8","updated":"2023-11-21 05:38:27.000000000","message":"Acknowledged","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9f17876a6dd96a9b30d97b498990d7c4a0194e00","unresolved":true,"context_lines":[{"line_number":927,"context_line":"                [lower.encode(\u0027utf-8\u0027), name.encode(\u0027utf-8\u0027)]"},{"line_number":928,"context_line":"                for lower, name in bounds]"},{"line_number":929,"context_line":"        ns_bound_list \u003d NamespaceBoundList(bounds)"},{"line_number":930,"context_line":"        infocache[cache_key] \u003d ns_bound_list"},{"line_number":931,"context_line":"    else:"},{"line_number":932,"context_line":"        ns_bound_list \u003d None"},{"line_number":933,"context_line":"    return ns_bound_list, cache_state"}],"source_content_type":"text/x-python","patch_set":1,"id":"49a3e820_35d1cfc8","line":930,"in_reply_to":"d6bd4419_b1a6be60","updated":"2023-11-20 23:31:34.000000000","message":"I think it makes sense to move these helpers into base since they\u0027re used in obj (for updating) \u0026 container (for listing)\n\nthis behavior is definately covered by existing tests:\n\nhttps://github.com/NVIDIA/swift/blob/master/test/unit/proxy/test_server.py#L4759\n\nswift/test/unit/proxy/test_server.py::TestReplicatedObjectController::test_backend_headers_update_shard_container_can_skip_cache","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9f17876a6dd96a9b30d97b498990d7c4a0194e00","unresolved":true,"context_lines":[{"line_number":953,"context_line":"                         raise_on_error\u003dTrue)"},{"line_number":954,"context_line":"            cache_state \u003d \u0027set\u0027"},{"line_number":955,"context_line":"        except MemcacheConnectionError:"},{"line_number":956,"context_line":"            cache_state \u003d \u0027set_error\u0027"},{"line_number":957,"context_line":"    return cache_state"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"b9905b7f_73eccccf","line":956,"updated":"2023-11-20 23:31:34.000000000","message":"nit: slightly clearer to me as:\n\n    diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py\n    index 76de1fd93..a60f6b4f7 100644\n    --- a/swift/proxy/controllers/base.py\n    +++ b/swift/proxy/controllers/base.py\n    @@ -945,15 +945,17 @@ def set_namespaces_in_cache(req, cache_key, ns_bound_list, time):\n         \"\"\"\n         infocache \u003d req.environ.setdefault(\u0027swift.infocache\u0027, {})\n         infocache[cache_key] \u003d ns_bound_list\n    -    cache_state \u003d \u0027disabled\u0027\n         memcache \u003d cache_from_env(req.environ, True)\n         if memcache and ns_bound_list:\n             try:\n                 memcache.set(cache_key, ns_bound_list.bounds, time\u003dtime,\n                              raise_on_error\u003dTrue)\n    -            cache_state \u003d \u0027set\u0027\n             except MemcacheConnectionError:\n                 cache_state \u003d \u0027set_error\u0027\n    +        else:\n    +            cache_state \u003d \u0027set\u0027\n    +    else:\n    +        cache_state \u003d \u0027disabled\u0027\n         return cache_state\n         \n... but that could be personal preference; I see no obviously objective reason to change it.","commit_id":"4952f91e8b4200e3f265a82f46a9196b7624ce89"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a5c695f4aebbb1c297faca40e98220f46ae9dbf4","unresolved":false,"context_lines":[{"line_number":953,"context_line":"                         raise_on_error\u003dTrue)"},{"line_number":954,"context_line":"            cache_state \u003d \u0027set\u0027"},{"line_number":955,"context_line":"        except MemcacheConnectionError:"},{"line_number":956,"context_line":"            cache_state \u003d \u0027set_error\u0027"},{"line_number":957,"context_line":"    return cache_state"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"07ba1abc_865c01d1","line":956,"in_reply_to":"b9905b7f_73eccccf","updated":"2023-11-21 10:27:47.000000000","message":"I agree","commit_id":"4952f91e8b4200e3f265a82f46a9196b7624ce89"}],"swift/proxy/controllers/container.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5b937a064ae551dd9c87877e64c6eb883349ed5e","unresolved":true,"context_lines":[{"line_number":188,"context_line":"            resp \u003d None"},{"line_number":189,"context_line":"        else:"},{"line_number":190,"context_line":"            # shard ranges can be returned from cache"},{"line_number":191,"context_line":"            infocache[cache_key] \u003d ns_bound_list"},{"line_number":192,"context_line":"            self.logger.debug(\u0027Found %d shards in cache for %s\u0027,"},{"line_number":193,"context_line":"                              len(ns_bound_list.bounds), req.path_qs)"},{"line_number":194,"context_line":"            headers.update({\u0027x-backend-record-type\u0027: \u0027shard\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"0abd7bee_6bc397d5","side":"PARENT","line":191,"updated":"2023-11-18 01:18:23.000000000","message":"this is one behavioral change, we don\u0027t save cached ``ns_bound_list`` into inforcache now, but should be okay since it\u0027s unlikely to be reused anymore.","commit_id":"b67c88714a4da72235f61ffa0bd2cf024bc0a76f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"95da5dab1e48777af6cfcaf71eaba01add5ac39b","unresolved":true,"context_lines":[{"line_number":188,"context_line":"            resp \u003d None"},{"line_number":189,"context_line":"        else:"},{"line_number":190,"context_line":"            # shard ranges can be returned from cache"},{"line_number":191,"context_line":"            infocache[cache_key] \u003d ns_bound_list"},{"line_number":192,"context_line":"            self.logger.debug(\u0027Found %d shards in cache for %s\u0027,"},{"line_number":193,"context_line":"                              len(ns_bound_list.bounds), req.path_qs)"},{"line_number":194,"context_line":"            headers.update({\u0027x-backend-record-type\u0027: \u0027shard\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"984c10f5_bc3b8e78","side":"PARENT","line":191,"in_reply_to":"0abd7bee_6bc397d5","updated":"2023-11-20 12:11:45.000000000","message":"we do! line 930 in base.py","commit_id":"b67c88714a4da72235f61ffa0bd2cf024bc0a76f"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1629d1b483b438d9016ce44cc2cd4bd4a9c576f7","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            resp \u003d None"},{"line_number":189,"context_line":"        else:"},{"line_number":190,"context_line":"            # shard ranges can be returned from cache"},{"line_number":191,"context_line":"            infocache[cache_key] \u003d ns_bound_list"},{"line_number":192,"context_line":"            self.logger.debug(\u0027Found %d shards in cache for %s\u0027,"},{"line_number":193,"context_line":"                              len(ns_bound_list.bounds), req.path_qs)"},{"line_number":194,"context_line":"            headers.update({\u0027x-backend-record-type\u0027: \u0027shard\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"049e23a6_3196ba79","side":"PARENT","line":191,"in_reply_to":"984c10f5_bc3b8e78","updated":"2023-11-21 05:38:27.000000000","message":"Acknowledged","commit_id":"b67c88714a4da72235f61ffa0bd2cf024bc0a76f"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5b937a064ae551dd9c87877e64c6eb883349ed5e","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            set_cache_state \u003d set_namespaces_in_cache("},{"line_number":205,"context_line":"                req, cache_key, ns_bound_list,"},{"line_number":206,"context_line":"                self.app.recheck_listing_shard_ranges)"},{"line_number":207,"context_line":"            if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":208,"context_line":"                self.logger.info("},{"line_number":209,"context_line":"                    \u0027Caching listing namespaces for %s (%d namespaces)\u0027,"},{"line_number":210,"context_line":"                    cache_key, len(ns_bound_list.bounds))"}],"source_content_type":"text/x-python","patch_set":1,"id":"00f54a7f_4614f39f","line":207,"updated":"2023-11-18 01:18:23.000000000","message":"we can print out an error message too, maybe we should also pass the description of exception?\n```\n            if set_cache_state \u003d\u003d \u0027set\u0027:\n                self.logger.info(\n                    \u0027Caching listing namespaces for %s (%d namespaces)\u0027,\n                    cache_key, len(ns_bound_list.bounds))\n            else:\n                self.logger.info(\n                    \u0027Failed to cache listing namespaces for %s (%d namespaces)\u0027,\n                    cache_key, len(ns_bound_list.bounds))```","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"95da5dab1e48777af6cfcaf71eaba01add5ac39b","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            set_cache_state \u003d set_namespaces_in_cache("},{"line_number":205,"context_line":"                req, cache_key, ns_bound_list,"},{"line_number":206,"context_line":"                self.app.recheck_listing_shard_ranges)"},{"line_number":207,"context_line":"            if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":208,"context_line":"                self.logger.info("},{"line_number":209,"context_line":"                    \u0027Caching listing namespaces for %s (%d namespaces)\u0027,"},{"line_number":210,"context_line":"                    cache_key, len(ns_bound_list.bounds))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ee9e0fb5_7c1ec57f","line":207,"in_reply_to":"00f54a7f_4614f39f","updated":"2023-11-20 12:11:45.000000000","message":"errors are already logged in MemcacheRing before the exception is raised:\n\nfor example, test.unit.common.test_memcached.TestMemcached.test_set_error_raise_on_error\n\n```\nERROR: Error talking to memcached: 1.2.3.4:11211: with key_prefix too-big, method set, time_spent 0.0, failed set: SERVER_ERROR object too large for cache```","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"18f309c098f2cf5ba63b680f02ade662466ed02a","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            set_cache_state \u003d set_namespaces_in_cache("},{"line_number":205,"context_line":"                req, cache_key, ns_bound_list,"},{"line_number":206,"context_line":"                self.app.recheck_listing_shard_ranges)"},{"line_number":207,"context_line":"            if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":208,"context_line":"                self.logger.info("},{"line_number":209,"context_line":"                    \u0027Caching listing namespaces for %s (%d namespaces)\u0027,"},{"line_number":210,"context_line":"                    cache_key, len(ns_bound_list.bounds))"}],"source_content_type":"text/x-python","patch_set":1,"id":"4f120e78_bd88292f","line":207,"in_reply_to":"45583176_9f66062d","updated":"2023-11-21 16:21:46.000000000","message":"The exception log line which Memcached client prints out only has prefix, doesn\u0027t have account/container which will be interesting to see for debugging purpose. But I agree, that\u0027s out of scope for this patch.","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a5c695f4aebbb1c297faca40e98220f46ae9dbf4","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            set_cache_state \u003d set_namespaces_in_cache("},{"line_number":205,"context_line":"                req, cache_key, ns_bound_list,"},{"line_number":206,"context_line":"                self.app.recheck_listing_shard_ranges)"},{"line_number":207,"context_line":"            if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":208,"context_line":"                self.logger.info("},{"line_number":209,"context_line":"                    \u0027Caching listing namespaces for %s (%d namespaces)\u0027,"},{"line_number":210,"context_line":"                    cache_key, len(ns_bound_list.bounds))"}],"source_content_type":"text/x-python","patch_set":1,"id":"45583176_9f66062d","line":207,"in_reply_to":"af15c074_b695f334","updated":"2023-11-21 10:27:47.000000000","message":"IIRC we added the info log back when we started seeing cache misses and wanted some indicator that cache was being set. It is unusual - we don\u0027t usually give a commentary on normal operation in the request path (perhaps we do more so in daemons). But I\u0027d prefer to keep it out of scope for this patch.","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9f17876a6dd96a9b30d97b498990d7c4a0194e00","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            set_cache_state \u003d set_namespaces_in_cache("},{"line_number":205,"context_line":"                req, cache_key, ns_bound_list,"},{"line_number":206,"context_line":"                self.app.recheck_listing_shard_ranges)"},{"line_number":207,"context_line":"            if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":208,"context_line":"                self.logger.info("},{"line_number":209,"context_line":"                    \u0027Caching listing namespaces for %s (%d namespaces)\u0027,"},{"line_number":210,"context_line":"                    cache_key, len(ns_bound_list.bounds))"}],"source_content_type":"text/x-python","patch_set":1,"id":"af15c074_b695f334","line":207,"in_reply_to":"ee9e0fb5_7c1ec57f","updated":"2023-11-20 23:31:34.000000000","message":"I think the existing logging of the cache result on success is questionable; additional signal on failure may be reasonable- but that would be a new behavior - not appropriate for this \"refactor only\" change.\n\nthe behavior is covered by existing tests:\n\nhttps://github.com/NVIDIA/swift/blob/master/test/unit/proxy/controllers/test_container.py#L3222-L3224\n\nswift/test/unit/proxy/controllers/test_container.py::TestGetPathShardCaching::test_GET_shard_ranges_write_to_cache","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"5b937a064ae551dd9c87877e64c6eb883349ed5e","unresolved":true,"context_lines":[{"line_number":348,"context_line":"                record_cache_op_metrics("},{"line_number":349,"context_line":"                    self.logger, self.server_type.lower(), \u0027shard_updating\u0027,"},{"line_number":350,"context_line":"                    set_cache_state, None)"},{"line_number":351,"context_line":"                if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":352,"context_line":"                    self.logger.info("},{"line_number":353,"context_line":"                        \u0027Caching updating shards for %s (%d shards)\u0027,"},{"line_number":354,"context_line":"                        cache_key, len(shard_ranges))"}],"source_content_type":"text/x-python","patch_set":1,"id":"c12e22ce_73ca4e22","line":351,"updated":"2023-11-18 01:18:23.000000000","message":"print out error log message here as well.","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a5c695f4aebbb1c297faca40e98220f46ae9dbf4","unresolved":false,"context_lines":[{"line_number":348,"context_line":"                record_cache_op_metrics("},{"line_number":349,"context_line":"                    self.logger, self.server_type.lower(), \u0027shard_updating\u0027,"},{"line_number":350,"context_line":"                    set_cache_state, None)"},{"line_number":351,"context_line":"                if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":352,"context_line":"                    self.logger.info("},{"line_number":353,"context_line":"                        \u0027Caching updating shards for %s (%d shards)\u0027,"},{"line_number":354,"context_line":"                        cache_key, len(shard_ranges))"}],"source_content_type":"text/x-python","patch_set":1,"id":"48023f26_a7feb392","line":351,"in_reply_to":"5fd88abf_557ea06a","updated":"2023-11-21 10:27:47.000000000","message":"Acknowledged","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9f17876a6dd96a9b30d97b498990d7c4a0194e00","unresolved":true,"context_lines":[{"line_number":348,"context_line":"                record_cache_op_metrics("},{"line_number":349,"context_line":"                    self.logger, self.server_type.lower(), \u0027shard_updating\u0027,"},{"line_number":350,"context_line":"                    set_cache_state, None)"},{"line_number":351,"context_line":"                if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":352,"context_line":"                    self.logger.info("},{"line_number":353,"context_line":"                        \u0027Caching updating shards for %s (%d shards)\u0027,"},{"line_number":354,"context_line":"                        cache_key, len(shard_ranges))"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fd88abf_557ea06a","line":351,"in_reply_to":"a786a9ef_d27472e6","updated":"2023-11-20 23:31:34.000000000","message":"i iike how the diff on this file is mostly red!\n\ni don\u0027t like how obviously \"stringly-typed\" the interfce for this new helper is; I could imagine an (equally?  slightly-better?) interface being a return tuple:\n\n    ok, more_info \u003d set_in_cache()\n    if ok:\n       log.info(\u0027it worked: %s\u0027 % more_info)\n       \n... BUT!  I think it\u0027s sufficient as-is because if I change this message we get a test failure:\n\n    vagrant@saio:~$ pytest swift/test/unit/proxy/test_server.py::TestReplicatedObjectController::test_backend_headers_update_shard_container_with_empty_cache\n    ...\n\n    swift/test/unit/proxy/test_server.py::TestReplicatedObjectController::test_backend_headers_update_shard_container_with_empty_cache FAILED                                       [100%]\n    ...\n        \n                expected \u003d {}\n                for i, device in enumerate([\u0027sda\u0027, \u0027sdb\u0027, \u0027sdc\u0027]):\n                    expected[device] \u003d \u002710.0.0.%d:100%d\u0027 % (i, i)\n                self.assertEqual(container_headers, expected)\n        \n    \u003e       do_test(\u0027POST\u0027, \u0027sharding\u0027)\n\n    swift/test/unit/proxy/test_server.py:4492: \n    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _\n    swift/test/unit/proxy/test_server.py:4439: in do_test\n        self.assertIn(\n    E   AssertionError: \u0027Caching updating shards for shard-updating-v2/a/c (3 shards)\u0027 not found in [\u0027Caching foo shards for shard-updating-v2/a/c (3 shards)\u0027]\n    ...\n    \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d short test summary info \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n    FAILED swift/test/unit/proxy/test_server.py::TestReplicatedObjectController::test_backend_headers_update_shard_container_with_empty_cache - AssertionError: \u0027Caching updating shards for shard-updating-v2/a/c (3 shards)\u0027 not found in [\u0027Caching foo shards for shard-updating-v2/a/c (3 shards)\u0027]\n    \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d 1 failed in 1.65s \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"95da5dab1e48777af6cfcaf71eaba01add5ac39b","unresolved":true,"context_lines":[{"line_number":348,"context_line":"                record_cache_op_metrics("},{"line_number":349,"context_line":"                    self.logger, self.server_type.lower(), \u0027shard_updating\u0027,"},{"line_number":350,"context_line":"                    set_cache_state, None)"},{"line_number":351,"context_line":"                if set_cache_state \u003d\u003d \u0027set\u0027:"},{"line_number":352,"context_line":"                    self.logger.info("},{"line_number":353,"context_line":"                        \u0027Caching updating shards for %s (%d shards)\u0027,"},{"line_number":354,"context_line":"                        cache_key, len(shard_ranges))"}],"source_content_type":"text/x-python","patch_set":1,"id":"a786a9ef_d27472e6","line":351,"in_reply_to":"c12e22ce_73ca4e22","updated":"2023-11-20 12:11:45.000000000","message":"see reply in container.py - already logged in MemcacheRing","commit_id":"38366a564f57fc2c9e807ea0b8f7c002f0ccf91a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9f17876a6dd96a9b30d97b498990d7c4a0194e00","unresolved":true,"context_lines":[{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        self.logger.info("},{"line_number":353,"context_line":"            \u0027Caching updating shards for %s (%d shards)\u0027,"},{"line_number":354,"context_line":"            cache_key, len(namespaces.bounds))"},{"line_number":355,"context_line":"        try:"},{"line_number":356,"context_line":"            memcache.set("},{"line_number":357,"context_line":"                cache_key, namespaces.bounds,"}],"source_content_type":"text/x-python","patch_set":2,"id":"60948367_e394e960","side":"PARENT","line":354,"updated":"2023-11-20 23:31:34.000000000","message":"this behavior is not in the helper method; but is covered by unittests\n\nhttps://github.com/NVIDIA/swift/blob/master/test/unit/proxy/test_server.py#L4439-L4441","commit_id":"b67c88714a4da72235f61ffa0bd2cf024bc0a76f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b4ad1af9e81b8d3cdbb028a5331747a522b6c5d8","unresolved":false,"context_lines":[{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        self.logger.info("},{"line_number":353,"context_line":"            \u0027Caching updating shards for %s (%d shards)\u0027,"},{"line_number":354,"context_line":"            cache_key, len(namespaces.bounds))"},{"line_number":355,"context_line":"        try:"},{"line_number":356,"context_line":"            memcache.set("},{"line_number":357,"context_line":"                cache_key, namespaces.bounds,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fab1b795_8e9e5955","side":"PARENT","line":354,"in_reply_to":"60948367_e394e960","updated":"2023-11-21 16:59:45.000000000","message":"Acknowledged","commit_id":"b67c88714a4da72235f61ffa0bd2cf024bc0a76f"}],"test/unit/proxy/controllers/test_base.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1629d1b483b438d9016ce44cc2cd4bd4a9c576f7","unresolved":false,"context_lines":[{"line_number":203,"context_line":"@patch_policies([StoragePolicy(0, \u0027zero\u0027, True, object_ring\u003dFakeRing())])"},{"line_number":204,"context_line":"class TestFuncs(BaseTest):"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def test_get_namespaces_from_cache_disabled(self):"},{"line_number":207,"context_line":"        cache_key \u003d \u0027shard-updating-v2/a/c/\u0027"},{"line_number":208,"context_line":"        req \u003d Request.blank(\u0027a/c\u0027)"},{"line_number":209,"context_line":"        actual \u003d get_namespaces_from_cache(req, cache_key, 0)"}],"source_content_type":"text/x-python","patch_set":2,"id":"77ab67d4_8933b221","line":206,"updated":"2023-11-21 05:38:27.000000000","message":"Great test cases to cover two newly added functions!","commit_id":"4952f91e8b4200e3f265a82f46a9196b7624ce89"}]}