)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"e255372bdfc5de1c3f1432a25fe1d89395c19996","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"627e4de6_5b4940b3","updated":"2026-01-16 12:54:11.000000000","message":"I will absolutely be removing the extra change on the radosgw file.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"f323f21ec64694d5ce8ac1aa57ec460bb15dba83","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ca9c7ee8_835fe601","updated":"2026-01-14 14:11:27.000000000","message":"It would be great to add tests for the new methods introduced (get_urls and url_for)","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"e98ce79bf8bcc6f546ce4a3129006c6496a920c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c8a3676f_a8a0f705","in_reply_to":"ca9c7ee8_835fe601","updated":"2026-01-21 18:58:05.000000000","message":"Done","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"9173477a157cc60b364d95bf9e300a68fda72bad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"01beb7b7_94269e59","updated":"2026-02-05 15:00:17.000000000","message":"Pleas ignore my latest commit, I was doing some experiments with Claude Code and it went crazy","commit_id":"d7e4ecbd889de258f0fd9aa05abda6698774e481"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"b3d8283680c686358d88b2315ebd3d53719d62f4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6be647be_5da54e92","updated":"2026-02-09 10:42:25.000000000","message":"The code itself looks good, but I\u0027d like to see 2 more testcases added:\n- The follow-up patch refactors the RGW related code. That code was modified a few months ago, because apparently there can be multiple services of the same type with different names. I believe in the original PS it was mentioned there were services with `service_type\u003dradosgw` and `service_name\u003dradosgw`, `service_name\u003dswift`. So please expand the the catalog_data to have 2 endpoints of the same type and then add a test case (or 2) to show that the correct url gets returned based on the specified service name with `url_for`.\n- As I understand it, the get_urls is supposed to return multiple urls if the parameters match multiple services and I\u0027d like to see a testcase for that. You can use e.g. the service_type from earlier in my comment or `interface\u003dpublic`\n\nAnd while doing changes, I think there was one interesting bit from Juan\u0027s \"Claude Code went crazy\". Let\u0027s replace the \"existant\" with \"existent\". I\u0027m pretty sure people are going around the OpenStack code with all sorts of automatic tools and sooner or later a patch would appear doing just that, so let\u0027s save us the work on reviewing it and do the replacement here.","commit_id":"d7e4ecbd889de258f0fd9aa05abda6698774e481"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"8bd767265af02292497d88737ba6f51f89d8b975","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e295467f_3fe70d04","in_reply_to":"01beb7b7_94269e59","updated":"2026-02-05 15:00:35.000000000","message":"comment*","commit_id":"d7e4ecbd889de258f0fd9aa05abda6698774e481"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"45f780dcf889236494c067624e5d76cedddc52cc","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"40d9804a_8e8381b6","in_reply_to":"6be647be_5da54e92","updated":"2026-02-09 20:24:39.000000000","message":"Here\u0027s the automatic tool for spellcheck https://review.opendev.org/c/openstack/ceilometer/+/976171","commit_id":"d7e4ecbd889de258f0fd9aa05abda6698774e481"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"5499e8acc6dbdfd894b7d3084535b54baa5f8dde","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"048a7877_c39f06e9","updated":"2026-02-17 20:00:52.000000000","message":"recheck","commit_id":"1412f14ae45350a59fb97a5522d9a127dc365ad1"}],"ceilometer/objectstore/rgw.py":[{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"d6cf7941d8bc3c205c25e00e95979ad646196d58","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            service_type \u003d conf.service_types.radosgw"},{"line_number":79,"context_line":"            service_name \u003d conf.rgw_client.rgw_service_name"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"            if service_type is None and service_name is None:"},{"line_number":82,"context_line":"                raise RuntimeError("},{"line_number":83,"context_line":"                    \"Required config not found, need either \""},{"line_number":84,"context_line":"                    \"rgw_client.rgw_service_name or service_types.radosgw\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3e95f5ad_dca47418","line":81,"updated":"2026-01-14 14:09:33.000000000","message":"Here we are checking that both service_type and service_name are None at the same time, while the message for the raised exception says that it needs one OR the other.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"842d73146290c5a4ad7896d581b16898b700df05","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            service_type \u003d conf.service_types.radosgw"},{"line_number":79,"context_line":"            service_name \u003d conf.rgw_client.rgw_service_name"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"            if service_type is None and service_name is None:"},{"line_number":82,"context_line":"                raise RuntimeError("},{"line_number":83,"context_line":"                    \"Required config not found, need either \""},{"line_number":84,"context_line":"                    \"rgw_client.rgw_service_name or service_types.radosgw\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"a9ead15f_5f6bd55c","line":81,"in_reply_to":"3e95f5ad_dca47418","updated":"2026-01-14 20:47:24.000000000","message":"This raises an exception if neither are present.\nThe original method calls in lines 94 and 83 were virtually identical, except for the service_name and service_type params.\nThe get_url function accepts both.\nBy checking up front for the case where neither are set, we fail early, and then can make the get_url function deal with them both being defined.\n\nThis change deals with adding the convenience function to keystoneclient, so that additional optimisation appear to be out of scope. I can split that into a separate change, so that the update is easier to grok and review.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"fe9607480673d172f7cf6851456abe48d38fc5a4","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            service_type \u003d conf.service_types.radosgw"},{"line_number":79,"context_line":"            service_name \u003d conf.rgw_client.rgw_service_name"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"            if service_type is None and service_name is None:"},{"line_number":82,"context_line":"                raise RuntimeError("},{"line_number":83,"context_line":"                    \"Required config not found, need either \""},{"line_number":84,"context_line":"                    \"rgw_client.rgw_service_name or service_types.radosgw\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3877bc7a_5a6cfcef","line":81,"in_reply_to":"a9ead15f_5f6bd55c","updated":"2026-01-16 12:53:28.000000000","message":"FYI, here\u0027s the behaviour of url_for with both the service_name and the service_type.\n\nWhen both the service_name and service_type are passed to url_for, the side_effects are:\n* returns a URL if both refer to the same service\n* raises EndpointNotFound if they refer to different services.\n* returns a URL if one is None and the other is set, and refers to an existing endpoint.\n\nAn alternative safeguard would be to build a dict of kwargs, setting either service_name or service_type, depending on which is enabled. and then pass that into url_for instead","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"f2169ca8f01866b29bdda4fa1d388a8ab655ea9c","unresolved":true,"context_lines":[{"line_number":78,"context_line":"            service_type \u003d conf.service_types.radosgw"},{"line_number":79,"context_line":"            service_name \u003d conf.rgw_client.rgw_service_name"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"            if service_type is None and service_name is None:"},{"line_number":82,"context_line":"                raise RuntimeError("},{"line_number":83,"context_line":"                    \"Required config not found, need either \""},{"line_number":84,"context_line":"                    \"rgw_client.rgw_service_name or service_types.radosgw\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"216c784e_485c77cd","line":81,"in_reply_to":"a9ead15f_5f6bd55c","updated":"2026-01-15 16:07:15.000000000","message":"Well ,this might be worth as testing radosGW metrics retrieval does require more than a trivial setup, so we might be able to go through first with adding the function and then we can go with the function usage in subsequent patches.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"d6cf7941d8bc3c205c25e00e95979ad646196d58","unresolved":true,"context_lines":[{"line_number":85,"context_line":"            try:"},{"line_number":86,"context_line":"                # Use the service_name to target the endpoint."},{"line_number":87,"context_line":"                # There are cases where both \u0027radosgw\u0027 and \u0027swift\u0027 are used."},{"line_number":88,"context_line":"                rgw_url \u003d keystone_client.url_for("},{"line_number":89,"context_line":"                    ksclient,"},{"line_number":90,"context_line":"                    service_type\u003dservice_type, service_name\u003dservice_name,"},{"line_number":91,"context_line":"                    interface\u003dconf.service_credentials.interface,"}],"source_content_type":"text/x-python","patch_set":2,"id":"85a8a0eb_f05107cb","line":88,"updated":"2026-01-14 14:09:33.000000000","message":"This is a huge change of behaviour. The original code first tried service_name, if that did not existed, it used service_type as a fallback. Now it passes both at the same time, which can return a different result that the one returned previously. Is this intended?","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"842d73146290c5a4ad7896d581b16898b700df05","unresolved":true,"context_lines":[{"line_number":85,"context_line":"            try:"},{"line_number":86,"context_line":"                # Use the service_name to target the endpoint."},{"line_number":87,"context_line":"                # There are cases where both \u0027radosgw\u0027 and \u0027swift\u0027 are used."},{"line_number":88,"context_line":"                rgw_url \u003d keystone_client.url_for("},{"line_number":89,"context_line":"                    ksclient,"},{"line_number":90,"context_line":"                    service_type\u003dservice_type, service_name\u003dservice_name,"},{"line_number":91,"context_line":"                    interface\u003dconf.service_credentials.interface,"}],"source_content_type":"text/x-python","patch_set":2,"id":"383d3238_77101859","line":88,"in_reply_to":"85a8a0eb_f05107cb","updated":"2026-01-14 20:47:24.000000000","message":"IINM, the parameters are mutually exclusive and get_service_catalog.url_for will complain. Per the last comment, I can split this out and we can review that change separately to the adding of the convenience function.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"d6cf7941d8bc3c205c25e00e95979ad646196d58","unresolved":true,"context_lines":[{"line_number":103,"context_line":"        return iter(cache[self.CACHE_KEY_METHOD])"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def _get_account_info(self, ksclient, tenants):"},{"line_number":106,"context_line":"        endpoint \u003d self._get_endpoint(self.conf, ksclient)"},{"line_number":107,"context_line":"        if not endpoint:"},{"line_number":108,"context_line":"            return"},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"a4cc1651_e490f34f","line":106,"updated":"2026-01-14 14:09:33.000000000","message":"Now this method can raise an exception we are doing nothing about here.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"}],"ceilometer/polling/manager.py":[{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"d6cf7941d8bc3c205c25e00e95979ad646196d58","unresolved":true,"context_lines":[{"line_number":947,"context_line":"                        service_type \u003d getattr("},{"line_number":948,"context_line":"                            self.conf.service_types,"},{"line_number":949,"context_line":"                            discoverer.KEYSTONE_REQUIRED_FOR_SERVICE)"},{"line_number":950,"context_line":"                        if not keystone_client.url_for("},{"line_number":951,"context_line":"                                self.keystone, service_type\u003dservice_type):"},{"line_number":952,"context_line":"                            LOG.warning("},{"line_number":953,"context_line":"                                \u0027Skipping %(name)s, %(service_type)s service \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"422b4a61_8460e42b","line":950,"updated":"2026-01-14 14:09:33.000000000","message":"Here you are replacing get_endpoints with url_for, but url_for does things different from the existing get_endpoints.\n\nget_endpoints() returns a dict of all endpoints while url_for() returns a single URL and may raise EndpointNotFound. If the exception is raised intead of just returning false, this check will break.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"fe9607480673d172f7cf6851456abe48d38fc5a4","unresolved":true,"context_lines":[{"line_number":947,"context_line":"                        service_type \u003d getattr("},{"line_number":948,"context_line":"                            self.conf.service_types,"},{"line_number":949,"context_line":"                            discoverer.KEYSTONE_REQUIRED_FOR_SERVICE)"},{"line_number":950,"context_line":"                        if not keystone_client.url_for("},{"line_number":951,"context_line":"                                self.keystone, service_type\u003dservice_type):"},{"line_number":952,"context_line":"                            LOG.warning("},{"line_number":953,"context_line":"                                \u0027Skipping %(name)s, %(service_type)s service \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"6f2d09b8_645a0d84","line":950,"in_reply_to":"1c3f01bc_20deb1f5","updated":"2026-01-16 12:53:28.000000000","message":"When both the service_name and service_type are passed to url_for, the side_effects are:\n* returns a URL if both refer to the same service\n* raises EndpointNotFound if they refer to different services.\n* returns a URL if one is None and the other is set, and refers to an existing endpoint.\n\nAn alternative safeguard would be to build a dict of kwargs, setting either service_name or service_type and","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"a753350e856a3c6706ca97c92a05b849aaf8c4bf","unresolved":true,"context_lines":[{"line_number":947,"context_line":"                        service_type \u003d getattr("},{"line_number":948,"context_line":"                            self.conf.service_types,"},{"line_number":949,"context_line":"                            discoverer.KEYSTONE_REQUIRED_FOR_SERVICE)"},{"line_number":950,"context_line":"                        if not keystone_client.url_for("},{"line_number":951,"context_line":"                                self.keystone, service_type\u003dservice_type):"},{"line_number":952,"context_line":"                            LOG.warning("},{"line_number":953,"context_line":"                                \u0027Skipping %(name)s, %(service_type)s service \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"1c3f01bc_20deb1f5","line":950,"in_reply_to":"422b4a61_8460e42b","updated":"2026-01-15 13:28:08.000000000","message":"I didn\u0027t consider the difference in side effect.\n\nI considered this replacement since it\u0027s checking for ANY endpoint to be found, so using url_for would fit that.\n\nAn appropriate update here could be to replace the if block with a try except and log the warning.\n OR update the get_urls helper function to return None with there are no URLs instead of propagating the error. This would preserve the existing functionality IINM.","commit_id":"4c2607847c8f0ecd07707e5ba86a07ff53318a9c"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"be340aa57118fdf473b9c5d79f70b45d5f2ca67f","unresolved":true,"context_lines":[{"line_number":947,"context_line":"                        service_type \u003d getattr("},{"line_number":948,"context_line":"                            self.conf.service_types,"},{"line_number":949,"context_line":"                            discoverer.KEYSTONE_REQUIRED_FOR_SERVICE)"},{"line_number":950,"context_line":"                        if not keystone_client.url_for("},{"line_number":951,"context_line":"                                self.keystone, service_type\u003dservice_type):"},{"line_number":952,"context_line":"                            LOG.warning("},{"line_number":953,"context_line":"                                \u0027Skipping %(name)s, %(service_type)s service \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"8501139f_69fd3e62","line":950,"updated":"2026-02-05 14:57:27.000000000","message":"**Potential behavioral change / regression**\n\nThe original code used `get_endpoints()` which returns a dict/list:\n```python\nif not keystone_client.get_service_catalog(self.keystone).get_endpoints(service_type\u003dservice_type):\n```\n\nThe new code uses `url_for()`:\n```python\nif not keystone_client.url_for(self.keystone, service_type\u003dservice_type):\n```\n\nThe difference:\n- `get_endpoints()` returns empty dict → falsy → gracefully skips pollster\n- `url_for()` raises `EndpointNotFound` if no endpoint exists → uncaught exception\n\n**Suggestion:** Use `get_urls()` instead, which returns an empty tuple when no endpoints are found (doesn\u0027t raise):\n\n```python\nif not keystone_client.get_urls(self.keystone, service_type\u003dservice_type):\n```\n\nOr wrap in try/except for `EndpointNotFound`.","commit_id":"d7e4ecbd889de258f0fd9aa05abda6698774e481"},{"author":{"_account_id":32968,"name":"Juan Larriba","email":"jlarriba@redhat.com","username":"jlarriba"},"change_message_id":"9173477a157cc60b364d95bf9e300a68fda72bad","unresolved":false,"context_lines":[{"line_number":947,"context_line":"                        service_type \u003d getattr("},{"line_number":948,"context_line":"                            self.conf.service_types,"},{"line_number":949,"context_line":"                            discoverer.KEYSTONE_REQUIRED_FOR_SERVICE)"},{"line_number":950,"context_line":"                        if not keystone_client.url_for("},{"line_number":951,"context_line":"                                self.keystone, service_type\u003dservice_type):"},{"line_number":952,"context_line":"                            LOG.warning("},{"line_number":953,"context_line":"                                \u0027Skipping %(name)s, %(service_type)s service \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"f70405d7_6604f4f7","line":950,"in_reply_to":"8501139f_69fd3e62","updated":"2026-02-05 15:00:17.000000000","message":"Done","commit_id":"d7e4ecbd889de258f0fd9aa05abda6698774e481"}]}