)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"2b81ce0409516d14117cd43f9288c1317741a517","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"565f68d9_a5a1f42c","updated":"2025-06-17 10:34:15.000000000","message":"For some reason I cannot access the template, not sure what I am missing","commit_id":"94c17f79993a3b2898b11ce95235386cbf024c92"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"424d48fdf5d90c57807af3d99e31c1dd702890ce","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c31b5a42_754cfaaf","in_reply_to":"50c57c60_dad0f859","updated":"2025-06-17 15:14:42.000000000","message":"thanks... I tried using the top level and it worked!\n\nI\u0027m curious though if this is the correct approach.\n\n@openstack@dopieralski.pl maybe you can give us some feedback here?","commit_id":"94c17f79993a3b2898b11ce95235386cbf024c92"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8933542385cba1e122256ad66cc9f1d3adfc3dbd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"50c57c60_dad0f859","in_reply_to":"565f68d9_a5a1f42c","updated":"2025-06-17 11:37:32.000000000","message":"so i have never got this to work with the templeate in the module that uses them.\n\nim sure there is a trick to that that we are both missing but that is why i put them at the top level.","commit_id":"94c17f79993a3b2898b11ce95235386cbf024c92"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"125595ffa9a3626c02f84d24d3aabf32ecbe1686","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"11c71807_70a11cb5","in_reply_to":"c31b5a42_754cfaaf","updated":"2025-06-17 16:10:18.000000000","message":"i know there are at least some exampels fo the top level directory in teh existing plugins. what i really want to knwo is what make manilla-ui special since it appreantly works at teh content level there.\n\nthere is soemthing we are obviously missing with how the template paths are registered.","commit_id":"94c17f79993a3b2898b11ce95235386cbf024c92"}],"src/grian_ui/datasource/__init__.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6af90f7ee537a81d99568440daafdce7fd49526d","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# SPDX-License-Identifier: Apache-2.0"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"from grian_ui.datasource import fake"},{"line_number":5,"context_line":"from grian_ui.datasource import prometheus"},{"line_number":6,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"444ef0ed_db872996","line":3,"updated":"2025-07-07 11:22:37.000000000","message":"can we avoid using module expores and jsut kepp the __init__.py files empty","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"b886dc7e8ca29ee4a8d60888641a5d13c5d30710","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# SPDX-License-Identifier: Apache-2.0"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"from grian_ui.datasource import fake"},{"line_number":5,"context_line":"from grian_ui.datasource import prometheus"},{"line_number":6,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1ad2d92c_f1847e06","line":3,"in_reply_to":"444ef0ed_db872996","updated":"2025-07-07 16:03:17.000000000","message":"Acknowledged","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"}],"src/grian_ui/datasource/api.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6af90f7ee537a81d99568440daafdce7fd49526d","unresolved":true,"context_lines":[{"line_number":10,"context_line":"from django.utils.module_loading import import_string"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"def get_datasource():"},{"line_number":14,"context_line":"    \"\"\""},{"line_number":15,"context_line":"    Get the configured datasource backend."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"4bb6df48_c7da30c1","line":13,"range":{"start_line":13,"start_character":4,"end_line":13,"end_character":18},"updated":"2025-07-07 11:22:37.000000000","message":"so the intent is that each data store woudl implement each of the api methods below\n\n\nso we either need to define them as a abstract base class or  a python protocl.\n\nthen the api modle is then a dynmaic instance of that interface that delegates to the relevent datastore.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"b886dc7e8ca29ee4a8d60888641a5d13c5d30710","unresolved":false,"context_lines":[{"line_number":10,"context_line":"from django.utils.module_loading import import_string"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"def get_datasource():"},{"line_number":14,"context_line":"    \"\"\""},{"line_number":15,"context_line":"    Get the configured datasource backend."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"c6605213_6213fee7","line":13,"range":{"start_line":13,"start_character":4,"end_line":13,"end_character":18},"in_reply_to":"4bb6df48_c7da30c1","updated":"2025-07-07 16:03:17.000000000","message":"Acknowledged","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669a835b1ccc851c8429a8cf3c622bbaace871f5","unresolved":true,"context_lines":[{"line_number":17,"context_line":"    Returns the datasource implementation based on GRIAN_PLUGIN[\u0027datasource\u0027]"},{"line_number":18,"context_line":"    setting. Defaults to \u0027fake\u0027 if not configured."},{"line_number":19,"context_line":"    \"\"\""},{"line_number":20,"context_line":"    plugin_config \u003d getattr(settings, \"GRIAN_PLUGIN\", {})"},{"line_number":21,"context_line":"    datasource_type \u003d plugin_config.get(\"datasource\", \"fake\")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    module_path \u003d f\"grian_ui.datasource.{datasource_type}\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    try:"},{"line_number":26,"context_line":"        datasource_module \u003d import_string(module_path)"}],"source_content_type":"text/x-python","patch_set":10,"id":"e41597c9_8c29393a","line":23,"range":{"start_line":20,"start_character":0,"end_line":23,"end_character":58},"updated":"2025-07-15 19:43:41.000000000","message":"instead of directly using Django settings, we really need to create a dedicated grain_ui.config module. This will centralize our configuration validation, so we don\u0027t have to repeat it everywhere a setting is used. Also, let\u0027s try to avoid getattr in general, as it can make things less explicit.\n\nMany Horizon plugins don\u0027t properly validate their config, often treating everything as a string. However, if a data source is an enum (of string values), we must validate that type before using it, especially when constructing module paths.\n\nMy strong suggestion is to create that grain_ui.config module.\n\nTake manila-ui (https://opendev.org/openstack/manila-ui/src/branch/master/manila_ui/features.py#L31-L50) – it\u0027s close, but it\u0027s not properly validating types. For example, manila_config.get(\u0027enable_share_groups\u0027, True) will incorrectly treat a truthy value like 1 as a boolean if it\u0027s not explicitly cast, and we need to be more robust than that.\n\nIdeally, we\u0027d use oslo.config (https://docs.openstack.org/horizon/latest/contributor/topics/ini-based-configuration.html), and the support for it is still there (https://github.com/openstack/horizon/blob/master/openstack_dashboard/utils/config_types.py). But I haven\u0027t seen it widely used in plugins, and Horizon core hasn\u0027t adopted oslo.config classes yet.\n\nAnother obvious approach would be https://github.com/jazzband/django-configurations to define our config. However, since our config is a Python file edited by the deployer, we can\u0027t actually keep the validation alongside the config option defaults in grain_ui/local/local_settings.d/_90_grain_ui.py.\n\nUsing oslo.config or django-configurations at this time is probably overkill. However, creating a dedicated module is the absolute minimum we should do here. That\u0027s why grain_ui/config.py is the most sensible path forward; it will centralize all our configuration logic, which is crucial for maintainability and reliability, without over-complicating things.\n\nif the validation fails we can rase a config error like horizon does\n\nhttps://github.com/openstack/horizon/blob/master/openstack_dashboard/api/base.py#L101-L119\n\nideally this woudl be a hard error that prevents horizon form starting but that may or may not be possibel depending on where/when we do the validation.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d7ffe33c7a9ccad4165baee283dd918aabb8c42a","unresolved":true,"context_lines":[{"line_number":17,"context_line":"    Returns the datasource implementation based on GRIAN_PLUGIN[\u0027datasource\u0027]"},{"line_number":18,"context_line":"    setting. Defaults to \u0027fake\u0027 if not configured."},{"line_number":19,"context_line":"    \"\"\""},{"line_number":20,"context_line":"    plugin_config \u003d getattr(settings, \"GRIAN_PLUGIN\", {})"},{"line_number":21,"context_line":"    datasource_type \u003d plugin_config.get(\"datasource\", \"fake\")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    module_path \u003d f\"grian_ui.datasource.{datasource_type}\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    try:"},{"line_number":26,"context_line":"        datasource_module \u003d import_string(module_path)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5d6c0d89_02978e6f","line":23,"range":{"start_line":20,"start_character":0,"end_line":23,"end_character":58},"in_reply_to":"ab70096e_39f16a80","updated":"2025-07-18 11:38:50.000000000","message":"grian_ui/config.py\nwould just have functions like https://opendev.org/openstack/manila-ui/src/branch/master/manila_ui/features.py#L31-L50\n\ncache parse and validate the config options provided in https://opendev.org/openstack/grian-ui/src/branch/master/src/grian_ui/local/local_settings.d/_90_grian_ui.py\n\nso i would update this code to something like \n\nat the module level do this\n```\nfrom grian_ui import config\n\nif config.get_datasource() \u003d\u003d config.FAKE_DATASORUCE:\n  from grian_ui.datasouce import fake as ds\nelse\n  from grian_ui.datasouce import promethous as ds\n```\nthen you can rename getDatasouce to get client or just call ds.get_client below and remove this method entirely.\n\n\nin grian_ui/config.py you would do something like this\n\n```\n\nfrom django.conf import settings\n\nfrom hoizon import exceptions\nfrom horizon.utils import memoized\n\n\n# this is the only use of getattr we shoudl have.\nPLUGIN_CONFIG \u003d getattr(settings, \u0027GRIAN_PLUGIN\u0027, {})\n\nFAKE_DATASORUCE \u003d \u0027fake\u0027\nPROMETHEUS_DATASOURCE \u003d \u0027prometheus\u0027\n\nVALID_DATASOURCES \u003d {FAKE_DATASORUCE,PROMETHEUS_DATASOURCE}\n\n@memoized.memoized\ndef get_datasource():\n    datasource \u003d PLUGIN_CONFIG.get(\"datasource\", PROMETHEUS_DATASOURCE)\n    if datasource not in VALID_DATASOURCES and not isinstance(str, datasource):\n        msg \u003d f\"{datasource} is not a valid datasource: {VALID_DATASOURCES}\"\n        raise exceptions.ConfigurationError(msg)\n    return datasource\n\n```\n\nthis way all the config defaulting and validation is in one place.\n_90_grian_ui.py can continue to have our default set but if we anted to go one step future we could replace the hardcode default in _90_grian_ui.py  with an import of \n\ngrian_ui/config.py\n\nand add DEFAULT_DATASOUCE \u003d PROMETHEUS_DATASOURCE\nand use DEFAULT_DATASOUCE in _90_grian_ui.py\n\nand do this instead\n\ngrian_ui/config.py\n```\nmemoized.memoized\ndef get_datasource():\n    datasource \u003d PLUGIN_CONFIG.get(\"datasource\", DEFAULT_DATASOUCE)\n    if datasource not in VALID_DATASOURCES and not isinstance(str, datasource):\n        msg \u003d f\"{datasource} is not a valid datasource: {VALID_DATASOURCES}\"\n        raise exceptions.ConfigurationError(msg)\n    return datasource\n\n```\n\n_90_grian_ui.py\n\n```\n#\n# SPDX-License-Identifier: Apache-2.0\nfrom grian_ui import config\nGRIAN_PLUGIN \u003d {\"datasource\": config.DEFAULT_DATASOUCE}\n\n```","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"be18f9b0dca89d554e2e34457ffb900f3796b213","unresolved":true,"context_lines":[{"line_number":17,"context_line":"    Returns the datasource implementation based on GRIAN_PLUGIN[\u0027datasource\u0027]"},{"line_number":18,"context_line":"    setting. Defaults to \u0027fake\u0027 if not configured."},{"line_number":19,"context_line":"    \"\"\""},{"line_number":20,"context_line":"    plugin_config \u003d getattr(settings, \"GRIAN_PLUGIN\", {})"},{"line_number":21,"context_line":"    datasource_type \u003d plugin_config.get(\"datasource\", \"fake\")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    module_path \u003d f\"grian_ui.datasource.{datasource_type}\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    try:"},{"line_number":26,"context_line":"        datasource_module \u003d import_string(module_path)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ab70096e_39f16a80","line":23,"range":{"start_line":20,"start_character":0,"end_line":23,"end_character":58},"in_reply_to":"e41597c9_8c29393a","updated":"2025-07-18 08:54:55.000000000","message":"I\u0027m not sure what the request here is. For how Horizon plugins work, we need to have a file for configuration of the plugin (in our case, https://opendev.org/openstack/grian-ui/src/branch/master/src/grian_ui/local/local_settings.d/_90_grian_ui.py) and then move that to Horizon local_settings.d so Horizon pick up the config for the plugin when enabled. I think is a good idea to align with this, as this is the expected way Horizon plugin works.\n\nI do take the feedback that maybe there is a better way to handle things though.\n\nHow do you see grian_ui/config.py working? What would be the format we should follow? How do we do type checking in that case? Is there a plugin or a project implementing this already?","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669a835b1ccc851c8429a8cf3c622bbaace871f5","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        datasource_module \u003d import_string(module_path)"},{"line_number":27,"context_line":"        return datasource_module.get_client()"},{"line_number":28,"context_line":"    except ImportError:"},{"line_number":29,"context_line":"        # Fallback to fake datasource"},{"line_number":30,"context_line":"        from grian_ui.datasource.fake import get_client"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        return get_client()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"def get_telemetry_overview():"}],"source_content_type":"text/x-python","patch_set":10,"id":"b4496e3c_604bc516","line":32,"range":{"start_line":29,"start_character":0,"end_line":32,"end_character":27},"updated":"2025-07-15 19:43:41.000000000","message":"this is incorrect for two reasons\n\nfirst all import including fallback import shoudl be at the module level.\n\nsecond openstack style rule say we can only import modules not functions, classes or constants.\n\nhttps://github.com/openstack/hacking/blob/master/HACKING.rst#imports\n\neven if we are using ruff for our general linting we shoudl still follow the hacking rules in general where there is not a conflict.\n\nin general we are not currently planning to support external plugins ot this plugin.\n\nif we were we woudl use https://opendev.org/openstack/stevedore\nto mange the loading like we do for other plugpoint in openstack.\n\nas such this shoudl never fail.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d7ffe33c7a9ccad4165baee283dd918aabb8c42a","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        datasource_module \u003d import_string(module_path)"},{"line_number":27,"context_line":"        return datasource_module.get_client()"},{"line_number":28,"context_line":"    except ImportError:"},{"line_number":29,"context_line":"        # Fallback to fake datasource"},{"line_number":30,"context_line":"        from grian_ui.datasource.fake import get_client"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        return get_client()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"def get_telemetry_overview():"}],"source_content_type":"text/x-python","patch_set":10,"id":"86724284_3c05e54f","line":32,"range":{"start_line":29,"start_character":0,"end_line":32,"end_character":27},"in_reply_to":"86bf62d2_db09c3e2","updated":"2025-07-18 11:38:50.000000000","message":"ack ya i left comment on that because i was not happy with the code that was generated and planned to rewrite it.\n\ni have more or less explained how i would do that above so ill resovle this for now.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"be18f9b0dca89d554e2e34457ffb900f3796b213","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        datasource_module \u003d import_string(module_path)"},{"line_number":27,"context_line":"        return datasource_module.get_client()"},{"line_number":28,"context_line":"    except ImportError:"},{"line_number":29,"context_line":"        # Fallback to fake datasource"},{"line_number":30,"context_line":"        from grian_ui.datasource.fake import get_client"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        return get_client()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"def get_telemetry_overview():"}],"source_content_type":"text/x-python","patch_set":10,"id":"86bf62d2_db09c3e2","line":32,"range":{"start_line":29,"start_character":0,"end_line":32,"end_character":27},"in_reply_to":"b4496e3c_604bc516","updated":"2025-07-18 08:54:55.000000000","message":"I took https://review.opendev.org/c/openstack/grian-ui/+/952608 as the example. As a started point, it seemed convenient. But I can definitely look for a different way to handle this.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"}],"src/grian_ui/datasource/fake/__init__.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6af90f7ee537a81d99568440daafdce7fd49526d","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    def get_overview(self):"},{"line_number":17,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":18,"context_line":"        return {"},{"line_number":19,"context_line":"            \"total_metrics\": 1247,"},{"line_number":20,"context_line":"            \"active_instances\": 18,"},{"line_number":21,"context_line":"            \"alerts_count\": 3,"},{"line_number":22,"context_line":"            \"cpu_usage_avg\": round(random.uniform(15.0, 85.0), 1),  # nosec"},{"line_number":23,"context_line":"            \"memory_usage_avg\": round(random.uniform(25.0, 75.0), 1),  # nosec"},{"line_number":24,"context_line":"            \"network_throughput\": round(random.uniform(50.0, 200.0), 1),  # nosec"},{"line_number":25,"context_line":"            \"disk_io_rate\": round(random.uniform(10.0, 100.0), 1),  # nosec"},{"line_number":26,"context_line":"            \"uptime_percentage\": round(random.uniform(98.5, 99.9), 2),  # nosec"},{"line_number":27,"context_line":"            \"last_updated\": datetime.now().strftime(\"%Y-%m-%d %H:%M:%S\"),"},{"line_number":28,"context_line":"        }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_client():"}],"source_content_type":"text/x-python","patch_set":10,"id":"95a6fde7_ca39a752","line":28,"range":{"start_line":18,"start_character":4,"end_line":28,"end_character":9},"updated":"2025-07-07 11:22:37.000000000","message":"is this what promethious returns by default with no actuall query\n\ni.e some metrics about presumably the prometheos server host?","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"4bb23b9a8903db9ee6cd0b266f135ab5dfda9c76","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    def get_overview(self):"},{"line_number":17,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":18,"context_line":"        return {"},{"line_number":19,"context_line":"            \"total_metrics\": 1247,"},{"line_number":20,"context_line":"            \"active_instances\": 18,"},{"line_number":21,"context_line":"            \"alerts_count\": 3,"},{"line_number":22,"context_line":"            \"cpu_usage_avg\": round(random.uniform(15.0, 85.0), 1),  # nosec"},{"line_number":23,"context_line":"            \"memory_usage_avg\": round(random.uniform(25.0, 75.0), 1),  # nosec"},{"line_number":24,"context_line":"            \"network_throughput\": round(random.uniform(50.0, 200.0), 1),  # nosec"},{"line_number":25,"context_line":"            \"disk_io_rate\": round(random.uniform(10.0, 100.0), 1),  # nosec"},{"line_number":26,"context_line":"            \"uptime_percentage\": round(random.uniform(98.5, 99.9), 2),  # nosec"},{"line_number":27,"context_line":"            \"last_updated\": datetime.now().strftime(\"%Y-%m-%d %H:%M:%S\"),"},{"line_number":28,"context_line":"        }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_client():"}],"source_content_type":"text/x-python","patch_set":10,"id":"ed635270_64e7f030","line":28,"range":{"start_line":18,"start_character":4,"end_line":28,"end_character":9},"in_reply_to":"7827515e_98780b4a","updated":"2025-07-08 09:47:42.000000000","message":"(I noticed I didn\u0027t finish replying this) With \"openstack metric list\" we get all the meters available that Ceilometer have scraped. There are some meters that doesn\u0027t produce anything until they are triggered somehow, e.g. the ceilometer_volume_size is only created when a volume is created, otherwise there is no entry for this meter.\n\nAnd with no query there is no default, \"openstack metric query\" expects a Prometheus query as a param, otherwise it fails. Unfortunately I cannot think on a simple query that will return an overview like this. If we want to return something on this fashion we probably need to perform a sequence of queries and parse the info before returning.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"b886dc7e8ca29ee4a8d60888641a5d13c5d30710","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    def get_overview(self):"},{"line_number":17,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":18,"context_line":"        return {"},{"line_number":19,"context_line":"            \"total_metrics\": 1247,"},{"line_number":20,"context_line":"            \"active_instances\": 18,"},{"line_number":21,"context_line":"            \"alerts_count\": 3,"},{"line_number":22,"context_line":"            \"cpu_usage_avg\": round(random.uniform(15.0, 85.0), 1),  # nosec"},{"line_number":23,"context_line":"            \"memory_usage_avg\": round(random.uniform(25.0, 75.0), 1),  # nosec"},{"line_number":24,"context_line":"            \"network_throughput\": round(random.uniform(50.0, 200.0), 1),  # nosec"},{"line_number":25,"context_line":"            \"disk_io_rate\": round(random.uniform(10.0, 100.0), 1),  # nosec"},{"line_number":26,"context_line":"            \"uptime_percentage\": round(random.uniform(98.5, 99.9), 2),  # nosec"},{"line_number":27,"context_line":"            \"last_updated\": datetime.now().strftime(\"%Y-%m-%d %H:%M:%S\"),"},{"line_number":28,"context_line":"        }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_client():"}],"source_content_type":"text/x-python","patch_set":10,"id":"7827515e_98780b4a","line":28,"range":{"start_line":18,"start_character":4,"end_line":28,"end_character":9},"in_reply_to":"95a6fde7_ca39a752","updated":"2025-07-07 16:03:17.000000000","message":"Basically we get all the metrics available that Ceilometer","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669a835b1ccc851c8429a8cf3c622bbaace871f5","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    def get_overview(self):"},{"line_number":17,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":18,"context_line":"        return {"},{"line_number":19,"context_line":"            \"total_metrics\": 1247,"},{"line_number":20,"context_line":"            \"active_instances\": 18,"},{"line_number":21,"context_line":"            \"alerts_count\": 3,"},{"line_number":22,"context_line":"            \"cpu_usage_avg\": round(random.uniform(15.0, 85.0), 1),  # nosec"},{"line_number":23,"context_line":"            \"memory_usage_avg\": round(random.uniform(25.0, 75.0), 1),  # nosec"},{"line_number":24,"context_line":"            \"network_throughput\": round(random.uniform(50.0, 200.0), 1),  # nosec"},{"line_number":25,"context_line":"            \"disk_io_rate\": round(random.uniform(10.0, 100.0), 1),  # nosec"},{"line_number":26,"context_line":"            \"uptime_percentage\": round(random.uniform(98.5, 99.9), 2),  # nosec"},{"line_number":27,"context_line":"            \"last_updated\": datetime.now().strftime(\"%Y-%m-%d %H:%M:%S\"),"},{"line_number":28,"context_line":"        }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_client():"}],"source_content_type":"text/x-python","patch_set":10,"id":"eff56d72_d899071f","line":28,"range":{"start_line":18,"start_character":4,"end_line":28,"end_character":9},"in_reply_to":"ed635270_64e7f030","updated":"2025-07-15 19:43:41.000000000","message":"ack so this is likely not a represeitnive example of a fucniton we woudl acutlly have then.\n\nwe woudl not need or want to have a single dashbaord that dispaly every metric form ever host across and entire cloud.\n\nand yes i think we will need to compose overview by using multiple queis.\ni.e. a hypervior metrics view woudl likely have 3 quiers oen for cpu ram and disk as a simle example.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6af90f7ee537a81d99568440daafdce7fd49526d","unresolved":true,"context_lines":[{"line_number":26,"context_line":"            \"uptime_percentage\": round(random.uniform(98.5, 99.9), 2),  # nosec"},{"line_number":27,"context_line":"            \"last_updated\": datetime.now().strftime(\"%Y-%m-%d %H:%M:%S\"),"},{"line_number":28,"context_line":"        }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_client():"},{"line_number":32,"context_line":"    \"\"\"Get a fake telemetry client instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f5c6d3a_24d53eba","line":29,"updated":"2025-07-07 11:22:37.000000000","message":"can you add some fake data for the other api methods","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"16f467c85b23fe4dbcc26f0b9cae640233448d95","unresolved":false,"context_lines":[{"line_number":26,"context_line":"            \"uptime_percentage\": round(random.uniform(98.5, 99.9), 2),  # nosec"},{"line_number":27,"context_line":"            \"last_updated\": datetime.now().strftime(\"%Y-%m-%d %H:%M:%S\"),"},{"line_number":28,"context_line":"        }"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_client():"},{"line_number":32,"context_line":"    \"\"\"Get a fake telemetry client instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"b71bc27c_b366efdd","line":29,"in_reply_to":"3f5c6d3a_24d53eba","updated":"2025-07-07 16:03:39.000000000","message":"Acknowledged","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"}],"src/grian_ui/datasource/prometheus/__init__.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6af90f7ee537a81d99568440daafdce7fd49526d","unresolved":true,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    def get_overview(self):"},{"line_number":19,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":20,"context_line":"        return self.client.query.list()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"ca596f21_7a8a9151","line":20,"updated":"2025-07-07 11:22:37.000000000","message":"what will this actully return?","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669a835b1ccc851c8429a8cf3c622bbaace871f5","unresolved":true,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    def get_overview(self):"},{"line_number":19,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":20,"context_line":"        return self.client.query.list()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"f3552fde_18421f45","line":20,"in_reply_to":"66da8d9d_6b26534e","updated":"2025-07-15 19:43:41.000000000","message":"given the discussion on the fak backedn im inclined to say we shoudl jsut remove this for now as i dont think we will need that for grian_ui.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"b886dc7e8ca29ee4a8d60888641a5d13c5d30710","unresolved":true,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    def get_overview(self):"},{"line_number":19,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":20,"context_line":"        return self.client.query.list()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"66da8d9d_6b26534e","line":20,"in_reply_to":"ca596f21_7a8a9151","updated":"2025-07-07 16:03:17.000000000","message":"This returns the names of the metrics published by Ceilometer at the moment\n\ne.g.\n\n```\n\u003e\u003e\u003e c.query.list()\n[\u0027ceilometer_image_size\u0027, \u0027ceilometer_snapshot_size\u0027, \u0027ceilometer_volume_provider_capacity_allocated\u0027, \u0027ceilometer_volume_provider_capacity_free\u0027, \u0027ceilometer_volume_provider_capacity_provisioned\u0027, \u0027ceilometer_volume_provider_capacity_total\u0027, \u0027ceilometer_volume_provider_capacity_virtual_free\u0027, \u0027ceilometer_volume_provider_pool_capacity_allocated\u0027, \u0027ceilometer_volume_provider_pool_capacity_free\u0027, \u0027ceilometer_volume_provider_pool_capacity_provisioned\u0027, \u0027ceilometer_volume_provider_pool_capacity_total\u0027, \u0027ceilometer_volume_provider_pool_capacity_virtual_free\u0027, \u0027ceilometer_volume_size\u0027, \u0027ceilometer_volume_snapshot_size\u0027, \u0027scrape_duration_seconds\u0027, \u0027scrape_samples_post_metric_relabeling\u0027, \u0027scrape_samples_scraped\u0027, \u0027scrape_series_added\u0027, \u0027sg_total_ceilometer_metric_decode_count\u0027, \u0027sg_total_ceilometer_metric_decode_error_count\u0027, \u0027sg_total_ceilometer_msg_received_count\u0027, \u0027up\u0027]\n```\n\nWe probably want to do some queries and aggregate data to show a proper overview","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"be18f9b0dca89d554e2e34457ffb900f3796b213","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    def get_overview(self):"},{"line_number":19,"context_line":"        \"\"\"Get telemetry overview data.\"\"\""},{"line_number":20,"context_line":"        return self.client.query.list()"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"d23c910b_c26cc0b9","line":20,"in_reply_to":"f3552fde_18421f45","updated":"2025-07-18 08:54:55.000000000","message":"Acknowledged","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6af90f7ee537a81d99568440daafdce7fd49526d","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"045abdf3_a09e1af2","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"updated":"2025-07-07 11:22:37.000000000","message":"rbac shoudl not really be an attribute fo the query it shoudl really be an attribute of the client.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"b886dc7e8ca29ee4a8d60888641a5d13c5d30710","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"a28604d0_39eeac08","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"045abdf3_a09e1af2","updated":"2025-07-07 16:03:17.000000000","message":"That was my expectation as well. I think we hit a bug in the python-observabilityclient, but basically, this call without \"disable_rbac\" defaults to the current project overriding the config set when the client is instantiated. I need to sync with the team to see why this might be happening.\n\nI just tested it again to make sure, using the client from a python shell\n\n```\n\u003e\u003e\u003e from observabilityclient import client\n\u003e\u003e\u003e c \u003d client.Client(\u00271\u0027, disable_rbac\u003dTrue)\n\n\u003e\u003e\u003e query \u003d c.query.query(\"count(ceilometer_volume_size)\")\n\u003e\u003e\u003e query\n[]\n\n\u003e\u003e\u003e query \u003d c.query.query(\"count(ceilometer_volume_size)\", disable_rbac\u003dTrue)\n\u003e\u003e\u003e query\n[\u003cobservabilityclient.prometheus_client.PrometheusMetric object at 0x7f6bada41870\u003e]\n\u003e\u003e\u003e query[0].__dict__\n{\u0027timestamp\u0027: 1751904070.23, \u0027labels\u0027: {}, \u0027value\u0027: \u00271\u0027}\n```","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"af3585f5264a4016f290d31c6ed6ee24cc59569d","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"f717f4a1_e58fb9d1","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"0904db6c_e6f7b3d8","updated":"2025-07-10 07:12:25.000000000","message":"There is a bit of a history behind how the client (the rbac, the configuration) used to work and how it works now. We\u0027re continuously getting to a bit of a better, more standard state than we used to.\n\nPreviously, we had the rbac feature implemented inside the client, which wasn\u0027t really doing much of what we wanted. This has since been deprecated in the CLI side of the client, but the rbac parameters of the functions stayed to not break things by removing them. If you use some more current version of the observabilityclient (which Victoria is doing now), you can safely just ignore the parameters and not set them. The rbac thing is disabled by default.\n\nRegarding the config, we have options to have a config file or env variables, which tell the client where to look for prometheus. This is used in some places, but a new way to configure this was added, which is using a keystone endpoint. If you have an endpoint of type `prometheus`, then that will get used. If that endpoint is also called `aetos`, then the client will also send tokens with each request (otherwise not).\n\nAs to why use this client instead of the PrometheusAPIClient directly:\n- In one of the comments of the spec, you complained about having to parse the Prometheus / Aetos URL from the keystone endpoint when initializing the PrometheusAPIClient. The proper observabilityclient I want you to use here will do that for you.\n- I guess grian isn\u0027t meant to be an admin only thing like Watcher? In that case you\u0027ll eventually want to scope queries to user\u0027s projects. For that you could use this https://opendev.org/openstack/python-observabilityclient/src/branch/master/observabilityclient/rbac.py#L100 . If you don\u0027t use the PrometheusAPIClient directly, you\u0027ll get the PromQLRbac project initialized correctly as well and you won\u0027t need to figure out how to do it yourself.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e02e843f8ea08858c10aefd12eb498f8bdad3f01","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"73756ed5_183d721a","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"2c01bb0c_146265a4","updated":"2025-07-09 14:11:20.000000000","message":"its not marked that way and i think it\u0027s actually the reverse the clinet you are using I think is only intended to be use by the cli not by Python project using it as Python binding.\n\nthe watcher spec we just approved for aetos integration also expcivly talks about using the PrometheusAPIClient object.\n\nhttps://opendev.org/openstack/watcher-specs/src/branch/master/specs/2025.2/approved/prometheus-multitenancy-support.rst\n\nSo there are no plans to move to using the other client module.\n\nif it was intended for internal use it shoudl be in an internal module like in os-vif \n\nhttps://github.com/openstack/os-vif/tree/master/os_vif/internal\nthat would be a breakign change however so that shoudl not be change now.\n\nit would need a new major release and we would not backport changes for that in all likelihood so chaning that now is kind of too late to do without deprecating it in 2026.1 and then removing it in 2026.2\n\nusing https://github.com/openstack/debtcollector/blob/master/debtcollector/moves.py","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"a12c396e8ba0418798cad6e4098bc3b5b407c06d","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"2c01bb0c_146265a4","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"2d079be1_a12db487","updated":"2025-07-09 13:39:48.000000000","message":"For what I understand, we should use the python-observabilityclient. The Prometheus client is only for internal use of the python-observabilityclient.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5b821102cdf1139b1acaf7a27171a5be699afd35","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"2d079be1_a12db487","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"4f61d8fb_3194a159","updated":"2025-07-08 16:30:36.000000000","message":"https://opendev.org/openstack/python-observabilityclient/src/branch/master/observabilityclient/v1/python_api.py#L51\n\nso the parmeter exists in that client\n\nbut not in https://opendev.org/openstack/python-observabilityclient/src/branch/master/observabilityclient/prometheus_client.py#L107\n\nwhich is hte one i was expectign to use","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f3e3bc84f3c7d379ef9ac30f2be0a04cfa636ad6","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"0904db6c_e6f7b3d8","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"5491cd77_45c41679","updated":"2025-07-10 01:52:05.000000000","message":"well the siginture of the new client are also not correct\n\n\nrbacs is a configuration of the server and not something that a client shoudl be opting in too and certnelly on on each call to an function on a clinet object\n\n\nthe other client is also descibe as \n\n\"\"\"Client for the observabilityclient api.\"\"\"\n\nwhich is not really a thing.\n\nit also goes out of band and logs configuration files\n\n```\nDEFAULT_CONFIG_LOCATIONS \u003d (\n    [os.path.join(os.environ[\"HOME\"], \".config/openstack/\"), \"/etc/openstack/\"]\n    if \"HOME\" in os.environ\n    else [\"/etc/openstack/\"]\n)\nCONFIG_FILE_NAME \u003d \"prometheus.yaml\"\n```\nhttps://opendev.org/openstack/python-observabilityclient/src/branch/master/observabilityclient/utils/metric_utils.py#L27-L70\n\n\nand swarkign the home/user directly wich may be ok in a clien but is not ok in a service.\n\nthat part of the reason we did not use it in watcher.\n\nServices are meant to use oslo. config for configuration, and we are not meant to look in locations like the user home directory.\n\nespically in the context of a horzion plugin.\n\nWhile there was an effort to move to oslo.config that was abandoned in the past, Horizon uses Python settings files for config, so looking for a Prometheus. YAMLs is not ok in this context.\n\nthere is a lot more technial debt in the client you are propsoing then the \nhttps://opendev.org/openstack/python-observabilityclient/src/branch/master/observabilityclient/prometheus_client.py\nmodule so why would we prefer https://opendev.org/openstack/python-observabilityclient/src/branch/master/observabilityclient/v1/python_api.py ?","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d7ffe33c7a9ccad4165baee283dd918aabb8c42a","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"b68f860d_39fef163","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"5bf36d8f_308ea412","updated":"2025-07-18 11:38:50.000000000","message":"thanks so part of the issue with walking the file system on construction of the client is we need to create client over and over again in the services when servicing request form diffent users and the file system walk is expensive.\n\nthat why we parse all the config once on service startup and then pass in the relevent paramteres\n\nthat why i said this approch is fine for a cli which often will not have a long running session and need to parse the cofnig for every execution.\nbut its a prefomnace issue for something like grian ui where should should not need to do it every time a user refresh there web broswer","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"ebc71dd5cb377eee55009c9c2f55954cd741bec0","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5491cd77_45c41679","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"73756ed5_183d721a","updated":"2025-07-09 21:51:10.000000000","message":"Victoria is right, about which module to use. The PrometheusAPIClient was indeed intended for internal use only, so lets keep it that way with new projects. I\u0027m aware that this may not be obvious, since it\u0027s lacking proper documentation (that\u0027s still in our backlog), but this is shown at least in the readme.\n\nOnce you have the client initialized, you\u0027ll be able to use the functions here https://opendev.org/openstack/python-observabilityclient/src/branch/master/observabilityclient/v1/python_api.py . If there is something missing, we can easily add it.\n\nRegarding the watcher-spec, Watcher is already successfully using the PrometheusAPIClient, so I went with what\u0027s already there. The purpose of the spec is to add Aetos support, which is completely possible while using the PrometheusAPIClient, not to persuade you to migrate to a different python module.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"be18f9b0dca89d554e2e34457ffb900f3796b213","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5bf36d8f_308ea412","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"8b0e418b_4022d69f","updated":"2025-07-18 08:54:55.000000000","message":"I will open a bug in python_observabilityclient to address this work.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669a835b1ccc851c8429a8cf3c622bbaace871f5","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"8b0e418b_4022d69f","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"8c934673_6c60e9cf","updated":"2025-07-15 19:43:41.000000000","message":"My primary concern with the current approach is that the observabilityclient attempts to locate a prometheus.yaml file by walking the filesystem when the client is instantiated. This behavior, seen in metric_utils.py and client.py, is problematic for Horizon plugins and services like Watcher or Aodh.\n\nWhile this might be acceptable for a CLI tool, Horizon plugins should exclusively store configuration information in local_settings.d files using the Django settings system.\n\nI\u0027m perfectly fine with addressing this directly in the observabilityclient so that the client can be instantiated without this filesystem traversal. However, until this is resolved, we won\u0027t be able to move Watcher to use the other client.\n\nClient Constructor Best Practices\n---------------------------------\n\nFor client projects intended as Python bindings to an OpenStack service, the client object is generally designed to take all necessary parameters as arguments to its constructor. Examples of this pattern can be seen in:\n\n    Neutron:\n\n        https://github.com/openstack/python-neutronclient/blob/master/neutronclient/v2_0/client.py#L188-L242\n\n        https://github.com/openstack/python-neutronclient/blob/master/neutronclient/neutron/client.py\n\n    Glance:\n\n        https://github.com/openstack/python-glanceclient/blob/master/glanceclient/client.py#L23\n\n    Nova:\n\n        https://github.com/openstack/python-novaclient/blob/master/novaclient/client.py#L237-L314\n\nThe OpenStack SDK, for example, supports various methods for creating connections based on application needs. For the OpenStack CLI, it supports the cloud.yaml format, but for existing projects, it allows using an existing Keystone auth session, an oslo.config object, or passing authentication parameters as keyword arguments. You can see this flexibility here: https://github.com/openstack/openstacksdk/blob/master/openstack/connection.py#L33-L211.\n\nWe initially selected PrometheusAPIClient because it closely followed this standard pattern.\n\nPath Forward for observability_client.Client\n--------------------------------------------\n\nI am amenable to using the other observability_client.Client, provided we ensure it functions end-to-end without relying on the existence of prometheus.yaml. All queries to Prometheus via observability_client.Client should originate from a client instance constructed with a session object that is built from the user\u0027s token.\n\nThis is similar to how Aodh handles it: https://opendev.org/openstack/aodh/src/branch/master/aodh/evaluator/prometheus.py#L44-L48.\n\nHowever, we should not use configuration values to generate the session object. Instead, we need to construct the session object directly, similar to https://github.com/openstack/keystoneauth/blob/master/keystoneauth1/session.py#L284.\n\nIn Horizon, this logic is typically abstracted away by the service client. Refer to these examples:\n\n    Glance: https://github.com/openstack/horizon/blob/master/openstack_dashboard/api/glance.py#L115-L123\n\n    Nova: https://github.com/openstack/horizon/blob/master/openstack_dashboard/api/_nova.py#L159-L182\n\n    Neutron: https://github.com/openstack/horizon/blob/master/openstack_dashboard/api/neutron.py#L971-L978\n\nNeutron is a particularly good example as it\u0027s currently migrating to the OpenStack SDK. You can observe how they manually create the session object from the request, including the cacert: https://github.com/openstack/horizon/blob/master/openstack_dashboard/api/neutron.py#L984-L1002.\n\nIdeally, this session creation logic would be ported to the __init__ method of the observability_client.Client.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"992b7e3119cbc5bdc3d547998e342ef40510b42d","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"4f61d8fb_3194a159","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"a28604d0_39eeac08","updated":"2025-07-08 16:23:37.000000000","message":"I noticed the weird behavior and it seemed I had a very old copy of the python-observabilityclient. I already have a fix for this. As correctly pointed out, we shouldn\u0027t need to explicitly pass the disable_rbac\u003dTrue. I will follow up tomorrow.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"0b24c387689252b5f225803cf8b336ac3be835c9","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    def get_volumes_count(self):"},{"line_number":23,"context_line":"        \"\"\"Get telemetry volumes count.\"\"\""},{"line_number":24,"context_line":"        query \u003d self.client.query.query("},{"line_number":25,"context_line":"            \"count(ceilometer_volume_size)\", disable_rbac\u003dTrue"},{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"8c934673_6c60e9cf","line":25,"range":{"start_line":25,"start_character":44,"end_line":25,"end_character":62},"in_reply_to":"f717f4a1_e58fb9d1","updated":"2025-07-15 16:07:33.000000000","message":"Thanks for chiming in Jaromir. I understand the concerns Sean, but I think we should use python-observabilityclient as intended by the maintainers. If there is tech debt, we should collaborate on fixing it. Using the PrometheusClient, while tempting, has its shortcomings and might cause issues in the future.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669a835b1ccc851c8429a8cf3c622bbaace871f5","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def get_snapshots_count(self):"},{"line_number":30,"context_line":"        \"\"\"Get telemetry snapshots count.\"\"\""},{"line_number":31,"context_line":"        query \u003d self.client.query.query("},{"line_number":32,"context_line":"            \"count(ceilometer_volume_snapshot_size)\", disable_rbac\u003dTrue"}],"source_content_type":"text/x-python","patch_set":10,"id":"d8c42aff_ef5da4d9","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":2},"updated":"2025-07-15 19:43:41.000000000","message":"so get_volumes_count and this shoudl be in teh fake backend right.\n\nfor now you can likely just hard code them 42 or any other placeholder value.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"be18f9b0dca89d554e2e34457ffb900f3796b213","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        )"},{"line_number":27,"context_line":"        return query[0].value if query else 0"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def get_snapshots_count(self):"},{"line_number":30,"context_line":"        \"\"\"Get telemetry snapshots count.\"\"\""},{"line_number":31,"context_line":"        query \u003d self.client.query.query("},{"line_number":32,"context_line":"            \"count(ceilometer_volume_snapshot_size)\", disable_rbac\u003dTrue"}],"source_content_type":"text/x-python","patch_set":10,"id":"02055860_fb2fcad2","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":2},"in_reply_to":"d8c42aff_ef5da4d9","updated":"2025-07-18 08:54:55.000000000","message":"Yes, I think it should. It is on the abstract class, then we should implement for all the datasources.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669a835b1ccc851c8429a8cf3c622bbaace871f5","unresolved":true,"context_lines":[{"line_number":34,"context_line":"        return query[0].value if query else 0"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_client():"},{"line_number":38,"context_line":"    \"\"\"Get a Prometheus telemetry client instance.\"\"\""},{"line_number":39,"context_line":"    return PrometheusTelemetryClient()"}],"source_content_type":"text/x-python","patch_set":10,"id":"01126c43_41b97272","line":37,"range":{"start_line":37,"start_character":4,"end_line":37,"end_character":14},"updated":"2025-07-15 19:43:41.000000000","message":"the other thing we need to be careful of is we cannot cache or reuse this across requests, we need to have a differnt client instnace per page request although we can use the single client object to make as many requests as needed for metrics for any given dashboard.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"be18f9b0dca89d554e2e34457ffb900f3796b213","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        return query[0].value if query else 0"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def get_client():"},{"line_number":38,"context_line":"    \"\"\"Get a Prometheus telemetry client instance.\"\"\""},{"line_number":39,"context_line":"    return PrometheusTelemetryClient()"}],"source_content_type":"text/x-python","patch_set":10,"id":"bce8ab76_f639aec1","line":37,"range":{"start_line":37,"start_character":4,"end_line":37,"end_character":14},"in_reply_to":"01126c43_41b97272","updated":"2025-07-18 08:54:55.000000000","message":"Indeed. I think we are already doing this, but I can check. Initially this shouldn\u0027t be a big issue if we aim to implement dashboards for the admin only.","commit_id":"4e1ccfbfd90a6b0f05293d0ed67a380aa2d89159"}],"src/grian_ui/local/local_settings.d/_90_grian_ui.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"742548d15582265fc12f9c57d4d8c829e3ee5b37","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# SPDX-License-Identifier: Apache-2.0"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"GRIAN_PLUGIN \u003d {\"datasource\": \"prometheus\"}"}],"source_content_type":"text/x-python","patch_set":12,"id":"c516ef00_25082a01","line":4,"updated":"2025-07-15 21:21:11.000000000","message":"thinking about this more we agreed to have a spereate Aetos data souce in watcher so im inclided to say we shoudl do the same here for consitency.\n\nthat raise the question however, do we want to support direct prometheus connections at all or should we require ateos form the start?","commit_id":"dceca39701953b9d6bfbdf0f255f68c27b386e53"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"be18f9b0dca89d554e2e34457ffb900f3796b213","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# SPDX-License-Identifier: Apache-2.0"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"GRIAN_PLUGIN \u003d {\"datasource\": \"prometheus\"}"}],"source_content_type":"text/x-python","patch_set":12,"id":"18a34344_f338d3ff","line":4,"in_reply_to":"c516ef00_25082a01","updated":"2025-07-18 08:54:55.000000000","message":"Well, with python-observabilityclient we get the Aetos setup and the wrapper for connecting with Prometheus. We just simply pass the tenant info and the resulting query corresponds to what that tenant can see.\n\nWith a direct Prometheus connection we lose that, but we might gain in simplicity.\n\nSince multitenancy is something we want to guarantee for this case, I would go first with python-observabilityclient (that way we collaborate with both projects) and then, if we see the need, we can implement a direct Prometheus datasource.\n\nWDYT?","commit_id":"dceca39701953b9d6bfbdf0f255f68c27b386e53"}]}