)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d8022fe38d2473b64c47b9de85f11818aa8e9736","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7dc95756_30a29a66","updated":"2024-05-17 06:16:11.000000000","message":"Thank you for inputs!","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"57585b3ad01c8c619ab8d72ac7c863a3c044c1be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"586b509e_f2ef95a3","updated":"2024-05-16 14:40:45.000000000","message":"thank you for inputs!","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c27f1782fd4999566644931bdc3fe8e54e02f7a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c03a2521_90de24c7","updated":"2024-05-17 13:51:57.000000000","message":"-1 is for the code that can\u0027t work, the other is just a comment.","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9c58724c92616225ef0787827e6c7440508d814e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"da26b547_62a93e3f","updated":"2024-05-17 06:16:33.000000000","message":"Need to move it to oslo.middleware","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"991b2544a988dcbdd02d2ce1d13f3a406112c7ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"32248910_81ae2aad","updated":"2024-05-17 14:02:44.000000000","message":"Thank you Dan for the inputs, will make changes in a patch where I will move this in Oslo middleware as plugin.","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"}],"glance/api/middleware/healthcheck.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f60bdd854427dbebc030d9cbc323b3983cfd9975","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"MOUNT_PATH_OPTS \u003d ["},{"line_number":9,"context_line":"    cfg.ListOpt(\u0027available_mount_paths\u0027,"},{"line_number":10,"context_line":"                default\u003d[],"},{"line_number":11,"context_line":"                help\u003d\u0027Check whether specified path is mounted or not\u0027),"},{"line_number":12,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"32ec05da_82e6b038","line":9,"updated":"2024-05-16 14:35:24.000000000","message":"So this will be a list of actual filenames, right? So something like `/var/lib/glance/images/.marker` ?\n\nI was thinking you would generate this automatically from the store\u0027s directory and some pre-defined marker file name. But, this is also simple and might be useful for people even if they\u0027re not using NFS. Basically just like disable-by-file but ... disable-if-missing.","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"57585b3ad01c8c619ab8d72ac7c863a3c044c1be","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"MOUNT_PATH_OPTS \u003d ["},{"line_number":9,"context_line":"    cfg.ListOpt(\u0027available_mount_paths\u0027,"},{"line_number":10,"context_line":"                default\u003d[],"},{"line_number":11,"context_line":"                help\u003d\u0027Check whether specified path is mounted or not\u0027),"},{"line_number":12,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"cacc01cf_04bce513","line":9,"in_reply_to":"32ec05da_82e6b038","updated":"2024-05-16 14:40:45.000000000","message":"yes, it will be something like /var/lib/glance/images/.marker\n\nI will change the Name of plugin and config option name to make it generic.","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c27f1782fd4999566644931bdc3fe8e54e02f7a","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":""},{"line_number":8,"context_line":"MOUNT_PATH_OPTS \u003d ["},{"line_number":9,"context_line":"    cfg.ListOpt(\u0027available_mount_paths\u0027,"},{"line_number":10,"context_line":"                default\u003d[],"},{"line_number":11,"context_line":"                help\u003d\u0027Check whether specified path is mounted or not\u0027),"},{"line_number":12,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"d906b2fa_772b2cbc","line":9,"in_reply_to":"cacc01cf_04bce513","updated":"2024-05-17 13:51:57.000000000","message":"Done","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f60bdd854427dbebc030d9cbc323b3983cfd9975","unresolved":true,"context_lines":[{"line_number":39,"context_line":"        self.mount_paths \u003d self._conf_get(\u0027available_mount_paths\u0027)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def healthcheck(self, server_port):"},{"line_number":42,"context_line":"        if not self.mount_paths:"},{"line_number":43,"context_line":"            LOG.warning(\u0027MountPath healthcheck middleware enabled \u0027"},{"line_number":44,"context_line":"                        \u0027without available_mount_paths set\u0027)"},{"line_number":45,"context_line":"            return pluginbase.HealthcheckResult("}],"source_content_type":"text/x-python","patch_set":1,"id":"c1486a07_ef4b4012","line":42,"updated":"2024-05-16 14:35:24.000000000","message":"As mentioned, I would make this not a warning. Let people always have this healthcheck enabled and just opt-in to the checking (if they\u0027re using NFS, etc) by configuring a path here.","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"57585b3ad01c8c619ab8d72ac7c863a3c044c1be","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        self.mount_paths \u003d self._conf_get(\u0027available_mount_paths\u0027)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def healthcheck(self, server_port):"},{"line_number":42,"context_line":"        if not self.mount_paths:"},{"line_number":43,"context_line":"            LOG.warning(\u0027MountPath healthcheck middleware enabled \u0027"},{"line_number":44,"context_line":"                        \u0027without available_mount_paths set\u0027)"},{"line_number":45,"context_line":"            return pluginbase.HealthcheckResult("}],"source_content_type":"text/x-python","patch_set":1,"id":"af7aed2d_2a2a3dc6","line":42,"in_reply_to":"c1486a07_ef4b4012","updated":"2024-05-16 14:40:45.000000000","message":"Acknowledged","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f60bdd854427dbebc030d9cbc323b3983cfd9975","unresolved":true,"context_lines":[{"line_number":46,"context_line":"                available\u003dTrue, reason\u003d\"OK\","},{"line_number":47,"context_line":"                details\u003d\"No \u0027available_mount_paths\u0027 configuration value\""},{"line_number":48,"context_line":"                        \" specified\")"},{"line_number":49,"context_line":"        for mount_path in self.mount_paths:"},{"line_number":50,"context_line":"            if os.path.ismount(mount_path):"},{"line_number":51,"context_line":"                return pluginbase.HealthcheckResult("},{"line_number":52,"context_line":"                    available\u003dTrue, reason\u003d\"OK\","}],"source_content_type":"text/x-python","patch_set":1,"id":"128b626e_c8da0330","line":49,"updated":"2024-05-16 14:35:24.000000000","message":"This will only ever hit one path because you always return. I think this should (per above) loop over all the files, return unhealthy if any are missing, and if we drop out the bottom of the loop, then return OK. That will also mean if the list is empty, we just return OK.","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"57585b3ad01c8c619ab8d72ac7c863a3c044c1be","unresolved":false,"context_lines":[{"line_number":46,"context_line":"                available\u003dTrue, reason\u003d\"OK\","},{"line_number":47,"context_line":"                details\u003d\"No \u0027available_mount_paths\u0027 configuration value\""},{"line_number":48,"context_line":"                        \" specified\")"},{"line_number":49,"context_line":"        for mount_path in self.mount_paths:"},{"line_number":50,"context_line":"            if os.path.ismount(mount_path):"},{"line_number":51,"context_line":"                return pluginbase.HealthcheckResult("},{"line_number":52,"context_line":"                    available\u003dTrue, reason\u003d\"OK\","}],"source_content_type":"text/x-python","patch_set":1,"id":"ce4512b5_7935b5b8","line":49,"in_reply_to":"128b626e_c8da0330","updated":"2024-05-16 14:40:45.000000000","message":"Acknowledged","commit_id":"e318e652eae43ce6ed544f8a5dd31781c3c82a8b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c27f1782fd4999566644931bdc3fe8e54e02f7a","unresolved":true,"context_lines":[{"line_number":53,"context_line":"        self.available_paths \u003d self._conf_get(\u0027available_file_paths\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def healthcheck(self, server_port):"},{"line_number":56,"context_line":"        if not self.avaialble_paths:"},{"line_number":57,"context_line":"            return pluginbase.HealthcheckResult("},{"line_number":58,"context_line":"                available\u003dTrue, reason\u003d\"OK\","},{"line_number":59,"context_line":"                details\u003d\"No \u0027available_file_paths\u0027 configuration value\""}],"source_content_type":"text/x-python","patch_set":2,"id":"25360896_41fcda0b","line":56,"range":{"start_line":56,"start_character":20,"end_line":56,"end_character":35},"updated":"2024-05-17 13:51:57.000000000","message":"`NameError` :(\n\nHowever, I\u0027m not sure you need a separate clause for the \"none configured\" case. Are you just doing this so that the operator may look at the result and see \"oh I *meant* to configure some\" ? If you remove this, you\u0027ll fall down to L72 and return OK.","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"991b2544a988dcbdd02d2ce1d13f3a406112c7ca","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.available_paths \u003d self._conf_get(\u0027available_file_paths\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def healthcheck(self, server_port):"},{"line_number":56,"context_line":"        if not self.avaialble_paths:"},{"line_number":57,"context_line":"            return pluginbase.HealthcheckResult("},{"line_number":58,"context_line":"                available\u003dTrue, reason\u003d\"OK\","},{"line_number":59,"context_line":"                details\u003d\"No \u0027available_file_paths\u0027 configuration value\""}],"source_content_type":"text/x-python","patch_set":2,"id":"8d152479_045d1122","line":56,"range":{"start_line":56,"start_character":20,"end_line":56,"end_character":35},"in_reply_to":"25360896_41fcda0b","updated":"2024-05-17 14:02:44.000000000","message":"Yeah, just to remind operator may be he missed to configure something, but I agree, this is not necessary, I will remove this.","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c27f1782fd4999566644931bdc3fe8e54e02f7a","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            return pluginbase.HealthcheckResult("},{"line_number":68,"context_line":"                available\u003dFalse, reason\u003d\"MOUNT PATH DISABLED\","},{"line_number":69,"context_line":"                details\u003d\"Mentioned file paths \u0027%s\u0027 are not \""},{"line_number":70,"context_line":"                        \"available\" % \",\".join(missing_file_paths))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        return pluginbase.HealthcheckResult("},{"line_number":73,"context_line":"            available\u003dTrue, reason\u003d\"OK\","}],"source_content_type":"text/x-python","patch_set":2,"id":"4057b108_5f79dede","line":70,"updated":"2024-05-17 13:51:57.000000000","message":"You might consider not doing this as it kinda exposes some data from the healthcheck, which you don\u0027t want to do. If the mount point has something sensitive in it, for example.\n\n`/srv/fileserver.example.org/images`\n\nA log message about which one was missing would be fine for an operator trying to determine why the healthcheck is failing, if it\u0027s not obvious.\n\nI said it before, but just one missing path is enough to return failure here, so no real need to check the others, and thus no need to collect the list of ones missing I think. Is there some reason you think you need to check them all in parallel and report them?","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"991b2544a988dcbdd02d2ce1d13f3a406112c7ca","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            return pluginbase.HealthcheckResult("},{"line_number":68,"context_line":"                available\u003dFalse, reason\u003d\"MOUNT PATH DISABLED\","},{"line_number":69,"context_line":"                details\u003d\"Mentioned file paths \u0027%s\u0027 are not \""},{"line_number":70,"context_line":"                        \"available\" % \",\".join(missing_file_paths))"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        return pluginbase.HealthcheckResult("},{"line_number":73,"context_line":"            available\u003dTrue, reason\u003d\"OK\","}],"source_content_type":"text/x-python","patch_set":2,"id":"ba160f6d_6310a9ca","line":70,"in_reply_to":"4057b108_5f79dede","updated":"2024-05-17 14:02:44.000000000","message":"I will return at first failure and will just log a message rather than returning it above.","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"991b2544a988dcbdd02d2ce1d13f3a406112c7ca","unresolved":true,"context_lines":[{"line_number":72,"context_line":"        return pluginbase.HealthcheckResult("},{"line_number":73,"context_line":"            available\u003dTrue, reason\u003d\"OK\","},{"line_number":74,"context_line":"            details\u003d\"Specified file paths are \""},{"line_number":75,"context_line":"                    \"available\" % \",\".join(self.available_paths))"}],"source_content_type":"text/x-python","patch_set":2,"id":"2952fb8b_af42b729","line":75,"range":{"start_line":75,"start_character":37,"end_line":75,"end_character":63},"updated":"2024-05-17 14:02:44.000000000","message":"Will remove this as well.","commit_id":"b509c9f31fbdade3d8251aa8ebd3e78bf17b9520"}]}