)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"67f7b01d7659ff605ec64cf393b9f37314bd952b","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Make object-expirer respect internal_client_conf_path"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the object-expirer decides it was loaded from a \"legacy_conf\" it won\u0027t respect the internal_client_conf_path."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I24ec702cd2ed074ca9df084cefc896418cece394"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"f0f03902_7445d614","line":10,"updated":"2024-09-24 10:28:04.000000000","message":"I found the commit message confusing: the subject says \"respect\" internal_client_conf_path, the body says it won\u0027t respect it.\nI think what may be intended is that previously it wasn\u0027t respected for a legacy conf, but now it is.\n\nI tend to assume that when we look back at git history we\u0027ll have forgotten what motivated a change so it\u0027s worth writing a clear message to remind ourselves :)\n\nA better form of words might be:\n\n```\nMake object-expirer always respect internal_client_conf_path\n\nPreviously the object-expirer would not respect an internal_client_conf_path option if its config was loaded from a \"legacy\" config filepath (e.g. **/object-expirer.conf). Legacy object-expirer config expected config sections for an internal-client to be included in the same file. However, \"modern\" object-expirer config files (e.g. **/object-server.conf) expect internal-client config to be loaded from a separate file specified by internal_client_conf_path and defaulting to \u0027internal_client_conf_path\u0027.\n\n\n\"Legacy\" expirer config is still used in production clusters but it is useful to use a shared internal-client config. This patch therefore makes the internal_client_conf_path option always be respected regardless of the path of the object-expirer conf file. If \u0027internal_client_conf_path\u0027 is not specified, \"modern\" config will continue to use the default \u0027/etc/swift/internal-client.conf\u0027, but \"legacy\" config will default to the path of the expirer conf file (i.e. the same as the previous behavior).\n```","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"8b165406e7fb752f5dbd7ed01e6775c08505eacf","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Make object-expirer respect internal_client_conf_path"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the object-expirer decides it was loaded from a \"legacy_conf\" it won\u0027t respect the internal_client_conf_path."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I24ec702cd2ed074ca9df084cefc896418cece394"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"c3d6efd0_d69e7046","line":10,"in_reply_to":"709583df_84e1cba3","updated":"2024-09-26 20:52:44.000000000","message":"Done. Thanks!","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5d23e2d4ed958504836ed50d526d9375fc56f304","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Make object-expirer respect internal_client_conf_path"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the object-expirer decides it was loaded from a \"legacy_conf\" it won\u0027t respect the internal_client_conf_path."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I24ec702cd2ed074ca9df084cefc896418cece394"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"709583df_84e1cba3","line":10,"in_reply_to":"f0f03902_7445d614","updated":"2024-09-24 10:49:34.000000000","message":"Also, it would be useful to reference the original change. PLease add\n\n```\nRelated-Change: Ib21568f9b9d8547da87a99d65ae73a550e9c3230\n```","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5d23e2d4ed958504836ed50d526d9375fc56f304","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"48586247_b276279f","updated":"2024-09-24 10:49:34.000000000","message":"Meant to also comment that object-expirer.conf-sample and object-expirer.conf.5 should be updated.\n\nThe original intent of ignoring internal_client_conf_path isn\u0027t obvious, other than if the author just didn\u0027t want to change the legacy set up *at all*. And they had a different mission in mind which was to move people towards the modern conf, so why do the work to enhance the legacy?\n\nNow with the passage of time we\u0027re still living with legacy and we can justify enhancing it.\n\nThere is of course a \"risk\" that someone has an object-expirer.conf with internal_client_conf_path that is currently being ignored, and with upgrade becomes significant. But there\u0027s always a risk that someone has typed an unsupported option into a conf file and we then choose to use that option. So on balance I feel that (a) it\u0027s unlikely and (b) it wasn\u0027t supported so you shouldn\u0027t have used it.","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"717bb30f0ddd92241cff7a66ff8e3fd0655ea538","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"bf874a3d_418603de","updated":"2024-09-24 03:29:34.000000000","message":"More of a question then a real critism. just want to understand why all the __file__ business.","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"e88196bb_738d2d80","updated":"2024-09-27 12:25:13.000000000","message":"@Chinemerem thanks! this makes sense, but I\u0027d prefer some of the re-formatting to be reverted","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1b0a185ba4990485218a68bc17b82bec5e62f0ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"5e6f1bf2_620d7fad","updated":"2024-09-27 16:07:00.000000000","message":"i had some comments I forgot to publish","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"4a0cf167_637a1e5d","updated":"2024-09-27 18:02:15.000000000","message":"Thanks for the reviews","commit_id":"318f455aedc9d0768c1d44d9b1e6564d8ffc4b8e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2a2257c04fdffb906c3152ff6fda72078b522503","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"cfb32b37_d51a20bd","updated":"2024-10-02 08:38:01.000000000","message":"oops, sent previous reply before finishing!\n\nThis is nearly there IMHO. Just some clean up in the tests, and one test scenario to add back that was lost","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2517485fba0ad75bd8aa5a1873304fac7a7ef4d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"66f30ce1_e9822b54","updated":"2024-10-02 08:36:32.000000000","message":"ve it","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a9764e51d0ae875fcde3d86613d2489ebd54532c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"194f6842_36793f69","updated":"2024-10-02 16:29:05.000000000","message":"LGTM","commit_id":"ee80f99aec8719fc3ad1d86bf8cd6bd48fc1bd33"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b00005159ddbb96e0c9b731190e4832c5ebbf124","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"06198738_c4fedd83","updated":"2024-10-02 16:40:29.000000000","message":"looks like test_init sort of duplicates the new test but that seems fine to me!","commit_id":"ee80f99aec8719fc3ad1d86bf8cd6bd48fc1bd33"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"9b37800ab48f909b97a0c71dd037c191cd99a9db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"2741ca41_346cdba8","updated":"2024-10-03 08:58:26.000000000","message":"recheck\n\nunrelated stats error with compute in devstack test\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\nFailed 1 tests - output below:\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\n\nopenstack.tests.unit.test_stats.TestStats.test_servers_no_detail\n----------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/openstack/tests/unit/test_stats.py\", line 292, in test_servers_no_detail\n    self.assert_reported_stat(\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/openstack/tests/unit/test_stats.py\", line 163, in assert_reported_stat\n    raise Exception(\"Key %s not found in reported stats\" % key)\n\n    Exception: Key openstack.api.compute.GET.servers.200 not found in reported stats\n\n\nCaptured statsd_content:\n~~~~~~~~~~~~~~~~~~~~~~~~\n    [b\u0027openstack.api.compute.GET.servers.200:10.000000|ms\\nopenstack.api.compute\u0027\n b\u0027.GET.servers.200:1|c\\nopenstack.api.compute.GET.servers.attempted:1|c\u0027]\n```","commit_id":"ee80f99aec8719fc3ad1d86bf8cd6bd48fc1bd33"}],"swift/obj/expirer.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1b0a185ba4990485218a68bc17b82bec5e62f0ed","unresolved":true,"context_lines":[{"line_number":162,"context_line":"        self.conf_path \u003d \\"},{"line_number":163,"context_line":"            self.conf.get(\u0027__file__\u0027) or \u0027/etc/swift/object-expirer.conf\u0027"},{"line_number":164,"context_line":"        # True, if the conf file is \u0027object-expirer.conf\u0027."},{"line_number":165,"context_line":"        is_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path"},{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"},{"line_number":167,"context_line":"        self.dequeue_from_legacy \u003d \\"},{"line_number":168,"context_line":"            True if is_legacy_conf else \\"}],"source_content_type":"text/x-python","patch_set":6,"id":"847bfe11_6296e411","line":165,"updated":"2024-09-27 16:07:00.000000000","message":"I don\u0027t love that the \"rules\" for \"guess default\" are way up here, and then the actual config path that gets selected is in a different method.","commit_id":"3c8b2135d4d77c4c154c69b187380d68624e7a78"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        self.conf_path \u003d \\"},{"line_number":163,"context_line":"            self.conf.get(\u0027__file__\u0027) or \u0027/etc/swift/object-expirer.conf\u0027"},{"line_number":164,"context_line":"        # True, if the conf file is \u0027object-expirer.conf\u0027."},{"line_number":165,"context_line":"        is_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path"},{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"},{"line_number":167,"context_line":"        self.dequeue_from_legacy \u003d \\"},{"line_number":168,"context_line":"            True if is_legacy_conf else \\"}],"source_content_type":"text/x-python","patch_set":6,"id":"ac186958_6e8fee1d","line":165,"in_reply_to":"847bfe11_6296e411","updated":"2024-09-27 18:02:15.000000000","message":"Acknowledged","commit_id":"3c8b2135d4d77c4c154c69b187380d68624e7a78"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1b0a185ba4990485218a68bc17b82bec5e62f0ed","unresolved":true,"context_lines":[{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"},{"line_number":167,"context_line":"        self.dequeue_from_legacy \u003d \\"},{"line_number":168,"context_line":"            True if is_legacy_conf else \\"},{"line_number":169,"context_line":"            config_true_value(conf.get(\u0027dequeue_from_legacy\u0027, \u0027false\u0027))"},{"line_number":170,"context_line":"        self.swift \u003d swift or self._make_internal_client(is_legacy_conf)"},{"line_number":171,"context_line":"        self.read_conf_for_queue_access()"},{"line_number":172,"context_line":"        self.report_interval \u003d float(conf.get(\u0027report_interval\u0027) or 300)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1bd839f5_42f21ef3","line":169,"updated":"2024-09-27 16:07:00.000000000","message":"I hate `self.dequeue_from_legacy` - this is another example of configuration value that\u0027s *ignored* in \"legacy config file\"\n\nN.B. we use a \"legacy config file\" in prod (and therefore always \"dequeue_from_legacy\") because the legacy queue is the only queue that actually exists:\n\n517389: Add object-expirer new mode to execute tasks from general task queue | https://review.opendev.org/c/openstack/swift/+/517389","commit_id":"3c8b2135d4d77c4c154c69b187380d68624e7a78"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"},{"line_number":167,"context_line":"        self.dequeue_from_legacy \u003d \\"},{"line_number":168,"context_line":"            True if is_legacy_conf else \\"},{"line_number":169,"context_line":"            config_true_value(conf.get(\u0027dequeue_from_legacy\u0027, \u0027false\u0027))"},{"line_number":170,"context_line":"        self.swift \u003d swift or self._make_internal_client(is_legacy_conf)"},{"line_number":171,"context_line":"        self.read_conf_for_queue_access()"},{"line_number":172,"context_line":"        self.report_interval \u003d float(conf.get(\u0027report_interval\u0027) or 300)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a9591e4b_4c915ecc","line":169,"in_reply_to":"1bd839f5_42f21ef3","updated":"2024-09-27 18:02:15.000000000","message":"Acknowledged","commit_id":"3c8b2135d4d77c4c154c69b187380d68624e7a78"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"67f7b01d7659ff605ec64cf393b9f37314bd952b","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        self.tasks_per_second \u003d float(conf.get(\u0027tasks_per_second\u0027, 50.0))"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        self.conf_path \u003d \\"},{"line_number":163,"context_line":"            self.conf.get(\u0027__file__\u0027) or \u0027/etc/swift/object-expirer.conf\u0027"},{"line_number":164,"context_line":"        # True, if the conf file is \u0027object-expirer.conf\u0027."},{"line_number":165,"context_line":"        is_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path"},{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"}],"source_content_type":"text/x-python","patch_set":9,"id":"b957966f_c5afce5a","line":163,"range":{"start_line":163,"start_character":38,"end_line":163,"end_character":73},"updated":"2024-09-24 10:28:04.000000000","message":"do we know when this condition might be true (...is it just in tests??)","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"8b165406e7fb752f5dbd7ed01e6775c08505eacf","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        self.tasks_per_second \u003d float(conf.get(\u0027tasks_per_second\u0027, 50.0))"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        self.conf_path \u003d \\"},{"line_number":163,"context_line":"            self.conf.get(\u0027__file__\u0027) or \u0027/etc/swift/object-expirer.conf\u0027"},{"line_number":164,"context_line":"        # True, if the conf file is \u0027object-expirer.conf\u0027."},{"line_number":165,"context_line":"        is_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path"},{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"}],"source_content_type":"text/x-python","patch_set":9,"id":"30ba4809_06684302","line":163,"range":{"start_line":163,"start_character":38,"end_line":163,"end_character":73},"in_reply_to":"38214c28_eed44f67","updated":"2024-09-26 20:52:44.000000000","message":"Yes, it\u0027s just in the tests.","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"5d23e2d4ed958504836ed50d526d9375fc56f304","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        self.tasks_per_second \u003d float(conf.get(\u0027tasks_per_second\u0027, 50.0))"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        self.conf_path \u003d \\"},{"line_number":163,"context_line":"            self.conf.get(\u0027__file__\u0027) or \u0027/etc/swift/object-expirer.conf\u0027"},{"line_number":164,"context_line":"        # True, if the conf file is \u0027object-expirer.conf\u0027."},{"line_number":165,"context_line":"        is_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path"},{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"}],"source_content_type":"text/x-python","patch_set":9,"id":"38214c28_eed44f67","line":163,"range":{"start_line":163,"start_character":38,"end_line":163,"end_character":73},"in_reply_to":"b957966f_c5afce5a","updated":"2024-09-24 10:49:34.000000000","message":"oh look, Romain had the same query 5years ago https://review.opendev.org/c/openstack/swift/+/601950/comment/9fdfeff1_d21a196d/ ... but the tests were never improved","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"67f7b01d7659ff605ec64cf393b9f37314bd952b","unresolved":true,"context_lines":[{"line_number":162,"context_line":"        self.conf_path \u003d \\"},{"line_number":163,"context_line":"            self.conf.get(\u0027__file__\u0027) or \u0027/etc/swift/object-expirer.conf\u0027"},{"line_number":164,"context_line":"        # True, if the conf file is \u0027object-expirer.conf\u0027."},{"line_number":165,"context_line":"        is_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path"},{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"},{"line_number":167,"context_line":"        self.dequeue_from_legacy \u003d \\"},{"line_number":168,"context_line":"            True if is_legacy_conf else \\"}],"source_content_type":"text/x-python","patch_set":9,"id":"314e322e_c92a0e71","line":165,"updated":"2024-09-24 10:28:04.000000000","message":"off-topic: this is very weak i.e. just matching on \u0027expirer\u0027","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"8b165406e7fb752f5dbd7ed01e6775c08505eacf","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        self.conf_path \u003d \\"},{"line_number":163,"context_line":"            self.conf.get(\u0027__file__\u0027) or \u0027/etc/swift/object-expirer.conf\u0027"},{"line_number":164,"context_line":"        # True, if the conf file is \u0027object-expirer.conf\u0027."},{"line_number":165,"context_line":"        is_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path"},{"line_number":166,"context_line":"        # object-expirer.conf supports only legacy queue"},{"line_number":167,"context_line":"        self.dequeue_from_legacy \u003d \\"},{"line_number":168,"context_line":"            True if is_legacy_conf else \\"}],"source_content_type":"text/x-python","patch_set":9,"id":"6cf0616f_3a31a955","line":165,"in_reply_to":"314e322e_c92a0e71","updated":"2024-09-26 20:52:44.000000000","message":"I agree.","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"717bb30f0ddd92241cff7a66ff8e3fd0655ea538","unresolved":true,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _make_internal_client(self, is_legacy_conf):"},{"line_number":189,"context_line":"        if is_legacy_conf:"},{"line_number":190,"context_line":"            default_guess \u003d \"__file__\""},{"line_number":191,"context_line":"        else:"},{"line_number":192,"context_line":"            default_guess \u003d \"/etc/swift/internal-client.conf\""},{"line_number":193,"context_line":"        ic_conf_path \u003d self.conf.get("}],"source_content_type":"text/x-python","patch_set":9,"id":"33aac677_aadff2b0","line":190,"updated":"2024-09-24 03:29:34.000000000","message":"All this `__file__` stuff looks clever, but I guess I don\u0027t fully understand why it\u0027s needed. Is there a case where `__file__` needs to be interrogated.\n\nThe `is_legecy_conf` is defined back on line 165:\n\n```\nis_legacy_conf \u003d \u0027expirer\u0027 in self.conf_path\n```\n\nWhich seems to indicate that we know the conf path already with `self.conf_path`. (as we used to use before this patch). And `self.conf_path` is build already from interrogating conf.get(\u0027__file__\u0027). So aren\u0027t we already passed the whole __file__ thing?\n\nSo can\u0027t we just:\n\n```\ndefault_guess \u003d \"/etc/swift/internal-client.conf\"\nif is_legacy_conf:\n    default_guess \u003d self.conf_path\nic_conf_path \u003d self.conf.get(\n    \"internal_client_conf_path\", default_guess)\n```","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"67f7b01d7659ff605ec64cf393b9f37314bd952b","unresolved":true,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _make_internal_client(self, is_legacy_conf):"},{"line_number":189,"context_line":"        if is_legacy_conf:"},{"line_number":190,"context_line":"            default_guess \u003d \"__file__\""},{"line_number":191,"context_line":"        else:"},{"line_number":192,"context_line":"            default_guess \u003d \"/etc/swift/internal-client.conf\""},{"line_number":193,"context_line":"        ic_conf_path \u003d self.conf.get("}],"source_content_type":"text/x-python","patch_set":9,"id":"a3126d5c_42e13450","line":190,"in_reply_to":"33aac677_aadff2b0","updated":"2024-09-24 10:28:04.000000000","message":"ditto Matt\u0027s comment. I\u0027m not sure in what sense it\u0027s a \"guess\"...other than maybe a reference to line 165 being pretty weak.","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"8b165406e7fb752f5dbd7ed01e6775c08505eacf","unresolved":false,"context_lines":[{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _make_internal_client(self, is_legacy_conf):"},{"line_number":189,"context_line":"        if is_legacy_conf:"},{"line_number":190,"context_line":"            default_guess \u003d \"__file__\""},{"line_number":191,"context_line":"        else:"},{"line_number":192,"context_line":"            default_guess \u003d \"/etc/swift/internal-client.conf\""},{"line_number":193,"context_line":"        ic_conf_path \u003d self.conf.get("}],"source_content_type":"text/x-python","patch_set":9,"id":"70b3bc44_cb490ac6","line":190,"in_reply_to":"a3126d5c_42e13450","updated":"2024-09-26 20:52:44.000000000","message":"The initial thought process was that by using the special keyword \"file\" one could explictly \"pass InternalClient the path from conf[__file__]. After discussion with Alistair and Clay, we decided that this was not necessary functionality, and so I will just do\n```\n        default_ic_conf_path \u003d \"/etc/swift/internal-client.conf\"\n        if is_legacy_conf:\n            default_ic_conf_path \u003d self.conf_path\n        ic_conf_path \u003d self.conf.get(\n            \"internal_client_conf_path\", default_ic_conf_path)\n```\nlike you suggest","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"67f7b01d7659ff605ec64cf393b9f37314bd952b","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        if is_legacy_conf:"},{"line_number":190,"context_line":"            default_guess \u003d \"__file__\""},{"line_number":191,"context_line":"        else:"},{"line_number":192,"context_line":"            default_guess \u003d \"/etc/swift/internal-client.conf\""},{"line_number":193,"context_line":"        ic_conf_path \u003d self.conf.get("},{"line_number":194,"context_line":"            \"internal_client_conf_path\", default_guess"},{"line_number":195,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":9,"id":"2dea8f9c_6f591a81","line":192,"range":{"start_line":192,"start_character":12,"end_line":192,"end_character":25},"updated":"2024-09-24 10:28:04.000000000","message":"nit: why not call it default_ic_conf_path ? Easier on my brain to read \n\n```\nic_conf_path \u003d self.conf.get(\n            \"internal_client_conf_path\", default_ic_conf_path\n        )\n```","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"8b165406e7fb752f5dbd7ed01e6775c08505eacf","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        if is_legacy_conf:"},{"line_number":190,"context_line":"            default_guess \u003d \"__file__\""},{"line_number":191,"context_line":"        else:"},{"line_number":192,"context_line":"            default_guess \u003d \"/etc/swift/internal-client.conf\""},{"line_number":193,"context_line":"        ic_conf_path \u003d self.conf.get("},{"line_number":194,"context_line":"            \"internal_client_conf_path\", default_guess"},{"line_number":195,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":9,"id":"5601799b_ed70fcb5","line":192,"range":{"start_line":192,"start_character":12,"end_line":192,"end_character":25},"in_reply_to":"2dea8f9c_6f591a81","updated":"2024-09-26 20:52:44.000000000","message":"Done.","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":191,"context_line":"            default_ic_conf_path \u003d self.conf_path"},{"line_number":192,"context_line":"        ic_conf_path \u003d self.conf.get("},{"line_number":193,"context_line":"            \"internal_client_conf_path\", default_ic_conf_path)"},{"line_number":194,"context_line":"        request_tries \u003d int(self.conf.get(\"request_tries\") or 3)"},{"line_number":195,"context_line":"        return InternalClient("},{"line_number":196,"context_line":"            ic_conf_path,"},{"line_number":197,"context_line":"            \"Swift Object Expirer\","}],"source_content_type":"text/x-python","patch_set":12,"id":"fefb0298_986d2b39","line":194,"range":{"start_line":194,"start_character":42,"end_line":194,"end_character":43},"updated":"2024-09-27 12:25:13.000000000","message":"please don\u0027t make unnecessary changes - apart from cluttering up the review, this changes the git history, and the use of \u0027 vs \" is the original author\u0027s choice","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":191,"context_line":"            default_ic_conf_path \u003d self.conf_path"},{"line_number":192,"context_line":"        ic_conf_path \u003d self.conf.get("},{"line_number":193,"context_line":"            \"internal_client_conf_path\", default_ic_conf_path)"},{"line_number":194,"context_line":"        request_tries \u003d int(self.conf.get(\"request_tries\") or 3)"},{"line_number":195,"context_line":"        return InternalClient("},{"line_number":196,"context_line":"            ic_conf_path,"},{"line_number":197,"context_line":"            \"Swift Object Expirer\","}],"source_content_type":"text/x-python","patch_set":12,"id":"15975cdf_0f67e32e","line":194,"range":{"start_line":194,"start_character":42,"end_line":194,"end_character":43},"in_reply_to":"fefb0298_986d2b39","updated":"2024-09-27 18:02:15.000000000","message":"Reverted","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":200,"context_line":"            global_conf\u003d{"},{"line_number":201,"context_line":"                \"log_name\": \"%s-ic\" % self.conf.get(\"log_name\", self.log_route)"},{"line_number":202,"context_line":"            },"},{"line_number":203,"context_line":"        )"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def read_conf_for_queue_access(self):"},{"line_number":206,"context_line":"        self.expiring_objects_account \u003d AUTO_CREATE_ACCOUNT_PREFIX + \\"}],"source_content_type":"text/x-python","patch_set":12,"id":"e87a8210_b43e39cb","line":203,"updated":"2024-09-27 12:25:13.000000000","message":"see comment in the test file - please revert the re-formatting","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            global_conf\u003d{"},{"line_number":201,"context_line":"                \"log_name\": \"%s-ic\" % self.conf.get(\"log_name\", self.log_route)"},{"line_number":202,"context_line":"            },"},{"line_number":203,"context_line":"        )"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def read_conf_for_queue_access(self):"},{"line_number":206,"context_line":"        self.expiring_objects_account \u003d AUTO_CREATE_ACCOUNT_PREFIX + \\"}],"source_content_type":"text/x-python","patch_set":12,"id":"6fb07f81_437d94e6","line":203,"in_reply_to":"e87a8210_b43e39cb","updated":"2024-09-27 18:02:15.000000000","message":"Reverted","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"}],"test/unit/obj/test_expirer.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"717bb30f0ddd92241cff7a66ff8e3fd0655ea538","unresolved":true,"context_lines":[{"line_number":409,"context_line":"        # specified as __file__ -\u003e object-expirer.conf value"},{"line_number":410,"context_line":"        conf \u003d {"},{"line_number":411,"context_line":"            \"__file__\": \"/etc/swift/object-expirer.conf\","},{"line_number":412,"context_line":"            \"internal_client_conf_path\": \"__file__\","},{"line_number":413,"context_line":"        }"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        with mock.patch.object("}],"source_content_type":"text/x-python","patch_set":9,"id":"16cd2cee_85574d67","line":412,"updated":"2024-09-24 03:29:34.000000000","message":"Do we every expect this to actually happen?","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"8b165406e7fb752f5dbd7ed01e6775c08505eacf","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        # specified as __file__ -\u003e object-expirer.conf value"},{"line_number":410,"context_line":"        conf \u003d {"},{"line_number":411,"context_line":"            \"__file__\": \"/etc/swift/object-expirer.conf\","},{"line_number":412,"context_line":"            \"internal_client_conf_path\": \"__file__\","},{"line_number":413,"context_line":"        }"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        with mock.patch.object("}],"source_content_type":"text/x-python","patch_set":9,"id":"77576de9_5f52ac66","line":412,"in_reply_to":"16cd2cee_85574d67","updated":"2024-09-26 20:52:44.000000000","message":"Not anymore, removed.","commit_id":"0e2391df63617c540d5c870ee00a6db070268bda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":306,"context_line":"        self.assertEqual(x.expiring_objects_account, \u0027.expiring_objects\u0027)"},{"line_number":307,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def test_guess_default_from_expirer_conf(self):"},{"line_number":310,"context_line":"        # conf read from object-expirer.conf, no internal_client_conf_path"},{"line_number":311,"context_line":"        conf \u003d {\"__file__\": \"/etc/swift/object-expirer.conf\"}"},{"line_number":312,"context_line":"        with mock.patch.object("}],"source_content_type":"text/x-python","patch_set":12,"id":"43350c12_347a1ec8","line":309,"updated":"2024-09-27 12:25:13.000000000","message":"the original test name told me what the test covered ``test_init_internal_client_path``, whereas the new name(s) just refer to ``default``. I\u0027d prefer to keep the original naming, albeit appended with the various scenarios.","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        self.assertEqual(x.expiring_objects_account, \u0027.expiring_objects\u0027)"},{"line_number":307,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def test_guess_default_from_expirer_conf(self):"},{"line_number":310,"context_line":"        # conf read from object-expirer.conf, no internal_client_conf_path"},{"line_number":311,"context_line":"        conf \u003d {\"__file__\": \"/etc/swift/object-expirer.conf\"}"},{"line_number":312,"context_line":"        with mock.patch.object("}],"source_content_type":"text/x-python","patch_set":12,"id":"cf2b6592_b007081d","line":309,"in_reply_to":"43350c12_347a1ec8","updated":"2024-09-27 18:02:15.000000000","message":"Updated.","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def test_guess_default_from_expirer_conf(self):"},{"line_number":310,"context_line":"        # conf read from object-expirer.conf, no internal_client_conf_path"},{"line_number":311,"context_line":"        conf \u003d {\"__file__\": \"/etc/swift/object-expirer.conf\"}"},{"line_number":312,"context_line":"        with mock.patch.object("},{"line_number":313,"context_line":"            expirer, \"InternalClient\", return_value\u003dself.fake_swift"},{"line_number":314,"context_line":"        ) as mock_ic:"}],"source_content_type":"text/x-python","patch_set":12,"id":"19eefd55_e02c2cf8","line":311,"updated":"2024-09-27 12:25:13.000000000","message":"ok. this is the substantive change.\n\nI think we\u0027d expect the same outcome from ``conf \u003d {}`` - could you add a case to cover that?","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def test_guess_default_from_expirer_conf(self):"},{"line_number":310,"context_line":"        # conf read from object-expirer.conf, no internal_client_conf_path"},{"line_number":311,"context_line":"        conf \u003d {\"__file__\": \"/etc/swift/object-expirer.conf\"}"},{"line_number":312,"context_line":"        with mock.patch.object("},{"line_number":313,"context_line":"            expirer, \"InternalClient\", return_value\u003dself.fake_swift"},{"line_number":314,"context_line":"        ) as mock_ic:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3ca18567_a7e733c4","line":311,"in_reply_to":"19eefd55_e02c2cf8","updated":"2024-09-27 18:02:15.000000000","message":"Done.","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                    global_conf\u003d{\"log_name\": \"object-expirer-ic\"},"},{"line_number":325,"context_line":"                )"},{"line_number":326,"context_line":"            ],"},{"line_number":327,"context_line":"        )"},{"line_number":328,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\"warning\"), [])"},{"line_number":329,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"e6f4c835_d540a2a9","line":327,"updated":"2024-09-27 12:25:13.000000000","message":"Reformatting without substantive changes to the code makes the review larger and changes the git history. I don\u0027t think anything has changed in lines 312-327 but I\u0027ve had to read the diff carefully to reach that conclusion. Sometimes it is justifiable, but in this case I\u0027m going to push back.\n\nSame for all the tests modified below...","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":324,"context_line":"                    global_conf\u003d{\"log_name\": \"object-expirer-ic\"},"},{"line_number":325,"context_line":"                )"},{"line_number":326,"context_line":"            ],"},{"line_number":327,"context_line":"        )"},{"line_number":328,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\"warning\"), [])"},{"line_number":329,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"d9b0f278_ea93f6c1","line":327,"in_reply_to":"e6f4c835_d540a2a9","updated":"2024-09-27 18:02:15.000000000","message":"Makes sense! Reverted.","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def test_override_default_from_expirer_conf(self):"},{"line_number":354,"context_line":"        # conf read from object-expirer.conf, internal_client_conf_path is"},{"line_number":355,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":356,"context_line":"        conf \u003d {"},{"line_number":357,"context_line":"            \"__file__\": \"/etc/swift/object-expirer.conf\","},{"line_number":358,"context_line":"            \"internal_client_conf_path\":"}],"source_content_type":"text/x-python","patch_set":12,"id":"266d20b6_b218c60d","line":355,"updated":"2024-09-27 12:25:13.000000000","message":"ok! this is the new behavior","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    def test_override_default_from_expirer_conf(self):"},{"line_number":354,"context_line":"        # conf read from object-expirer.conf, internal_client_conf_path is"},{"line_number":355,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":356,"context_line":"        conf \u003d {"},{"line_number":357,"context_line":"            \"__file__\": \"/etc/swift/object-expirer.conf\","},{"line_number":358,"context_line":"            \"internal_client_conf_path\":"}],"source_content_type":"text/x-python","patch_set":12,"id":"11f6dede_bc8958c9","line":355,"in_reply_to":"266d20b6_b218c60d","updated":"2024-09-27 18:02:15.000000000","message":"Acknowledged","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":354,"context_line":"        # conf read from object-expirer.conf, internal_client_conf_path is"},{"line_number":355,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":356,"context_line":"        conf \u003d {"},{"line_number":357,"context_line":"            \"__file__\": \"/etc/swift/object-expirer.conf\","},{"line_number":358,"context_line":"            \"internal_client_conf_path\":"},{"line_number":359,"context_line":"                \"/etc/swift/other-internal-client.conf\","},{"line_number":360,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":12,"id":"2cc0e62d_5972d43b","line":357,"updated":"2024-09-27 12:25:13.000000000","message":"as above, I think we also get the same outcome if the conf doesn\u0027t have \u0027__file__\u0027","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":354,"context_line":"        # conf read from object-expirer.conf, internal_client_conf_path is"},{"line_number":355,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":356,"context_line":"        conf \u003d {"},{"line_number":357,"context_line":"            \"__file__\": \"/etc/swift/object-expirer.conf\","},{"line_number":358,"context_line":"            \"internal_client_conf_path\":"},{"line_number":359,"context_line":"                \"/etc/swift/other-internal-client.conf\","},{"line_number":360,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":12,"id":"18890cf2_3247c0c3","line":357,"in_reply_to":"2cc0e62d_5972d43b","updated":"2024-09-27 18:02:15.000000000","message":"Acknowledged","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e1f2ac6f4590c8e0a52c97894543206220d8e671","unresolved":true,"context_lines":[{"line_number":404,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\"warning\"), [])"},{"line_number":405,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    def test_override_default_from_other_conf(self):"},{"line_number":408,"context_line":"        # conf read from other file, internal_client_conf_path is"},{"line_number":409,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":410,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/other-object-server.conf\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"70ef11aa_70f7342b","line":407,"updated":"2024-09-27 12:25:13.000000000","message":"Apart from my gripes about formatting and test naming, these tests look great - nicely structured and covering each scenario in a separate test.","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"dfdc230d190875b4c467e25312476f5d15100b47","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\"warning\"), [])"},{"line_number":405,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    def test_override_default_from_other_conf(self):"},{"line_number":408,"context_line":"        # conf read from other file, internal_client_conf_path is"},{"line_number":409,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":410,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/other-object-server.conf\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"4607d727_95a3505f","line":407,"in_reply_to":"70ef11aa_70f7342b","updated":"2024-09-27 18:02:15.000000000","message":"Acknowledged","commit_id":"b4ad01b3c4f0376c15ca8f6fb88cb7d88ab0de01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"b2e45d6678459d4c2f43a6be94fe584f37cd7b55","unresolved":true,"context_lines":[{"line_number":322,"context_line":"        # conf read from /etc/swift/object-expirer.conf"},{"line_number":323,"context_line":"        # -\u003e /etc/swift/object-expirer.conf"},{"line_number":324,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-expirer.conf\u0027,"},{"line_number":325,"context_line":"                \u0027internal_client_conf_path\u0027: \u0027ignored\u0027}"},{"line_number":326,"context_line":"        with mock.patch.object(expirer, \u0027InternalClient\u0027,"},{"line_number":327,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":328,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":17,"id":"c7a0ecd5_e335ac9e","side":"PARENT","line":325,"updated":"2024-09-30 20:39:32.000000000","message":"maybe keep this test, but the expected behavior is for the value to be used instead of ignored.","commit_id":"94dc4cad090390b1826bf6a2fbe5986e1516219e"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"2346f9d623217e31cb7439c10b8cdab3da3a1b3f","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        # conf read from /etc/swift/object-expirer.conf"},{"line_number":323,"context_line":"        # -\u003e /etc/swift/object-expirer.conf"},{"line_number":324,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-expirer.conf\u0027,"},{"line_number":325,"context_line":"                \u0027internal_client_conf_path\u0027: \u0027ignored\u0027}"},{"line_number":326,"context_line":"        with mock.patch.object(expirer, \u0027InternalClient\u0027,"},{"line_number":327,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":328,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":17,"id":"06ad463e_df78300d","side":"PARENT","line":325,"in_reply_to":"c7a0ecd5_e335ac9e","updated":"2024-10-01 16:26:13.000000000","message":"Added.","commit_id":"94dc4cad090390b1826bf6a2fbe5986e1516219e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2517485fba0ad75bd8aa5a1873304fac7a7ef4d0","unresolved":true,"context_lines":[{"line_number":365,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":366,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/other-object-server.conf\u0027,"},{"line_number":367,"context_line":"                \u0027internal_client_conf_path\u0027:"},{"line_number":368,"context_line":"                    \u0027/etc/swift/other-internal-client.conf\u0027}"},{"line_number":369,"context_line":"        with mock.patch.object(expirer, \u0027InternalClient\u0027,"},{"line_number":370,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":371,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":19,"id":"fe8755e8_7f182ddb","side":"PARENT","line":368,"updated":"2024-10-02 08:36:32.000000000","message":"I didn\u0027t notice this on earlier review - we\u0027ve lost this case, but I think it is useful to cover. I guess in the spirit of the naming scheme it would be something like\n\n```\ntest_init_internal_client_path_from_other_and_other_conf\n```","commit_id":"94dc4cad090390b1826bf6a2fbe5986e1516219e"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"0d1a32c32613358739035d8f2a41212155ac4e72","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        # specified -\u003e internal_client_conf_path value"},{"line_number":366,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/other-object-server.conf\u0027,"},{"line_number":367,"context_line":"                \u0027internal_client_conf_path\u0027:"},{"line_number":368,"context_line":"                    \u0027/etc/swift/other-internal-client.conf\u0027}"},{"line_number":369,"context_line":"        with mock.patch.object(expirer, \u0027InternalClient\u0027,"},{"line_number":370,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":371,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":19,"id":"147a0723_fa417613","side":"PARENT","line":368,"in_reply_to":"fe8755e8_7f182ddb","updated":"2024-10-02 16:02:02.000000000","message":"Added.","commit_id":"94dc4cad090390b1826bf6a2fbe5986e1516219e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2517485fba0ad75bd8aa5a1873304fac7a7ef4d0","unresolved":true,"context_lines":[{"line_number":319,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\u0027warning\u0027), [])"},{"line_number":320,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def test_init_internal_client_path_from_internal__conf(self):"},{"line_number":323,"context_line":"        # conf read from /etc/swift/object-expirer.conf"},{"line_number":324,"context_line":"        # -\u003e /etc/swift/object-expirer.conf"},{"line_number":325,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-expirer.conf\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"f9c65b49_1631de64","line":322,"range":{"start_line":322,"start_character":52,"end_line":322,"end_character":55},"updated":"2024-10-02 08:36:32.000000000","message":"typo \n\ns/__/_/\n\nactually i wonder if you had intended to name this\n\n```\ntest_init_internal_client_path_from_internal_and_other_conf\n```\n\nto match line 351","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"0d1a32c32613358739035d8f2a41212155ac4e72","unresolved":false,"context_lines":[{"line_number":319,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\u0027warning\u0027), [])"},{"line_number":320,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def test_init_internal_client_path_from_internal__conf(self):"},{"line_number":323,"context_line":"        # conf read from /etc/swift/object-expirer.conf"},{"line_number":324,"context_line":"        # -\u003e /etc/swift/object-expirer.conf"},{"line_number":325,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-expirer.conf\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"4f0390cf_c2b069eb","line":322,"range":{"start_line":322,"start_character":52,"end_line":322,"end_character":55},"in_reply_to":"f9c65b49_1631de64","updated":"2024-10-02 16:02:02.000000000","message":"Updated.","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2517485fba0ad75bd8aa5a1873304fac7a7ef4d0","unresolved":true,"context_lines":[{"line_number":323,"context_line":"        # conf read from /etc/swift/object-expirer.conf"},{"line_number":324,"context_line":"        # -\u003e /etc/swift/object-expirer.conf"},{"line_number":325,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-expirer.conf\u0027,"},{"line_number":326,"context_line":"                \u0027internal_client_conf_path\u0027: \u0027ignored\u0027}"},{"line_number":327,"context_line":"        with mock.patch.object(expirer, \u0027InternalClient\u0027,"},{"line_number":328,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":329,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":19,"id":"2720900a_786d0c97","line":326,"range":{"start_line":326,"start_character":45,"end_line":326,"end_character":54},"updated":"2024-10-02 08:36:32.000000000","message":"this option is no longer ignored so it would avoid confusion to give it a different value e..g. ``/etc/swift/other-internal-client.conf``","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"0d1a32c32613358739035d8f2a41212155ac4e72","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        # conf read from /etc/swift/object-expirer.conf"},{"line_number":324,"context_line":"        # -\u003e /etc/swift/object-expirer.conf"},{"line_number":325,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-expirer.conf\u0027,"},{"line_number":326,"context_line":"                \u0027internal_client_conf_path\u0027: \u0027ignored\u0027}"},{"line_number":327,"context_line":"        with mock.patch.object(expirer, \u0027InternalClient\u0027,"},{"line_number":328,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":329,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"}],"source_content_type":"text/x-python","patch_set":19,"id":"a2a020b8_ac23523a","line":326,"range":{"start_line":326,"start_character":45,"end_line":326,"end_character":54},"in_reply_to":"2720900a_786d0c97","updated":"2024-10-02 16:02:02.000000000","message":"Updated","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2517485fba0ad75bd8aa5a1873304fac7a7ef4d0","unresolved":true,"context_lines":[{"line_number":334,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\u0027warning\u0027), [])"},{"line_number":335,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def test_init_internal_client_path_from_server_conf(self):"},{"line_number":338,"context_line":"        # conf read from object-server.conf, no internal_client_conf_path"},{"line_number":339,"context_line":"        # specified -\u003e /etc/swift/internal-client.conf"},{"line_number":340,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-server.conf\u0027}"}],"source_content_type":"text/x-python","patch_set":19,"id":"9e4b9e0d_91b93431","line":337,"updated":"2024-10-02 08:36:32.000000000","message":"+1, IIRC maybe I wrote the original test - you\u0027ve improved it by breaking up into tests for each scenario, thanks","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"0d1a32c32613358739035d8f2a41212155ac4e72","unresolved":false,"context_lines":[{"line_number":334,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\u0027warning\u0027), [])"},{"line_number":335,"context_line":"        self.assertIs(x.swift, self.fake_swift)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def test_init_internal_client_path_from_server_conf(self):"},{"line_number":338,"context_line":"        # conf read from object-server.conf, no internal_client_conf_path"},{"line_number":339,"context_line":"        # specified -\u003e /etc/swift/internal-client.conf"},{"line_number":340,"context_line":"        conf \u003d {\u0027__file__\u0027: \u0027/etc/swift/object-server.conf\u0027}"}],"source_content_type":"text/x-python","patch_set":19,"id":"172aaf2a_6f72a0b1","line":337,"in_reply_to":"9e4b9e0d_91b93431","updated":"2024-10-02 16:02:02.000000000","message":"Acknowledged","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2517485fba0ad75bd8aa5a1873304fac7a7ef4d0","unresolved":true,"context_lines":[{"line_number":370,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":371,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"},{"line_number":372,"context_line":"        self.assertEqual(mock_ic.mock_calls, [mock.call("},{"line_number":373,"context_line":"            \u0027/etc/swift/object-expirer.conf\u0027, \u0027Swift Object Expirer\u0027, 3,"},{"line_number":374,"context_line":"            use_replication_network\u003dTrue,"},{"line_number":375,"context_line":"            global_conf\u003d{\u0027log_name\u0027: \u0027object-expirer-ic\u0027})])"},{"line_number":376,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\u0027warning\u0027), [])"}],"source_content_type":"text/x-python","patch_set":19,"id":"9c665700_b61f84ca","line":373,"updated":"2024-10-02 08:36:32.000000000","message":"ok, so empty conf means no ``__file__`` so the expirer defaults to assume the legacy object-expirer.conf","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"0d1a32c32613358739035d8f2a41212155ac4e72","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                               return_value\u003dself.fake_swift) as mock_ic:"},{"line_number":371,"context_line":"            x \u003d expirer.ObjectExpirer(conf, logger\u003dself.logger)"},{"line_number":372,"context_line":"        self.assertEqual(mock_ic.mock_calls, [mock.call("},{"line_number":373,"context_line":"            \u0027/etc/swift/object-expirer.conf\u0027, \u0027Swift Object Expirer\u0027, 3,"},{"line_number":374,"context_line":"            use_replication_network\u003dTrue,"},{"line_number":375,"context_line":"            global_conf\u003d{\u0027log_name\u0027: \u0027object-expirer-ic\u0027})])"},{"line_number":376,"context_line":"        self.assertEqual(self.logger.get_lines_for_level(\u0027warning\u0027), [])"}],"source_content_type":"text/x-python","patch_set":19,"id":"a5f36e32_cf1a9850","line":373,"in_reply_to":"9c665700_b61f84ca","updated":"2024-10-02 16:02:02.000000000","message":"Acknowledged","commit_id":"29dfe2b875e4b63e03d63e782d0962b12d2f30cd"}]}