)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9b61f951af716e15cd7f36990d6ef45e60f4501d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1977d950_6b69d23b","updated":"2026-05-08 12:02:53.000000000","message":"lgtm, It\u0027s consistent with the approach and implementation for openstacksdk usage for nova.\n\nJust a suggestion about missing testing coverage, coming from previous implementation tbh, but i\u0027d say good to improve.","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"058b94011ff91e22550f4a77c5c90e37048bf9c9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e3da218a_fc23391f","updated":"2026-05-15 15:02:39.000000000","message":"-1: updating the vote due to the osc parameter consideration.","commit_id":"8f8c4684c6b761027729a640fbb320d22e30491c"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ca2bca4fb8504d2071ad0464d0850304a5b6caee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"628ed34c_378dd5db","updated":"2026-05-12 10:50:18.000000000","message":"LGTM","commit_id":"8f8c4684c6b761027729a640fbb320d22e30491c"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e883f588a36d9ee81aa98ba117d9907dcac151","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d352bce6_06ce0d94","updated":"2026-05-18 19:37:44.000000000","message":"the code looks good to me. An additional patch was added to move common content to a base class, which is fine since we still have other clients to migrate.","commit_id":"00faaed59da4541ad2a9bce9f3b6be6f089d99d0"}],"releasenotes/notes/keystoneclient-openstacksdk-migration-23fe0611b0052209.yaml":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"058b94011ff91e22550f4a77c5c90e37048bf9c9","unresolved":true,"context_lines":[{"line_number":9,"context_line":"    The keystone_client configuration options are deprecated and will be removed"},{"line_number":10,"context_line":"    in a future release. Operators should migrate to the keystoneauth adapter"},{"line_number":11,"context_line":"    configuration options in the [keystone] configuration group."},{"line_number":12,"context_line":"other:"},{"line_number":13,"context_line":"  - |"},{"line_number":14,"context_line":"    Added support for openstacksdk as an alternative to keystoneclient."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"1f78267c_3efdde32","line":12,"in_reply_to":"9420230f_e46933b7","updated":"2026-05-15 15:02:39.000000000","message":"there are release note updates in other changes in the chain","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"}],"watcher/common/keystone_helper.py":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"6ec7aa765b3c50b1bbfd081a5d8f8f344b8b3043","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class KeystoneHelper:"},{"line_number":29,"context_line":"    def __init__(self, osc\u003dNone, session\u003dNone, context\u003dNone):"},{"line_number":30,"context_line":"        \"\"\"Create and return a helper to call the keystone service"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        :param osc: an OpenStackClients instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"eafff10d_14726a8a","line":29,"in_reply_to":"15d0de75_97cddfea","updated":"2026-05-15 18:24:09.000000000","message":"I originally intended to remove all usage of osc once all the clients were migrated to the sdk and the OpenstackClients class could be removed. I did not remove it in the NovaHelper either https://github.com/openstack/watcher/blob/902e6a10abd1056a7f52994222b3032810cbe5ef/watcher/applier/actions/stop.py#L69. It looks like we won\u0027t completely remove that class since for example gnocchi is not part of the sdk, so I can either bring that change here or create a follow up patch in the chain to remove the osc parameter","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"058b94011ff91e22550f4a77c5c90e37048bf9c9","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class KeystoneHelper:"},{"line_number":29,"context_line":"    def __init__(self, osc\u003dNone, session\u003dNone, context\u003dNone):"},{"line_number":30,"context_line":"        \"\"\"Create and return a helper to call the keystone service"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        :param osc: an OpenStackClients instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"15d0de75_97cddfea","line":29,"in_reply_to":"6e573cd9_53a6896e","updated":"2026-05-15 15:02:39.000000000","message":"\u003e KeystoneHelper.__init__ accepts an `osc` parameter but silently ignores it. The old constructor used `osc` to obtain a keystone client; the new constructor never reads it. Callers passing `osc\u003d...` will get no error but their argument is discarded.\n\u003e \n\u003e **Severity**: HIGH | **Confidence**: 0.9\n\u003e \n\u003e **Risk**: Existing callers that pass osc\u003d will silently lose the ability to share an OpenStackClients instance, potentially creating duplicate sessions or failing to use the intended credentials.\n\u003e \n\u003e **Priority**: Before merge\n\u003e **Why This Matters**: Breaking the osc\u003d parameter silently is a backwards-compatibility hazard. Any code that instantiates KeystoneHelper(osc\u003dsome_client) will appear to work but will create a new SDK connection instead of reusing the provided client\u0027s session.\n\u003e \n\u003e **Recommendation**:\n\u003e Either remove the `osc` parameter from __init__ entirely (and update any callers), or implement a code path that uses the osc session when provided. At minimum, add a LOG.warning if osc is passed and ignored, so deployers can detect the issue.\n\nPlease fix.\nThis is valid, osc is not used anymore, which means that you would also need to update the manager[1]\n\n[1] https://github.com/openstack/watcher/blob/902e6a10abd1056a7f52994222b3032810cbe5ef/watcher/decision_engine/model/collector/manager.py#L39","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"6cf8fdcf915e7ff5587efd149bbc8ffef6978f5c","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class KeystoneHelper:"},{"line_number":29,"context_line":"    def __init__(self, osc\u003dNone, session\u003dNone, context\u003dNone):"},{"line_number":30,"context_line":"        \"\"\"Create and return a helper to call the keystone service"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        :param osc: an OpenStackClients instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"926e9b1c_9f1ca6fc","line":29,"in_reply_to":"b4efecd0_c4c29ba3","updated":"2026-05-18 12:42:44.000000000","message":"yeah, that was wrong I probably should have not replied so late in a Friday afternoon ;). Actually, the reason why we did not delete it in NovaHelper is that we need it there to use the cinder client. I\u0027ll remove it once we migrate cinderclient to the sdk later this cycle. So your right, there is no reason to not remove it in this patch, I\u0027ve done it in patchset 4","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"7442bcff4ff3748666ee35b99ffffa7531837b52","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class KeystoneHelper:"},{"line_number":29,"context_line":"    def __init__(self, osc\u003dNone, session\u003dNone, context\u003dNone):"},{"line_number":30,"context_line":"        \"\"\"Create and return a helper to call the keystone service"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        :param osc: an OpenStackClients instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"b4efecd0_c4c29ba3","line":29,"in_reply_to":"eafff10d_14726a8a","updated":"2026-05-18 11:19:47.000000000","message":"This looks wrong to me, we should not carry an argument that has no usage in the code. From this commit and forward all keystone configuration with osc is invalid ans since this is not a positional argument, should be fine to remove it and fix other places in the code. We should also fix the Nova case.","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8b2002ec639d4795136cb425176caef12ba7853e","unresolved":true,"context_lines":[{"line_number":37,"context_line":"        \"\"\""},{"line_number":38,"context_line":"        self._create_sdk_connection(context\u003dcontext, session\u003dsession)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def _create_sdk_connection(self, session\u003dNone, context\u003dNone):"},{"line_number":41,"context_line":"        \"\"\"Create and return an OpenStackSDK Connection"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        :param session: Optional keystone session to create the openstack"}],"source_content_type":"text/x-python","patch_set":4,"id":"98a8dac7_f2ed613e","line":40,"range":{"start_line":40,"start_character":8,"end_line":40,"end_character":30},"updated":"2026-05-15 14:53:38.000000000","message":"it seems that we are going to repeat this pattern from now, I think that we should consider a base helper class, wdyt?","commit_id":"8f8c4684c6b761027729a640fbb320d22e30491c"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"7469100cc325b5746605bd4bd848c762bc327692","unresolved":true,"context_lines":[{"line_number":37,"context_line":"        \"\"\""},{"line_number":38,"context_line":"        self._create_sdk_connection(context\u003dcontext, session\u003dsession)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def _create_sdk_connection(self, session\u003dNone, context\u003dNone):"},{"line_number":41,"context_line":"        \"\"\"Create and return an OpenStackSDK Connection"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        :param session: Optional keystone session to create the openstack"}],"source_content_type":"text/x-python","patch_set":4,"id":"ce797642_26d045bc","line":40,"range":{"start_line":40,"start_character":8,"end_line":40,"end_character":30},"in_reply_to":"98a8dac7_f2ed613e","updated":"2026-05-18 12:44:27.000000000","message":"I\u0027ve created a follow up patch moving the method to a base class https://review.opendev.org/c/openstack/watcher/+/988964","commit_id":"8f8c4684c6b761027729a640fbb320d22e30491c"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e883f588a36d9ee81aa98ba117d9907dcac151","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        \"\"\""},{"line_number":38,"context_line":"        self._create_sdk_connection(context\u003dcontext, session\u003dsession)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def _create_sdk_connection(self, session\u003dNone, context\u003dNone):"},{"line_number":41,"context_line":"        \"\"\"Create and return an OpenStackSDK Connection"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        :param session: Optional keystone session to create the openstack"}],"source_content_type":"text/x-python","patch_set":4,"id":"d5adcc17_70aeb7de","line":40,"range":{"start_line":40,"start_character":8,"end_line":40,"end_character":30},"in_reply_to":"ce797642_26d045bc","updated":"2026-05-18 19:37:44.000000000","message":"Acknowledged","commit_id":"8f8c4684c6b761027729a640fbb320d22e30491c"}],"watcher/conf/keystone_client.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8b2002ec639d4795136cb425176caef12ba7853e","unresolved":false,"context_lines":[{"line_number":28,"context_line":"        default\u003d\u0027public\u0027,"},{"line_number":29,"context_line":"        choices\u003d[\u0027internal\u0027, \u0027public\u0027, \u0027admin\u0027],"},{"line_number":30,"context_line":"        deprecated_for_removal\u003dTrue,"},{"line_number":31,"context_line":"        deprecated_reason\u003d_("},{"line_number":32,"context_line":"            \u0027To replace the frozen keystoneclient with \u0027"},{"line_number":33,"context_line":"            \u0027the openstacksdk identity proxy, the options \u0027"},{"line_number":34,"context_line":"            \u0027need to be under the [keystone] group.\u0027"},{"line_number":35,"context_line":"        ),"},{"line_number":36,"context_line":"        deprecated_since\u003d\u00272026.2\u0027,"},{"line_number":37,"context_line":"        help\u003d\u0027Type of endpoint to use in keystoneclient.\u0027,"},{"line_number":38,"context_line":"    ),"}],"source_content_type":"text/x-python","patch_set":4,"id":"df33083f_877fe8be","line":35,"range":{"start_line":31,"start_character":0,"end_line":35,"end_character":10},"updated":"2026-05-15 14:53:38.000000000","message":"ack, in the keystone section there is also the deprecated option, which maps valid-interfaces from the old interface option","commit_id":"8f8c4684c6b761027729a640fbb320d22e30491c"}],"watcher/tests/unit/common/test_keystone_helper.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9b61f951af716e15cd7f36990d6ef45e60f4501d","unresolved":true,"context_lines":[{"line_number":32,"context_line":"        self.keystone_svs.return_value \u003d [ks_service.Service(is_enabled\u003dTrue)]"},{"line_number":33,"context_line":"        self.assertTrue("},{"line_number":34,"context_line":"            self.keystone_helper.is_service_enabled_by_type(\u0027block-storage\u0027)"},{"line_number":35,"context_line":"        )"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_is_service_enabled_not_found(self):"},{"line_number":38,"context_line":"        self.keystone_svs.return_value \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"e3bf96de_bab75993","line":35,"updated":"2026-05-08 12:02:53.000000000","message":"I think there should be a test_is_service_enabled_when_disabled(self) to cover the case where there is only a service and it is disabled in addition to the one that is not found.","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8b2002ec639d4795136cb425176caef12ba7853e","unresolved":true,"context_lines":[{"line_number":32,"context_line":"        self.keystone_svs.return_value \u003d [ks_service.Service(is_enabled\u003dTrue)]"},{"line_number":33,"context_line":"        self.assertTrue("},{"line_number":34,"context_line":"            self.keystone_helper.is_service_enabled_by_type(\u0027block-storage\u0027)"},{"line_number":35,"context_line":"        )"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_is_service_enabled_not_found(self):"},{"line_number":38,"context_line":"        self.keystone_svs.return_value \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"c1c4fa20_dcfd0258","line":35,"in_reply_to":"d29af7bc_f160414a","updated":"2026-05-15 14:53:38.000000000","message":"yeah, it is out of the scope and the unit test improvement could even be backported.","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a0a3e389f11b33afb77831249ce6fc4a47ed94f3","unresolved":true,"context_lines":[{"line_number":32,"context_line":"        self.keystone_svs.return_value \u003d [ks_service.Service(is_enabled\u003dTrue)]"},{"line_number":33,"context_line":"        self.assertTrue("},{"line_number":34,"context_line":"            self.keystone_helper.is_service_enabled_by_type(\u0027block-storage\u0027)"},{"line_number":35,"context_line":"        )"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_is_service_enabled_not_found(self):"},{"line_number":38,"context_line":"        self.keystone_svs.return_value \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"d29af7bc_f160414a","line":35,"in_reply_to":"e3bf96de_bab75993","updated":"2026-05-08 12:10:11.000000000","message":"while technically this would be out of the scope, since it would be a very small change I think we can make it here to improve coverage","commit_id":"79cf4b3166b0b7bf39a244961cbc17ebe1638cc6"}]}