)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"7fb2bc8f61af69809ce39a2c161f34d949bc758d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1d8c3f84_506b1950","updated":"2024-05-20 14:13:20.000000000","message":"I agree with the overall direction. Leaving a note about the config example and probably a better naming which is consistent with the existing plugin.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"7f4cee8da3ef5f85dcbf08d0fcd931d0ee0ef2b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"363bb340_9381741a","updated":"2024-05-17 15:22:27.000000000","message":"I\u0027m not expert of this kind of oslo.middleware feature, but at first glance, it seems to make sense, I think I still need to think twice about this proposal though.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"e2c7e6602dd231a266fe3c77f81e7a03e29e5e2d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ccb89bc5_360946eb","updated":"2024-05-17 15:36:23.000000000","message":"I\u0027m wondering if the plugin should be maintained in glance repository. The described use case is quite generic but has tradeoff with easiness of configurations. For example if we implement a glance specific healthcheck plugin then you may load glance options to determine the required file paths. It may later allow us to implement store backend specific logic which can be switched based on the glance\u0027s configuration.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dfb6a7ca3d30905141d4434390b4bab5d58b4ae4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"147cbae1_98b7e8b1","updated":"2024-05-20 14:18:29.000000000","message":"Thanks!","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8cd79631e79b9ffc3addcc1904df4ead5039be14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6f02a380_37ba83fe","updated":"2024-05-20 14:30:45.000000000","message":"thank you Dan and Kajinami san for inputs.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dfb6a7ca3d30905141d4434390b4bab5d58b4ae4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"90b6fb50_fb6a7912","in_reply_to":"23e8f30a_3da89d5b","updated":"2024-05-20 14:18:29.000000000","message":"Ack, I think \"specless\" makes sense to me if it\u0027s not required...","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"787b650c92c0b191f76fb5e2475ebeb73ad57bdd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8f78c4e0_eb189a2f","in_reply_to":"2c297825_3333e1c5","updated":"2024-05-18 15:32:51.000000000","message":"Well, in the glance case, it\u0027s pretty clear, I think. Glance itself would not tell operators which paths to put here, but rather they would configure these to be similar to (the parent of) their filesystem repositories. It\u0027s not about a service\u0027s \"need\", it\u0027s about the operator\u0027s \"need.\" So if I have three repos in my glance, say one NFS, one cephfs and one local filesystem, I would likely put the two remote filesystems\u0027 mount points in the list of things to check for health. So it\u0027s not like we\u0027d tell the operators that \"when using this with glance, always put X in the health-unless list\", it would be the kind of thing they\u0027d use in addition, if they wanted glance to be considered unhealthy if that mount went away.\n\nYou\u0027re right that the deployment tool may not (likely won\u0027t) know which things to automatically put there, not only because of nfs-vs-local but just because the operator may or may not want the behavior at all. If you don\u0027t want your glance to become unhealthy if the NFS mount disappears (like because restarting the container won\u0027t fix anything) then you wouldn\u0027t even want NFS things here.\n\nAnyway, if you\u0027re willing, my vote is to put this in oslo so multiple services can use it, and of course, we\u0027ll have to document the proper usage in the service docs.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8aa19aa3b5798da739038b457edbf987b773b101","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2c297825_3333e1c5","in_reply_to":"60b02a32_ae8e3cad","updated":"2024-05-18 02:26:23.000000000","message":"\u003e There\u0027s already healthy-unless-file in oslo, right? This is pretty much the inverse of that logic, and with an array of files. Seems appropriate to be in oslo to me...\n\nhealth-useless-file and health-if-file are similar but different. health-useless-file is not much related to the services because the file is just a flag, while health-if-file is heavily dependent on the requirement by services and needs correct understanding about the requirement.\n\nMy concern is that not very many operators may understand the correct list of file paths a specific service needs. A deployment tool may need to create a list of file paths used by each service which needs an external logic which simulate the behavior of services based on config options. For example /var/lib/glance/images may need only when nfs backend is used and /var/lib/glance/cache may need only when image cache is used. The path might be customized in glace-api.conf and these values need to be adjusted.\n\nI\u0027m ok with having the bare base implementation in oslo, but we may still have to check how this would be configured in real deployments and how (easily) operators may use it.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"33444634c8ee7e343d37ba3500ce04310ea0ff9b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"60b02a32_ae8e3cad","in_reply_to":"6cdf029e_cf1457b7","updated":"2024-05-17 19:08:07.000000000","message":"Yeah, something like this could be used for nova-compute if/when it supports the oslo healthcheck pattern on nova-compute. We have the same problem where if our `instances` directory goes away, we need to stop advertising ourselves as healthy.\n\nThere\u0027s already healthy-unless-file in oslo, right? This is pretty much the inverse of that logic, and with an array of files. Seems appropriate to be in oslo to me...\n\nObviously the easiest thing (for glance) is to just do it in our own repo, but it would be unfortunate to have more than one project implement something like this in their own repo.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"7fb2bc8f61af69809ce39a2c161f34d949bc758d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"23e8f30a_3da89d5b","in_reply_to":"8f78c4e0_eb189a2f","updated":"2024-05-20 14:13:20.000000000","message":"The opt-in behavior can be achieved by making the healthcheck plugin optional and let them add these plugins when they need (I think that\u0027s what was done with the octavia\u0027s plugin).\n\nBut I agree we can start with the bare plugin in oslo and can extend/refine it here or in glance in the future if needed. We can probably omit the spec given the limited scope of this work, but we can merge this as we already have the base spec.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8cd79631e79b9ffc3addcc1904df4ead5039be14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c265437b_607ded28","in_reply_to":"90b6fb50_fb6a7912","updated":"2024-05-20 14:30:45.000000000","message":"Ack, I will change the name to enable_by_files and then submit the patch in oslo.middleware","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9ffb2a0e31b082dffbced2f1ccdfe5b05db4e25a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6cdf029e_cf1457b7","in_reply_to":"9ef35885_99f18fbe","updated":"2024-05-17 15:41:52.000000000","message":"Initially I thought it to have in glance itself, if you see PoC example I have added it in glance only,  but I think in future other projects also use it if added in oslo middleware","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"6d56f660902053e852d18d0bf16180375304fd43","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9ef35885_99f18fbe","in_reply_to":"ccb89bc5_360946eb","updated":"2024-05-17 15:38:59.000000000","message":"For example octavia maintains its in-tree healthcheck plugin","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"00a1f097a0f90b61a353418083aac88d9cfe1285","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b1eed0d9_433e7956","updated":"2024-06-24 08:12:06.000000000","message":"Daniel, Thank you for review,\nJust changed the Name of the healthcheck plugin as suggested by Kajinami san!\n\nRest of the design is same. Sorry for inconvenience.","commit_id":"0075c4e56652e662bbea22a6d4e6b4a6bff07b93"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"aa8d190303776fc740bd79d407b6cc56fbfb36fe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b32aa065_d964c389","updated":"2024-07-01 14:58:35.000000000","message":"Thank you for inputs, please provide me some suggestions on Existing API section","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4fae4376fa04759bad5a96b5aa26fa2a4513f3f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a9fede49_e9110a29","updated":"2024-07-01 16:40:06.000000000","message":"Thank you for the inputs.","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"72d3d8ad8578cde7c11afaa941ab3caa4400e914","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"50ba6bf3_ea4aea81","updated":"2024-07-02 08:02:24.000000000","message":"Thank you Hervé for review!!\nAppreciate your inputs!!","commit_id":"e115ac787ed257afd3d4e22e682d44097871245c"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"e23c1da648c8abf9a6b9e9ebcdee83b99815687c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d1cd30b6_915a2d7b","updated":"2024-07-02 07:04:19.000000000","message":"excellent, thank you for your answers","commit_id":"e115ac787ed257afd3d4e22e682d44097871245c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"69a6f3331020214548899ed7f00b207c2b935d36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"cd8aad74_fc2e3dc0","updated":"2024-07-03 03:45:50.000000000","message":"@Hervé Beraud\n\nChanged the spec to remove warning will be logged if both backends configured at once.","commit_id":"5c6d98552ff766972b682a6d527bbb65077b07ed"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"0c3adc74fce2e2cca5dc8d3e13250fbd5ac9cfa2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"e97fa377_5cb19a4a","updated":"2024-07-03 07:23:33.000000000","message":"Excellent thanks.\nWell done","commit_id":"5c6d98552ff766972b682a6d527bbb65077b07ed"}],"specs/dalmatian/disable_if_missing_healthcheck_plugin.rst":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"e2c7e6602dd231a266fe3c77f81e7a03e29e5e2d","unresolved":true,"context_lines":[{"line_number":30,"context_line":".. code-block:: ini"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"  [app:healthcheck]"},{"line_number":33,"context_line":"  paste.app_factory \u003d oslo_middleware:Healthcheck.app_factory"},{"line_number":34,"context_line":"  backends \u003d disable_if_missing (optional, default: empty)"},{"line_number":35,"context_line":"  # used by the \u0027disable_if_missing\u0027 backend"},{"line_number":36,"context_line":"  available_file_paths \u003d /var/lib/glance/images,/var/lib/glance/cache (optional, default: empty)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"  # Use this composite for keystone auth with caching and cache management"},{"line_number":39,"context_line":"  [composite:glance-api-keystone+cachemanagement]"}],"source_content_type":"text/x-rst","patch_set":1,"id":"ddd695b3_07d0d63a","line":36,"range":{"start_line":33,"start_character":61,"end_line":36,"end_character":96},"updated":"2024-05-17 15:36:23.000000000","message":"IIUC these can be configured in glance-api.conf, in [oslo.middleware] section, which is prefered way","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8cd79631e79b9ffc3addcc1904df4ead5039be14","unresolved":false,"context_lines":[{"line_number":30,"context_line":".. code-block:: ini"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"  [app:healthcheck]"},{"line_number":33,"context_line":"  paste.app_factory \u003d oslo_middleware:Healthcheck.app_factory"},{"line_number":34,"context_line":"  backends \u003d disable_if_missing (optional, default: empty)"},{"line_number":35,"context_line":"  # used by the \u0027disable_if_missing\u0027 backend"},{"line_number":36,"context_line":"  available_file_paths \u003d /var/lib/glance/images,/var/lib/glance/cache (optional, default: empty)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"  # Use this composite for keystone auth with caching and cache management"},{"line_number":39,"context_line":"  [composite:glance-api-keystone+cachemanagement]"}],"source_content_type":"text/x-rst","patch_set":1,"id":"451900e7_98728450","line":36,"range":{"start_line":33,"start_character":61,"end_line":36,"end_character":96},"in_reply_to":"201841b9_f3bb194a","updated":"2024-05-20 14:30:45.000000000","message":"Acknowledged","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dfb6a7ca3d30905141d4434390b4bab5d58b4ae4","unresolved":true,"context_lines":[{"line_number":30,"context_line":".. code-block:: ini"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"  [app:healthcheck]"},{"line_number":33,"context_line":"  paste.app_factory \u003d oslo_middleware:Healthcheck.app_factory"},{"line_number":34,"context_line":"  backends \u003d disable_if_missing (optional, default: empty)"},{"line_number":35,"context_line":"  # used by the \u0027disable_if_missing\u0027 backend"},{"line_number":36,"context_line":"  available_file_paths \u003d /var/lib/glance/images,/var/lib/glance/cache (optional, default: empty)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"  # Use this composite for keystone auth with caching and cache management"},{"line_number":39,"context_line":"  [composite:glance-api-keystone+cachemanagement]"}],"source_content_type":"text/x-rst","patch_set":1,"id":"201841b9_f3bb194a","line":36,"range":{"start_line":33,"start_character":61,"end_line":36,"end_character":96},"in_reply_to":"bbb1bed3_30150225","updated":"2024-05-20 14:18:29.000000000","message":"We want to check for a file (not a directory) but I think `enable_by_files` makes sense and mirrors the inverse behavior name.","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"7fb2bc8f61af69809ce39a2c161f34d949bc758d","unresolved":true,"context_lines":[{"line_number":30,"context_line":".. code-block:: ini"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"  [app:healthcheck]"},{"line_number":33,"context_line":"  paste.app_factory \u003d oslo_middleware:Healthcheck.app_factory"},{"line_number":34,"context_line":"  backends \u003d disable_if_missing (optional, default: empty)"},{"line_number":35,"context_line":"  # used by the \u0027disable_if_missing\u0027 backend"},{"line_number":36,"context_line":"  available_file_paths \u003d /var/lib/glance/images,/var/lib/glance/cache (optional, default: empty)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"  # Use this composite for keystone auth with caching and cache management"},{"line_number":39,"context_line":"  [composite:glance-api-keystone+cachemanagement]"}],"source_content_type":"text/x-rst","patch_set":1,"id":"bbb1bed3_30150225","line":36,"range":{"start_line":33,"start_character":61,"end_line":36,"end_character":96},"in_reply_to":"ddd695b3_07d0d63a","updated":"2024-05-20 14:13:20.000000000","message":"Also, I\u0027d suggest the following names, based on the existing namings.\n * enable_by_files (or probably enable_by_directories ?) plugin\n * enable_file_paths (or probably enable_direcory_paths) options","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"7f4cee8da3ef5f85dcbf08d0fcd931d0ee0ef2b3","unresolved":true,"context_lines":[{"line_number":44,"context_line":"The middleware will return \"200 OK\" if everything is OK,"},{"line_number":45,"context_line":"or \"503 \u003cREASON\u003e\" if not with the reason of why this API should not be used."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"\"backends\" will the name of a stevedore extentions in the namespace"},{"line_number":48,"context_line":"\"oslo.middleware.healthcheck\"."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c27b09a1_5e08d9b8","line":47,"range":{"start_line":47,"start_character":11,"end_line":47,"end_character":17},"updated":"2024-05-17 15:22:27.000000000","message":"be?","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8cd79631e79b9ffc3addcc1904df4ead5039be14","unresolved":false,"context_lines":[{"line_number":44,"context_line":"The middleware will return \"200 OK\" if everything is OK,"},{"line_number":45,"context_line":"or \"503 \u003cREASON\u003e\" if not with the reason of why this API should not be used."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"\"backends\" will the name of a stevedore extentions in the namespace"},{"line_number":48,"context_line":"\"oslo.middleware.healthcheck\"."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":1,"id":"95ef3068_0e457992","line":47,"range":{"start_line":47,"start_character":11,"end_line":47,"end_character":17},"in_reply_to":"c27b09a1_5e08d9b8","updated":"2024-05-20 14:30:45.000000000","message":"Acknowledged","commit_id":"c37bf6b4e7ce139a59108dc187d1ba0e7f0d1410"}],"specs/dalmatian/enable_by_files_healthcheck_plugin.rst":[{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"bebac008cf07a3d3970cc7fb24d476c0663c3b73","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"6f21d408_beed853d","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"updated":"2024-07-01 14:53:02.000000000","message":"Are you sure that this new plugin won\u0027t change the existing API?\nIMO it would at least add new related options and perhaps a dedicated submodule, isn\u0027t?\n\nCould you please give an overview of the new API provided by this new plugin and the potential changes on the existing API (perhaps you have to add new arguments to existing methods or things like that, don\u0027t know).\n\nThis overview could be reused during the implementation step and then allow to gain time.","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"4fae4376fa04759bad5a96b5aa26fa2a4513f3f7","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5f0cffbc_8833107b","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"in_reply_to":"0ab5639a_8d801165","updated":"2024-07-01 16:40:06.000000000","message":"\u003e Do this change will add new public API endpoints?\n\u003e I think that yes.\n\u003e \n\u003e By example if I take the `disable_by_file` submodule, this one seems to be a \"backend\" like yours: https://github.com/openstack/oslo.middleware/blob/master/oslo_middleware/healthcheck/disable_by_file.py\n\u003e \n\u003e So your module path will surely looks like to something like:\n\u003e \n\u003e `oslo_middleware.healthcheck.enable_by_files`\n\u003e \n\u003e and it will surely provide the a class named `EnabledByFileHealthcheck` with a method named `healthcheck` which will be the endpoints called by when this backend will be loaded. \n\n\nYes, this is correct.\n\nPerhaps you also want to implement another class like the `DisableByFilesPortsHealthcheck` which is provided the `disable_by_file` backend, do not know. That\u0027s the kind of question to highlight there.\n\nNo, this is not current requirement as of now.\n\u003e \n\u003e Also, adding this new backend would also mean adding a new option to the opt of the healthcheck module like the with the disable_by_file backend:\n\u003e \n\u003e https://github.com/openstack/oslo.middleware/blob/e2ea8dc556a7e8944b923ffd2a0028495bb60b5e/oslo_middleware/healthcheck/opts.py#L43\n\nYes, this is true.\n\n\u003e \n\u003e One last thing, is that `enable_by_files` and `disable_by_file` both semantics looks close to each other, maybe it could be possible to share common code between the both features, and in this case, your addition would also impact the `disable_by_file` API, I do not know,.\n\u003e \n\nPrevious suggestion was to keep it different and that is why opted this way.\n\n\n\u003e Let me know if you have questions.\n\nSo i will modify this section to mention that it will have the new API impact.","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"ab6c4719baeb72a0d09e150da2d08cc977698df3","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"575f8367_317e3cc7","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"in_reply_to":"4e335afd_1751486d","updated":"2024-07-01 19:15:50.000000000","message":"ok, then, as we only own one plugin for now, that\u0027s a point of attention to have during the implementation step","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ae01bf3a737693114b8c84de7b927a9caa07e39c","unresolved":false,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"042b6627_d4273285","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"in_reply_to":"575f8367_317e3cc7","updated":"2024-07-02 05:32:59.000000000","message":"Acknowledged","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"6a00b593e6afb90a27f5288ee28857d6a3866b66","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"aa0e6010_57a02eda","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"in_reply_to":"5f0cffbc_8833107b","updated":"2024-07-01 16:58:10.000000000","message":"Just for my personal understanding, at the rest API level, this backend will be available and reachable by the `healthcheck` endpoint, exact?\n\nExample:\n\n`curl -X GET -i -H \"Accept: text/html\" \"http://0.0.0.0:8775/healthcheck\"`\n\nFor now, as far as I can see, we only have the `disable_by_file` plugin so there is not questions about the result returned by targeting this http endpoint (the one in the curl example above), but what if both backends are enabled at the same time? (I don\u0027t know if this is possible).\n\nAt first glance, it seems to me that both backends cannot be activated at the same time, loading one backend seems exclusive for other backends.\n\nBut if that\u0027s not the case, how to differentiate the results if `enable_by_files` and `disable_by_file` are enabled in the same process? I mean, from the client http request point of view.","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"aa8d190303776fc740bd79d407b6cc56fbfb36fe","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"f79fcbe9_ecba10ae","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"in_reply_to":"6f21d408_beed853d","updated":"2024-07-01 14:58:35.000000000","message":"Sorry, I didn\u0027t understood what exactly you want me to mention here.\n\nDo you want me to refer any existing API?\n\nThis is where we want to use it,\n\nhttps://review.opendev.org/c/openstack/glance-specs/+/917284/6/specs/2024.2/approved/glance/improve-filesystem-driver.rst","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"197a8a128ac9fc3ccfb80b7857b13108c9d59cdd","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"4e335afd_1751486d","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"in_reply_to":"aa0e6010_57a02eda","updated":"2024-07-01 17:02:25.000000000","message":"I don\u0027t think it is possible (or recommended) to enable two backends of healthcheck plugin at a time.","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"c72744b96a34092ff2bb7ea3a204dd7a39ad1386","unresolved":true,"context_lines":[{"line_number":55,"context_line":"Impact on Existing APIs"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Security impact"},{"line_number":61,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"0ab5639a_8d801165","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":4},"in_reply_to":"f79fcbe9_ecba10ae","updated":"2024-07-01 16:07:26.000000000","message":"Do this change will add new public API endpoints?\nI think that yes.\n\nBy example if I take the `disable_by_file` submodule, this one seems to be a \"backend\" like yours: https://github.com/openstack/oslo.middleware/blob/master/oslo_middleware/healthcheck/disable_by_file.py\n\nSo your module path will surely looks like to something like:\n\n`oslo_middleware.healthcheck.enable_by_files`\n\nand it will surely provide the a class named `EnabledByFileHealthcheck` with a method named `healthcheck` which will be the endpoints called by when this backend will be loaded. Perhaps you also want to implement another class like the `DisableByFilesPortsHealthcheck` which is provided the `disable_by_file` backend, do not know. That\u0027s the kind of question to highlight there.\n\nAlso, adding this new backend would also mean adding a new option to the opt of the healthcheck module like the with the disable_by_file backend:\n\nhttps://github.com/openstack/oslo.middleware/blob/e2ea8dc556a7e8944b923ffd2a0028495bb60b5e/oslo_middleware/healthcheck/opts.py#L43\n\nOne last thing, is that `enable_by_files` and `disable_by_file` both semantics looks close to each other, maybe it could be possible to share common code between the both features, and in this case, your addition would also impact the `disable_by_file` API, I do not know,.\n\nLet me know if you have questions.","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"bebac008cf07a3d3970cc7fb24d476c0663c3b73","unresolved":true,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Target Milestone for completion:"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"* Dalmatian-1"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Work Items"},{"line_number":107,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"206220e3_54408e95","line":104,"range":{"start_line":104,"start_character":11,"end_line":104,"end_character":13},"updated":"2024-07-01 14:53:02.000000000","message":"Milestone\u0027s deadline is this week, so milestone 1 is already behind us.\nWe should notice that we are close from specs freeze ~ 2 weeks.","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"aa8d190303776fc740bd79d407b6cc56fbfb36fe","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Target Milestone for completion:"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"* Dalmatian-1"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Work Items"},{"line_number":107,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"e4739310_3366f845","line":104,"range":{"start_line":104,"start_character":11,"end_line":104,"end_character":13},"in_reply_to":"206220e3_54408e95","updated":"2024-07-01 14:58:35.000000000","message":"Acknowledged","commit_id":"5e36cb98d5de87d9f3a73193abe2d1b1a0e5ac5b"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"ca7a57b2199db253a77b0215cc9b084a0e776740","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"ddf73743_5653c878","line":60,"range":{"start_line":60,"start_character":40,"end_line":60,"end_character":67},"updated":"2024-07-01 19:18:24.000000000","message":"perhaps we should warn if both plugins are found in the configuration, and exit the process with a failure, thoughts?","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"ca7a57b2199db253a77b0215cc9b084a0e776740","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"1bb48f3a_b90115b5","line":60,"range":{"start_line":60,"start_character":12,"end_line":60,"end_character":27},"updated":"2024-07-01 19:18:24.000000000","message":"typo","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"72d3d8ad8578cde7c11afaa941ab3caa4400e914","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"8aefbe04_965dc63a","line":60,"range":{"start_line":60,"start_character":12,"end_line":60,"end_character":27},"in_reply_to":"1bb48f3a_b90115b5","updated":"2024-07-02 08:02:24.000000000","message":"Done","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"1cf5fee4d1d2fc5cd59c6b3b0830bc6e54b4b40c","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"e80124de_d2865f6b","line":60,"range":{"start_line":60,"start_character":40,"end_line":60,"end_character":67},"in_reply_to":"270e7265_28210e8d","updated":"2024-07-02 06:33:18.000000000","message":"Jul 02 06:31:34 akekane-ng glance-api[2722452]: WARNING oslo_middleware.healthcheck [None req-d1bb392e-9e99-4693-b79b-291795d91394 None None] `enable_by_files` and `disable_by_file` healthcheck middleware should not be configured at once.\nJul 02 06:31:34 akekane-ng systemd[1]: devstack@g-api.service: Deactivated successfully.\nJul 02 06:31:34 akekane-ng systemd[1]: devstack@g-api.service: Consumed 1.456s CPU time.","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"},{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"4462ba1da983f5fe36c5f5f525382514783451f4","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"8b461217_091e3ba1","line":60,"range":{"start_line":60,"start_character":40,"end_line":60,"end_character":67},"in_reply_to":"6745eeac_c94a3391","updated":"2024-07-02 06:13:06.000000000","message":"Good question...\nFor now I\u0027d suggest to simply focus on this plugin and to exit in error if both plugins are given in the config.\n\nRunning both plugin at a time could be an other development.","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8d74353097ffc1243a6bd8ea611b6a17253d0cb6","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"270e7265_28210e8d","line":60,"range":{"start_line":60,"start_character":40,"end_line":60,"end_character":67},"in_reply_to":"8b461217_091e3ba1","updated":"2024-07-02 06:21:30.000000000","message":"Ok, this will make my job easier.","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ae01bf3a737693114b8c84de7b927a9caa07e39c","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"6745eeac_c94a3391","line":60,"range":{"start_line":60,"start_character":40,"end_line":60,"end_character":67},"in_reply_to":"ddf73743_5653c878","updated":"2024-07-02 05:32:59.000000000","message":"We should allow both plugins at a time but the path specified for enable and disable should be different, thoughts?","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"09968ede6f335ddeb89889c323fdda8debc8d480","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"This new healthcheck plugin is exactly opposite of existing plugin"},{"line_number":59,"context_line":"`disable_by_file`. So the `disable_by_file` plugin is also configured"},{"line_number":60,"context_line":"along with `enable_ba_files` plugin the operator needs to make sure"},{"line_number":61,"context_line":"that file paths specified in `disable_by_file` plugin using"},{"line_number":62,"context_line":"`disable_by_file_path` should not be included in `enable_by_file_paths`"},{"line_number":63,"context_line":"configuration option of `enable_by_files` plugin."}],"source_content_type":"text/x-rst","patch_set":4,"id":"8547f44d_cf7ec680","line":60,"range":{"start_line":60,"start_character":40,"end_line":60,"end_character":67},"in_reply_to":"e80124de_d2865f6b","updated":"2024-07-02 06:59:31.000000000","message":"https://review.opendev.org/c/openstack/oslo.middleware/+/920055","commit_id":"c3ec559ce040728f94663359474796f11960e4e1"}]}