)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"38e9916e18290cfab188c661396600833ac58175","unresolved":true,"context_lines":[{"line_number":12,"context_line":"SLO object."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patch will try keeping the manifest files in page cache"},{"line_number":15,"context_line":"by not evicting them after reading."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Co-Authored-By: Tim Burke \u003ctim.burke@gmail.com\u003e"},{"line_number":18,"context_line":"Co-Authored-By: Clay Gerrard \u003cclay.gerrard@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"00a67a1a_3a4049ba","line":15,"updated":"2023-06-09 14:44:29.000000000","message":"nit: mention that the new behaviour is configurable","commit_id":"0c661b4fe81932a9320a72a9c1519a155e04d712"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"c4cc1cde62f2bfab425b52481b93a6ffea395030","unresolved":false,"context_lines":[{"line_number":12,"context_line":"SLO object."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patch will try keeping the manifest files in page cache"},{"line_number":15,"context_line":"by not evicting them after reading."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Co-Authored-By: Tim Burke \u003ctim.burke@gmail.com\u003e"},{"line_number":18,"context_line":"Co-Authored-By: Clay Gerrard \u003cclay.gerrard@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"8d077eef_235b1fa9","line":15,"in_reply_to":"00a67a1a_3a4049ba","updated":"2023-06-10 21:52:34.000000000","message":"Done","commit_id":"0c661b4fe81932a9320a72a9c1519a155e04d712"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7a1ab16e8596bc8a816f8803f6ffeb25fc8e2bb3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5cc1498e_773b2fac","updated":"2023-06-05 23:10:26.000000000","message":"Yeah, I\u0027d be interested in carrying this. Might be sort of nice if we had some feedback mechanism based on req/s so we don\u0027t fill up on infrequently-accessed (but still _occasionally_ accessed) manifests. Makes me think a little of https://github.com/openstack/swift/commit/de888629817a2a326b6e8dc66edb0ce3168818a7 ...","commit_id":"fd830fa39492067d8bf3daf7351568e919ff0396"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"83419a851a107e59e782804d26ee1f4cae09b577","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"38433d0d_2e8a7d5a","updated":"2023-06-06 03:47:54.000000000","message":"Thanks for the review!","commit_id":"f3a0efbede8b45d94e8d31a5e535d2855a2cf1c1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"38e9916e18290cfab188c661396600833ac58175","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0b14b125_d38ef05a","updated":"2023-06-09 14:44:29.000000000","message":"LGTM but I\u0027d like to see some tests. Sadly I didn\u0027t find any coverage for the existing keep_cache behavior 😞","commit_id":"0c661b4fe81932a9320a72a9c1519a155e04d712"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"da748bb5c4c936575cfb926a40b81f9374fc6971","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"460e13f6_96350cb8","updated":"2023-06-08 17:36:07.000000000","message":"Still willing to carry. Good to give ops a knob they can use to get back to the old behavior; even better that they have to opt in to the _new_ behavior. Some tests under `test/unit/obj/` would be appreciated, but it looks like we don\u0027t have any for `keep_cache_private`, so... \\*shrug\\*\n\nI\u0027ve got this vague premonition of us wanting to evolve this toward some `keep_cache` option with its own little DSL like `(unauthenticated and size \u003c 5MiB) or is_slo` but I don\u0027t entirely _like_ that idea, so I\u0027m glad we\u0027re going this way for now.\n\nWill be very interested in seeing what kinds of changes we can see in prod with this.","commit_id":"0c661b4fe81932a9320a72a9c1519a155e04d712"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"c4cc1cde62f2bfab425b52481b93a6ffea395030","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"dfbbbc1a_40ed8b2f","updated":"2023-06-10 21:52:34.000000000","message":"thanks for reviews!","commit_id":"0c661b4fe81932a9320a72a9c1519a155e04d712"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0cf156f33d4afd019bfbf18462bfa93bc91fe7d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"21d96b66_f92a1c20","updated":"2023-06-12 15:10:12.000000000","message":"it\u0027s not obvious to me form the logs why the proxy got a 503\n\n\tproxy-server.error\n\tJun 12 09:20:40 np0034281953 proxy-server: Object returning 503 for [] (txn: txa7f2f7d47a2f4e1680cd9-006486e367) (client_ip: 127.0.0.1)\n\tJun 12 09:20:40 np0034281953 proxy-server: STDERR: 127.0.0.1 - - [12/Jun/2023 09:20:40] \"GET /v1/AUTH_test/container-2fcd9d9e-aa86-4a98-b356-9072db63e080/object-f8a28246-2194-42a0-b5ff-571467a90690 HTTP/1.1\" 503 360 0.033616 (txn: txa7f2f7d47a2f4e1680cd9-006486e367)\n\n\tproxy-server.log\n\tJun 12 09:20:40 np0034281953 proxy-server: 127.0.0.1 127.0.0.1 12/Jun/2023/09/20/40 GET /v1/AUTH_test/container-2fcd9d9e-aa86-4a98-b356-9072db63e080/object-f8a28246-2194-42a0-b5ff-571467a90690 HTTP/1.0 503 - python-swiftclient-3.13.1 AUTH_tk30c6bc166... - 118 - txa7f2f7d47a2f4e1680cd9-006486e367 - 0.0326 - - 1686561639.994406939 1686561640.026966095 2\n\n\nMaybe the object-server\u0027s blew up?  I wasn\u0027t able to pull up ALL the storage server logs from Zuul - maybe we can try to recheck or reproduce locally.","commit_id":"8e8a64a653d83e3cb86ab9ec0b5593342ee6dd58"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"dc7b8c51a8465fdb74716ee32c0888ffb899eb08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"001701b7_70fea2da","updated":"2023-06-12 08:57:13.000000000","message":"recheck\n\ngrenade test unrelated to patch","commit_id":"8e8a64a653d83e3cb86ab9ec0b5593342ee6dd58"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29c2121f88840f20807d181728a54615d56338d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"75daf1ac_5ed783e2","updated":"2023-06-12 15:08:10.000000000","message":"recheck \n\nprobe test failed with unrelated E           ClientException: Object GET failed: http://127.0.0.1:8080/v1/AUTH_test/container-489ec1e0-550d-4172-9595-11b8ac9af3ee/object-8f4e132f-0d17-4964-9ac0-23be5614c395 503 Service Unavailable  [first 60 chars of response] \u003chtml\u003e\u003ch1\u003eService Unavailable\u003c/h1\u003e\u003cp\u003eThe server is currently (txn: tx8d77391c030b4f12b53d4-006486e97a)","commit_id":"8e8a64a653d83e3cb86ab9ec0b5593342ee6dd58"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"4a8aa92b0ee97dcda15d069c6b9b99a17431ffda","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"31353714_110237e2","updated":"2023-07-10 18:40:13.000000000","message":"Seems reasonable -- it\u0027s configurable (and defaults to prior behavior) and tested. It\u0027s generally a little off-putting to have middleware knowledge plumbed down to the object layer like that, but I think we can make an exception since an SLO manifest likely to be requested more often than other objects. A similar argument could be made for DLO or symlink, but those are (usually) zero-byte objects anyway, so it\u0027s kinda moot.\n\n_Maybe_ we\u0027d do well to have the proxy send a backend header that says \"if you see metadata with _this_ name, don\u0027t drop cache\", move the configurability to SLO, and make SLO responsible for setting the header appropriately... basically, the `X-Backend-Ignore-Range-If-Metadata-Present` model from https://github.com/openstack/swift/commit/e8b654f3\n\nBut IDK that we\u0027ve got any other use-cases in mind that would want to do this. (Maybe something like swauth? Or ALOs, if/when we get around to that?)\n\nI\u0027m going to think about it some more before I approve, but I\u0027m certainly not *opposed* to this landing as-is.","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"071013a7305ce3de54a8cdec91dcfce880184367","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5c84d710_c04a10f9","updated":"2023-07-10 21:45:05.000000000","message":"Thanks a lot for the reviews!","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"763d56ae_ed6d414a","updated":"2023-07-10 18:33:26.000000000","message":"This is a stright forward and useful new option; I could see `keep_cache_slo_manifest \u003d true` becoming the default some day.  I have no qualms with the implementation, and the docs and example config look great!\n\nI think the use of a mock so close to behavior is a little \"on the nose\" for my taste in unittests; but it\u0027s probably fine.","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"}],"etc/object-server.conf-sample":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"da748bb5c4c936575cfb926a40b81f9374fc6971","unresolved":true,"context_lines":[{"line_number":144,"context_line":"# keep_cache_private \u003d false"},{"line_number":145,"context_line":"#"},{"line_number":146,"context_line":"# If true, SLO object\u0027s manifest file for GET requests may be kept in buffer cache"},{"line_number":147,"context_line":"# if smaller than \u0027keep_cache_size\u0027"},{"line_number":148,"context_line":"# keep_cache_slo_manifest \u003d false"},{"line_number":149,"context_line":"#"},{"line_number":150,"context_line":"# on PUTs, sync data every n MB"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"9aad4d69_b6f37e09","line":147,"updated":"2023-06-08 17:36:07.000000000","message":"Might be worth noting how this only has any effect when `keep_cache_private \u003d false`\n\nI\u0027m also realizing that tempurls for smallish objects probably don\u0027t trip the cache-dropping since they wouldn\u0027t have any token -- I\u0027m not clear if that\u0027s a bug or a feature :-/","commit_id":"0c661b4fe81932a9320a72a9c1519a155e04d712"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"c4cc1cde62f2bfab425b52481b93a6ffea395030","unresolved":false,"context_lines":[{"line_number":144,"context_line":"# keep_cache_private \u003d false"},{"line_number":145,"context_line":"#"},{"line_number":146,"context_line":"# If true, SLO object\u0027s manifest file for GET requests may be kept in buffer cache"},{"line_number":147,"context_line":"# if smaller than \u0027keep_cache_size\u0027"},{"line_number":148,"context_line":"# keep_cache_slo_manifest \u003d false"},{"line_number":149,"context_line":"#"},{"line_number":150,"context_line":"# on PUTs, sync data every n MB"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"2c4e504b_d8d71b6a","line":147,"in_reply_to":"9aad4d69_b6f37e09","updated":"2023-06-10 21:52:34.000000000","message":"Done","commit_id":"0c661b4fe81932a9320a72a9c1519a155e04d712"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":146,"context_line":"# If true, SLO object\u0027s manifest file for GET requests may be kept in buffer cache"},{"line_number":147,"context_line":"# if smaller than \u0027keep_cache_size\u0027. And this config will only matter when"},{"line_number":148,"context_line":"# \u0027keep_cache_private\u0027 is false."},{"line_number":149,"context_line":"# keep_cache_slo_manifest \u003d false"},{"line_number":150,"context_line":"#"},{"line_number":151,"context_line":"# on PUTs, sync data every n MB"},{"line_number":152,"context_line":"# mb_per_sync \u003d 512"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"ee171a5d_fe8e3ce4","line":149,"updated":"2023-07-10 18:33:26.000000000","message":"haha, this actually makes the `keep_cache[_for]_private[_objects]` name a little easier to read!  kudos!","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"}],"swift/obj/server.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"aa0432f60389bf59a4bd792d7495d4a33137d73c","unresolved":true,"context_lines":[{"line_number":1105,"context_line":"                    or ("},{"line_number":1106,"context_line":"                        \"X-Static-Large-Object\" in request.headers"},{"line_number":1107,"context_line":"                        and request.headers[\"X-Static-Large-Object\"].lower()"},{"line_number":1108,"context_line":"                        \u003d\u003d \"true\""},{"line_number":1109,"context_line":"                    )"},{"line_number":1110,"context_line":"                )"},{"line_number":1111,"context_line":"                conditional_etag \u003d resolve_etag_is_at_header(request, metadata)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a49bde63_195a16b0","line":1108,"updated":"2023-06-05 18:48:26.000000000","message":"`request.headers`, or `metadata`?\n\nAssuming the latter, I\u0027d maybe go with\n```\nor config_true_value(metadata.get(\u0027X-Static-Large-Object\u0027))\n```\nSee https://github.com/openstack/swift/blob/2.31.1/swift/common/middleware/slo.py#L1539 for example.","commit_id":"dc28ed29a94b99a6834e8381abf4efcae6527967"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7a1ab16e8596bc8a816f8803f6ffeb25fc8e2bb3","unresolved":false,"context_lines":[{"line_number":1105,"context_line":"                    or ("},{"line_number":1106,"context_line":"                        \"X-Static-Large-Object\" in request.headers"},{"line_number":1107,"context_line":"                        and request.headers[\"X-Static-Large-Object\"].lower()"},{"line_number":1108,"context_line":"                        \u003d\u003d \"true\""},{"line_number":1109,"context_line":"                    )"},{"line_number":1110,"context_line":"                )"},{"line_number":1111,"context_line":"                conditional_etag \u003d resolve_etag_is_at_header(request, metadata)"}],"source_content_type":"text/x-python","patch_set":1,"id":"547df0d0_868d260f","line":1108,"in_reply_to":"a49bde63_195a16b0","updated":"2023-06-05 23:10:26.000000000","message":"Done","commit_id":"dc28ed29a94b99a6834e8381abf4efcae6527967"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"85f287d2b8f22136fe095afce9f7f88bca7f16ad","unresolved":false,"context_lines":[{"line_number":1105,"context_line":"                    or ("},{"line_number":1106,"context_line":"                        \"X-Static-Large-Object\" in request.headers"},{"line_number":1107,"context_line":"                        and request.headers[\"X-Static-Large-Object\"].lower()"},{"line_number":1108,"context_line":"                        \u003d\u003d \"true\""},{"line_number":1109,"context_line":"                    )"},{"line_number":1110,"context_line":"                )"},{"line_number":1111,"context_line":"                conditional_etag \u003d resolve_etag_is_at_header(request, metadata)"}],"source_content_type":"text/x-python","patch_set":1,"id":"73fba4c8_671aef25","line":1108,"in_reply_to":"a49bde63_195a16b0","updated":"2023-06-06 00:36:32.000000000","message":"yeah, it should be metadata. And config_true_value is great, thanks!","commit_id":"dc28ed29a94b99a6834e8381abf4efcae6527967"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"7a1ab16e8596bc8a816f8803f6ffeb25fc8e2bb3","unresolved":true,"context_lines":[{"line_number":1103,"context_line":"                        and \"X-Storage-Token\" not in request.headers"},{"line_number":1104,"context_line":"                    )"},{"line_number":1105,"context_line":"                    or ("},{"line_number":1106,"context_line":"                        \"X-Static-Large-Object\" in metadata"},{"line_number":1107,"context_line":"                        and config_true_value(metadata.get(\u0027X-Static-Large-Object\u0027))"},{"line_number":1108,"context_line":"                    )"},{"line_number":1109,"context_line":"                )"}],"source_content_type":"text/x-python","patch_set":2,"id":"44274ee4_d6aa0a89","line":1106,"updated":"2023-06-05 23:10:26.000000000","message":"nit: Nice thing about the new phrasing is that we don\u0027t need the `in` check -- if it\u0027s not there, `.get()` will come back `None`, and `config_true_value()` will return `False`.","commit_id":"fd830fa39492067d8bf3daf7351568e919ff0396"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"83419a851a107e59e782804d26ee1f4cae09b577","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"                        and \"X-Storage-Token\" not in request.headers"},{"line_number":1104,"context_line":"                    )"},{"line_number":1105,"context_line":"                    or ("},{"line_number":1106,"context_line":"                        \"X-Static-Large-Object\" in metadata"},{"line_number":1107,"context_line":"                        and config_true_value(metadata.get(\u0027X-Static-Large-Object\u0027))"},{"line_number":1108,"context_line":"                    )"},{"line_number":1109,"context_line":"                )"}],"source_content_type":"text/x-python","patch_set":2,"id":"145463ce_5e90d7dd","line":1106,"in_reply_to":"44274ee4_d6aa0a89","updated":"2023-06-06 03:47:54.000000000","message":"Done","commit_id":"fd830fa39492067d8bf3daf7351568e919ff0396"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":1110,"context_line":"                            metadata.get(\"X-Static-Large-Object\")"},{"line_number":1111,"context_line":"                        )"},{"line_number":1112,"context_line":"                    )"},{"line_number":1113,"context_line":"                )"},{"line_number":1114,"context_line":"                conditional_etag \u003d resolve_etag_is_at_header(request, metadata)"},{"line_number":1115,"context_line":"                response \u003d Response("},{"line_number":1116,"context_line":"                    app_iter\u003ddisk_file.reader(keep_cache\u003dkeep_cache),"}],"source_content_type":"text/x-python","patch_set":6,"id":"1e57f2d9_78a5c066","line":1113,"updated":"2023-07-10 18:33:26.000000000","message":"this is not too bad as far as complex boolean equations go - thanks for writing it all out","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"}],"test/unit/common/test_utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":2298,"context_line":"            self.assertTrue(utils.config_true_value(True) is True)"},{"line_number":2299,"context_line":"            self.assertTrue(utils.config_true_value(\u0027foo\u0027) is False)"},{"line_number":2300,"context_line":"            self.assertTrue(utils.config_true_value(False) is False)"},{"line_number":2301,"context_line":"            self.assertTrue(utils.config_true_value(None) is False)"},{"line_number":2302,"context_line":"        finally:"},{"line_number":2303,"context_line":"            utils.TRUE_VALUES \u003d orig_trues"},{"line_number":2304,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"65aef738_d76fc713","line":2301,"updated":"2023-07-10 18:33:26.000000000","message":"+1","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"29c2121f88840f20807d181728a54615d56338d9","unresolved":true,"context_lines":[{"line_number":4293,"context_line":"        # metadata has \"X-Static-Large-Object\", then disk_file.reader"},{"line_number":4294,"context_line":"        # will be called with keep_cache\u003dFalse."},{"line_number":4295,"context_line":"        # Set up a new ObjectController with customized configurations."},{"line_number":4296,"context_line":"        slo_tmpdir \u003d mkdtemp()"},{"line_number":4297,"context_line":"        slo_testdir \u003d os.path.join(slo_tmpdir,"},{"line_number":4298,"context_line":"                                   \u0027tmp_test_object_server_ObjectController\u0027)"},{"line_number":4299,"context_line":"        mkdirs(os.path.join(slo_testdir, \u0027sda1\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"a6efaed2_919edbf2","line":4296,"range":{"start_line":4296,"start_character":8,"end_line":4296,"end_character":30},"updated":"2023-06-12 15:08:10.000000000","message":"please use the with_tempdir decorator: https://github.com/openstack/swift/blob/647ee83906df8fae336b479c0f169be2ced1772c/test/unit/__init__.py#L564\n\nthat way, if the test raises an exception and line 4338 in not reached, the temp dir is still removed","commit_id":"8e8a64a653d83e3cb86ab9ec0b5593342ee6dd58"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"14ad62d658757f0274917b2e34391a76a7419c5b","unresolved":false,"context_lines":[{"line_number":4293,"context_line":"        # metadata has \"X-Static-Large-Object\", then disk_file.reader"},{"line_number":4294,"context_line":"        # will be called with keep_cache\u003dFalse."},{"line_number":4295,"context_line":"        # Set up a new ObjectController with customized configurations."},{"line_number":4296,"context_line":"        slo_tmpdir \u003d mkdtemp()"},{"line_number":4297,"context_line":"        slo_testdir \u003d os.path.join(slo_tmpdir,"},{"line_number":4298,"context_line":"                                   \u0027tmp_test_object_server_ObjectController\u0027)"},{"line_number":4299,"context_line":"        mkdirs(os.path.join(slo_testdir, \u0027sda1\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"c31eec9e_cfe89cdb","line":4296,"range":{"start_line":4296,"start_character":8,"end_line":4296,"end_character":30},"in_reply_to":"a6efaed2_919edbf2","updated":"2023-07-07 19:54:49.000000000","message":"I will reuse \u0027self.testdir\u0027 which will be cleaned by tearDown().","commit_id":"8e8a64a653d83e3cb86ab9ec0b5593342ee6dd58"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a6efd6703314c17e303d3bb0956ac6d206cfdf00","unresolved":true,"context_lines":[{"line_number":4486,"context_line":"                gmtime(math.ceil(float(timestamp)))),"},{"line_number":4487,"context_line":"        })"},{"line_number":4488,"context_line":"        # clean up for this customized ObjectController."},{"line_number":4489,"context_line":"        rmtree(slo_tmpdir)"},{"line_number":4490,"context_line":""},{"line_number":4491,"context_line":"    @mock.patch(\"time.time\", mock_time)"},{"line_number":4492,"context_line":"    def test_DELETE(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"f6bf095e_9e32510f","line":4489,"updated":"2023-06-12 15:11:20.000000000","message":"great tests.\n\nas a follow-on, we should extend these to backfill coverage for keep_cache_private","commit_id":"8e8a64a653d83e3cb86ab9ec0b5593342ee6dd58"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"14ad62d658757f0274917b2e34391a76a7419c5b","unresolved":false,"context_lines":[{"line_number":4486,"context_line":"                gmtime(math.ceil(float(timestamp)))),"},{"line_number":4487,"context_line":"        })"},{"line_number":4488,"context_line":"        # clean up for this customized ObjectController."},{"line_number":4489,"context_line":"        rmtree(slo_tmpdir)"},{"line_number":4490,"context_line":""},{"line_number":4491,"context_line":"    @mock.patch(\"time.time\", mock_time)"},{"line_number":4492,"context_line":"    def test_DELETE(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"03bfc735_c0695e7e","line":4489,"in_reply_to":"f6bf095e_9e32510f","updated":"2023-07-07 19:54:49.000000000","message":"Done","commit_id":"8e8a64a653d83e3cb86ab9ec0b5593342ee6dd58"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4458,"context_line":"        reader_mock \u003d mock.Mock(keep_cache\u003dFalse)"},{"line_number":4459,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.BaseDiskFile.reader\u0027, reader_mock):"},{"line_number":4460,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4461,"context_line":"            reader_mock.assert_called_with(keep_cache\u003dFalse)"},{"line_number":4462,"context_line":"        self.assertEqual(resp.status_int, 200)"},{"line_number":4463,"context_line":"        etag \u003d \u0027\"%s\"\u0027 % md5(b\u0027VERIFY\u0027, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":4464,"context_line":"        self.assertEqual(dict(resp.headers), {"}],"source_content_type":"text/x-python","patch_set":6,"id":"82fd4968_92d97dc3","line":4461,"updated":"2023-07-10 18:33:26.000000000","message":"this is the main assert, but I\u0027m not sure I understand how it\u0027s related to L4458 where we explicitly set keep_cache\u003dFalse\n\n... is this fake overriding the careful test setup/configuration?","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"071013a7305ce3de54a8cdec91dcfce880184367","unresolved":true,"context_lines":[{"line_number":4458,"context_line":"        reader_mock \u003d mock.Mock(keep_cache\u003dFalse)"},{"line_number":4459,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.BaseDiskFile.reader\u0027, reader_mock):"},{"line_number":4460,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4461,"context_line":"            reader_mock.assert_called_with(keep_cache\u003dFalse)"},{"line_number":4462,"context_line":"        self.assertEqual(resp.status_int, 200)"},{"line_number":4463,"context_line":"        etag \u003d \u0027\"%s\"\u0027 % md5(b\u0027VERIFY\u0027, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":4464,"context_line":"        self.assertEqual(dict(resp.headers), {"}],"source_content_type":"text/x-python","patch_set":6,"id":"c2022030_b956bafe","line":4461,"in_reply_to":"82fd4968_92d97dc3","updated":"2023-07-10 21:45:05.000000000","message":"Good point. I can make a change to L4458 to have keep_cache default as True, then test case will be more accurate. I have another follow-up TODO to add keep_cache_size test cases, will make this changes within that patch.","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4484,"context_line":"        conf \u003d {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027false\u0027,"},{"line_number":4485,"context_line":"                \u0027container_update_timeout\u0027: 0.0,"},{"line_number":4486,"context_line":"                \u0027keep_cache_private\u0027: \u0027false\u0027,"},{"line_number":4487,"context_line":"                \u0027keep_cache_slo_manifest\u0027: \u0027false\u0027}"},{"line_number":4488,"context_line":"        obj_controller \u003d object_server.ObjectController("},{"line_number":4489,"context_line":"            conf, logger\u003dself.logger)"},{"line_number":4490,"context_line":"        obj_controller.bytes_per_sync \u003d 1"}],"source_content_type":"text/x-python","patch_set":6,"id":"f5c6add8_c24ec5e0","line":4487,"updated":"2023-07-10 18:33:26.000000000","message":"this is not that different than the test_GET_keep_cache_slo_manifest_no_config case","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4495,"context_line":"                                     \u0027X-Static-Large-Object\u0027: \u0027True\u0027})"},{"line_number":4496,"context_line":"        req.body \u003d b\u0027VERIFY\u0027"},{"line_number":4497,"context_line":"        resp \u003d req.get_response(obj_controller)"},{"line_number":4498,"context_line":"        self.assertEqual(resp.status_int, 201)"},{"line_number":4499,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a/c/o\u0027,"},{"line_number":4500,"context_line":"                            headers\u003d{\u0027Content-Type\u0027: \u0027application/x-test\u0027,"},{"line_number":4501,"context_line":"                                     \u0027X-Auth-Token\u0027: \u00272340lsdfhhjl02lxfjj\u0027})"}],"source_content_type":"text/x-python","patch_set":6,"id":"cb2d12ee_fda7f0ca","line":4498,"updated":"2023-07-10 18:33:26.000000000","message":"ok, so we only validate slo manifest in proxy - at object layer we treat x-static-large-object as fact and write it to test self.testdir","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4503,"context_line":"        reader_mock \u003d mock.Mock(keep_cache\u003dFalse)"},{"line_number":4504,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.BaseDiskFile.reader\u0027, reader_mock):"},{"line_number":4505,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4506,"context_line":"            reader_mock.assert_called_with(keep_cache\u003dFalse)"},{"line_number":4507,"context_line":"        self.assertEqual(resp.status_int, 200)"},{"line_number":4508,"context_line":"        etag \u003d \u0027\"%s\"\u0027 % md5(b\u0027VERIFY\u0027, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":4509,"context_line":"        self.assertEqual(dict(resp.headers), {"}],"source_content_type":"text/x-python","patch_set":6,"id":"eea1dbbd_31cec325","line":4506,"updated":"2023-07-10 18:33:26.000000000","message":"same behavior","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4529,"context_line":"        conf \u003d {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027false\u0027,"},{"line_number":4530,"context_line":"                \u0027container_update_timeout\u0027: 0.0,"},{"line_number":4531,"context_line":"                \u0027keep_cache_private\u0027: \u0027false\u0027,"},{"line_number":4532,"context_line":"                \u0027keep_cache_slo_manifest\u0027: \u0027true\u0027}"},{"line_number":4533,"context_line":"        obj_controller \u003d object_server.ObjectController("},{"line_number":4534,"context_line":"            conf, logger\u003dself.logger)"},{"line_number":4535,"context_line":"        obj_controller.bytes_per_sync \u003d 1"}],"source_content_type":"text/x-python","patch_set":6,"id":"87d9e0f5_0e5b68fc","line":4532,"updated":"2023-07-10 18:33:26.000000000","message":"oh yeah, here we go!","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4545,"context_line":"                            headers\u003d{\u0027Content-Type\u0027: \u0027application/x-test\u0027,"},{"line_number":4546,"context_line":"                                     \u0027X-Auth-Token\u0027: \u00272340lsdfhhjl02lxfjj\u0027})"},{"line_number":4547,"context_line":""},{"line_number":4548,"context_line":"        reader_mock \u003d mock.Mock(keep_cache\u003dFalse)"},{"line_number":4549,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.BaseDiskFile.reader\u0027, reader_mock):"},{"line_number":4550,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4551,"context_line":"            reader_mock.assert_called_with(keep_cache\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":6,"id":"46addd9c_56fe02a3","line":4548,"updated":"2023-07-10 18:33:26.000000000","message":"ok, so it seems this is probably mostly related to the keep_cache_private cofig value","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4548,"context_line":"        reader_mock \u003d mock.Mock(keep_cache\u003dFalse)"},{"line_number":4549,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.BaseDiskFile.reader\u0027, reader_mock):"},{"line_number":4550,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4551,"context_line":"            reader_mock.assert_called_with(keep_cache\u003dTrue)"},{"line_number":4552,"context_line":"        self.assertEqual(resp.status_int, 200)"},{"line_number":4553,"context_line":"        etag \u003d \u0027\"%s\"\u0027 % md5(b\u0027VERIFY\u0027, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":4554,"context_line":"        self.assertEqual(dict(resp.headers), {"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ea7237a_84256d93","line":4551,"updated":"2023-07-10 18:33:26.000000000","message":"boom!  that\u0027s what we want\n\nI wonder if we should mock the falloate drop cache call so we can leave the reader configuration alone","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"071013a7305ce3de54a8cdec91dcfce880184367","unresolved":true,"context_lines":[{"line_number":4548,"context_line":"        reader_mock \u003d mock.Mock(keep_cache\u003dFalse)"},{"line_number":4549,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.BaseDiskFile.reader\u0027, reader_mock):"},{"line_number":4550,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4551,"context_line":"            reader_mock.assert_called_with(keep_cache\u003dTrue)"},{"line_number":4552,"context_line":"        self.assertEqual(resp.status_int, 200)"},{"line_number":4553,"context_line":"        etag \u003d \u0027\"%s\"\u0027 % md5(b\u0027VERIFY\u0027, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":4554,"context_line":"        self.assertEqual(dict(resp.headers), {"}],"source_content_type":"text/x-python","patch_set":6,"id":"b97be821_fd098352","line":4551,"in_reply_to":"5ea7237a_84256d93","updated":"2023-07-10 21:45:05.000000000","message":"maybe we need some unit test cases for reader() function itself?\n\n    def reader(self, keep_cache\u003dFalse,\n               _quarantine_hook\u003dlambda m: None):","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4574,"context_line":"        conf \u003d {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027false\u0027,"},{"line_number":4575,"context_line":"                \u0027container_update_timeout\u0027: 0.0,"},{"line_number":4576,"context_line":"                \u0027keep_cache_private\u0027: \u0027false\u0027,"},{"line_number":4577,"context_line":"                \u0027keep_cache_slo_manifest\u0027: \u0027true\u0027}"},{"line_number":4578,"context_line":"        obj_controller \u003d object_server.ObjectController("},{"line_number":4579,"context_line":"            conf, logger\u003dself.logger)"},{"line_number":4580,"context_line":"        obj_controller.bytes_per_sync \u003d 1"}],"source_content_type":"text/x-python","patch_set":6,"id":"ad41b52b_9460fb5e","line":4577,"updated":"2023-07-10 18:33:26.000000000","message":"ok, so same config as before...","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4581,"context_line":"        timestamp \u003d normalize_timestamp(time())"},{"line_number":4582,"context_line":"        req \u003d Request.blank(\u0027/sda1/p/a/c/o\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027},"},{"line_number":4583,"context_line":"                            headers\u003d{\u0027X-Timestamp\u0027: timestamp,"},{"line_number":4584,"context_line":"                                     \u0027Content-Type\u0027: \u0027application/x-test\u0027})"},{"line_number":4585,"context_line":"        req.body \u003d b\u0027VERIFY\u0027"},{"line_number":4586,"context_line":"        resp \u003d req.get_response(obj_controller)"},{"line_number":4587,"context_line":"        self.assertEqual(resp.status_int, 201)"}],"source_content_type":"text/x-python","patch_set":6,"id":"f78af09b_416a78ae","line":4584,"updated":"2023-07-10 18:33:26.000000000","message":"... but not SLO","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"938b6dfc93fe78f92a9caa0696afe30e3b6560fe","unresolved":true,"context_lines":[{"line_number":4592,"context_line":"        reader_mock \u003d mock.Mock(keep_cache\u003dFalse)"},{"line_number":4593,"context_line":"        with mock.patch(\u0027swift.obj.diskfile.BaseDiskFile.reader\u0027, reader_mock):"},{"line_number":4594,"context_line":"            resp \u003d req.get_response(obj_controller)"},{"line_number":4595,"context_line":"            reader_mock.assert_called_with(keep_cache\u003dFalse)"},{"line_number":4596,"context_line":"        self.assertEqual(resp.status_int, 200)"},{"line_number":4597,"context_line":"        etag \u003d \u0027\"%s\"\u0027 % md5(b\u0027VERIFY\u0027, usedforsecurity\u003dFalse).hexdigest()"},{"line_number":4598,"context_line":"        self.assertEqual(dict(resp.headers), {"}],"source_content_type":"text/x-python","patch_set":6,"id":"458899fd_b6ff2bbb","line":4595,"updated":"2023-07-10 18:33:26.000000000","message":"... and so we don\u0027t keep the cache.","commit_id":"cb1e584e6495b4e35b9b2833f250ac38b76ab0b3"}]}