Update Grafana configuration and fix script paths#59
Conversation
- Modified JSON configuration for dashboards and panels by adding extensions and limits groups panels. - Updated folder paths (`resources/` to `scripts/resources/`) for some scripts that were broken and add a missing print.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGrafana resource base paths for alerts and dashboards were moved to Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/grafana_create_alerts.py (1)
17-19: Use a script-relative resource path here.Changing the literal to
scripts/resources/...still makes discovery depend on the caller's cwd. If this script is launched fromscripts/or another directory, the walk can quietly import nothing. Resolve the base directory fromscripts/grafana_create_alerts.pyinstead.📍 Suggested refactor
import argparse import os +from pathlib import Path @@ -GRAFANA_ALERTS_DIR = 'scripts/resources/grafana/alerts/' -GRAFANA_LOGS_ALERTS_DIR = GRAFANA_ALERTS_DIR + 'logs' -GRAFANA_METRICS_ALERTS_DIR = GRAFANA_ALERTS_DIR + 'metrics' +GRAFANA_ALERTS_DIR = Path(__file__).resolve().parent / 'resources' / 'grafana' / 'alerts' +GRAFANA_LOGS_ALERTS_DIR = GRAFANA_ALERTS_DIR / 'logs' +GRAFANA_METRICS_ALERTS_DIR = GRAFANA_ALERTS_DIR / 'metrics'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/grafana_create_alerts.py` around lines 17 - 19, GRAFANA_ALERTS_DIR, GRAFANA_LOGS_ALERTS_DIR and GRAFANA_METRICS_ALERTS_DIR currently use relative literals which break when the cwd differs; change GRAFANA_ALERTS_DIR to be resolved from the script location (use the module file path via __file__ — e.g. Path(__file__).resolve().parent / 'resources' / 'grafana' / 'alerts') and derive GRAFANA_LOGS_ALERTS_DIR and GRAFANA_METRICS_ALERTS_DIR from that Path (or str() them where strings are required) so the walker always finds resources regardless of caller cwd.scripts/grafana_create_dashboards.py (1)
17-17: Make the dashboard root independent from the process cwd.This literal still only works for one launch location. In the
--allbranch, running the script fromscripts/or another cwd will walk a non-existent directory and import nothing. Derive the path fromscripts/grafana_create_dashboards.pyinstead.📍 Suggested refactor
import argparse import os +from pathlib import Path @@ -GRAFANA_DASHBOARDS_DIR = 'scripts/resources/grafana/dashboards' +GRAFANA_DASHBOARDS_DIR = Path(__file__).resolve().parent / 'resources' / 'grafana' / 'dashboards'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/grafana_create_dashboards.py` at line 17, GRAFANA_DASHBOARDS_DIR is a hard-coded relative path that fails when the script is run from a different cwd; change the constant to compute the dashboards root relative to the script file by resolving the current module path (use Path(__file__).resolve().parent and append "resources/grafana/dashboards") so all code that references GRAFANA_DASHBOARDS_DIR (the constant in the grafana_create_dashboards module) will walk the correct directory regardless of the process cwd.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/grafana_create_folder.py`:
- Line 23: create_folder(folder_name_to_create) can return None on conflict, so
printing it yields "None" on reruns; instead call
functions.grafana.folders.create_folder and if it returns None, look up the
existing folder UID and print that. Concretely: assign uid =
functions.grafana.folders.create_folder(folder_name_to_create); if uid is falsy,
call the folders lookup/list helper (e.g.
functions.grafana.folders.get_folder_by_title or list_folders and match title ==
folder_name_to_create) to extract the existing folder's uid, then print that uid
— ensuring the script always emits a usable folder UID rather than None.
In
`@scripts/resources/grafana/dashboards/applications/network_storage_metrics.json`:
- Line 21: The top-level dashboard numeric "id" should be unset to avoid
instance-local ID coupling; in the dashboard JSON remove the "id" field (or set
it to null) so imports rely on "dashboard.uid" with overwrite:true instead of a
fixed numeric id; locate the top-level "id" entry (currently "id": 24) in the
dashboard JSON and delete it or change its value to null.
---
Nitpick comments:
In `@scripts/grafana_create_alerts.py`:
- Around line 17-19: GRAFANA_ALERTS_DIR, GRAFANA_LOGS_ALERTS_DIR and
GRAFANA_METRICS_ALERTS_DIR currently use relative literals which break when the
cwd differs; change GRAFANA_ALERTS_DIR to be resolved from the script location
(use the module file path via __file__ — e.g. Path(__file__).resolve().parent /
'resources' / 'grafana' / 'alerts') and derive GRAFANA_LOGS_ALERTS_DIR and
GRAFANA_METRICS_ALERTS_DIR from that Path (or str() them where strings are
required) so the walker always finds resources regardless of caller cwd.
In `@scripts/grafana_create_dashboards.py`:
- Line 17: GRAFANA_DASHBOARDS_DIR is a hard-coded relative path that fails when
the script is run from a different cwd; change the constant to compute the
dashboards root relative to the script file by resolving the current module path
(use Path(__file__).resolve().parent and append "resources/grafana/dashboards")
so all code that references GRAFANA_DASHBOARDS_DIR (the constant in the
grafana_create_dashboards module) will walk the correct directory regardless of
the process cwd.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21d8e1f7-509d-4cd8-af05-dcbeb56cd2e0
📒 Files selected for processing (4)
scripts/grafana_create_alerts.pyscripts/grafana_create_dashboards.pyscripts/grafana_create_folder.pyscripts/resources/grafana/dashboards/applications/network_storage_metrics.json
…ion: - Removed redundant return statements in `create_folder`. - Adjusted print statements for clearer folder UID logging. - Set dashboard `id` to `null` in JSON configuration. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/functions/grafana/folders.py (1)
22-27:⚠️ Potential issue | 🔴 Critical
create_folder()return contract regression breaks caller logicAt Line 26,
create_folder(...)no longer returns a UID. Inscripts/grafana_create_dashboards.py(Lines 38 and 35), that return value is stored/read forparent_folder_uid; returningNonecan collapse folder hierarchy creation.Suggested fix
def create_folder(folder_name, parent_folder_uid = '', recreate = False): folder_uid = folder_name + FOLDER_BASE_UID if recreate: delete_folder(folder_uid) - __create_folder({'uid': folder_uid, 'title': folder_name, 'parentUid' : parent_folder_uid}) + __create_folder({'uid': folder_uid, 'title': folder_name, 'parentUid' : parent_folder_uid}) + return folder_uid🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/functions/grafana/folders.py` around lines 22 - 27, The create_folder function lost its return value causing callers (e.g., scripts/grafana_create_dashboards.py reading parent_folder_uid) to get None; restore a meaningful return by returning the folder UID (or the created folder's UID from __create_folder if that is authoritative). Specifically update create_folder(folder_name, parent_folder_uid='', recreate=False) to return folder_uid (or the response UID from __create_folder) after calling delete_folder and __create_folder so callers receive the expected UID.scripts/resources/grafana/dashboards/applications/network_storage_metrics.json (1)
25-33:⚠️ Potential issue | 🟠 MajorMove panels out of expanded row containers.
In Grafana's dashboard JSON, when a row is expanded (
"collapsed": false), its child panels must not be nested inside the row object'spanelsarray. Instead, they must appear as siblings in the top-levelpanels[]array immediately after the row header. Grafana's official behavior treats row membership for expanded rows based on panel ordering, not nesting—nestedpanelsin expanded rows are ignored, which risks panels disappearing or misaligning on save/import.These four row panels (lines 25-33, 652-660, 1176-1184, 1292-1300) are marked
collapsed: falsebut still contain all charts inside theirpanelsarrays. Either move each section's charts to the top-levelpanelsarray right after its row header, or change the rows tocollapsed: trueto enable nested storage.🧭 Suggested structure
{ "collapsed": false, "gridPos": { ... }, "id": 14, - "panels": [ - { ... first section panel ... }, - { ... second section panel ... } - ], + "panels": [], "title": "General metrics", "type": "row" }, + { ... first "General metrics" panel ... }, + { ... second "General metrics" panel ... },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/resources/grafana/dashboards/applications/network_storage_metrics.json` around lines 25 - 33, The row object with "id": 14 (and the other three expanded rows noted) is marked "collapsed": false but still contains child charts in its "panels" array; Grafana ignores nested panels for expanded rows. Fix by either setting "collapsed": true on those row objects (so nested "panels" may remain) or by extracting each row's child panel objects out of the row's "panels" array and inserting them as siblings in the top-level "panels" array immediately after the row header object (preserving their gridPos and id fields) so panel ordering defines row membership.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/functions/grafana/folders.py`:
- Around line 28-34: The Grafana HTTP calls currently use requests without
timeouts and can hang; update every requests call in this module to include a
timeout (e.g., timeout=30). Specifically add timeout=30 to the requests.post
call inside __create_folder and to the other requests.get and requests.delete
calls referenced in this file so all HTTP requests (post/get/delete) pass a
timeout argument to prevent indefinite blocking.
---
Outside diff comments:
In `@scripts/functions/grafana/folders.py`:
- Around line 22-27: The create_folder function lost its return value causing
callers (e.g., scripts/grafana_create_dashboards.py reading parent_folder_uid)
to get None; restore a meaningful return by returning the folder UID (or the
created folder's UID from __create_folder if that is authoritative).
Specifically update create_folder(folder_name, parent_folder_uid='',
recreate=False) to return folder_uid (or the response UID from __create_folder)
after calling delete_folder and __create_folder so callers receive the expected
UID.
In
`@scripts/resources/grafana/dashboards/applications/network_storage_metrics.json`:
- Around line 25-33: The row object with "id": 14 (and the other three expanded
rows noted) is marked "collapsed": false but still contains child charts in its
"panels" array; Grafana ignores nested panels for expanded rows. Fix by either
setting "collapsed": true on those row objects (so nested "panels" may remain)
or by extracting each row's child panel objects out of the row's "panels" array
and inserting them as siblings in the top-level "panels" array immediately after
the row header object (preserving their gridPos and id fields) so panel ordering
defines row membership.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df3c2920-2ee2-4454-98fb-50d9c9d92654
📒 Files selected for processing (2)
scripts/functions/grafana/folders.pyscripts/resources/grafana/dashboards/applications/network_storage_metrics.json
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/functions/grafana/folders.py (2)
35-39:⚠️ Potential issue | 🟠 MajorGuard exception paths when no HTTP response exists
In these
except RequestExceptionblocks,resultcan beNone(e.g., connection error before response), soresult.status_code/result.contentcan raiseAttributeErrorand hide the real failure.Proposed fix
except RequestException as e: - if result.status_code == requests.codes.conflict: + status_code = getattr(result, "status_code", None) + if status_code == requests.codes.conflict: print("Folder already exist : %s" % json_data['title']) else: - raise SystemExit(e, result.content) + raise SystemExit(e, getattr(result, "content", None))Also applies to: 47-48, 61-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/functions/grafana/folders.py` around lines 35 - 39, The except RequestException handlers (e.g., in the folder creation/update functions around the "except RequestException as e:" blocks) assume a response object `result` always exists; when a request fails before a response `result` can be None and `result.status_code` or `result.content` will raise and mask the real error. Fix by guarding accesses: test `if result is not None and getattr(result, "status_code", None) == requests.codes.conflict:` to detect conflicts, and when logging/raising include safe fallbacks (use `result.content` only if `result` is not None, otherwise include `str(e)` or `repr(e)`). Apply the same guard pattern to the other except blocks noted (lines ~47-48 and ~61-62).
22-27:⚠️ Potential issue | 🔴 CriticalAPI contract break:
create_foldernow returnsNone, but callers depend on the return valueThe function no longer returns a value, yet line 38 in
scripts/grafana_create_dashboards.pyassigns the return tofolders_uidsdict:folders_uids[os.path.join(path, subdir)] = create_folder(subdir, parent_folder_uid, recreate=True)This will store
Noneinstead of the expected folder UID, causing silent failures in any code that consumes from this dict. Either restore the return statement (returning the folder_uid) or update all callers to not use the return value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/functions/grafana/folders.py` around lines 22 - 27, The create_folder function removed its return value causing callers that assign its result (e.g. storing into folders_uids from create_folder(...)) to receive None; restore the API by returning the computed folder_uid at the end of create_folder (after the __create_folder call) so callers relying on the folder UID continue to work; ensure the returned value is the same symbol folder_uid used inside the function.
♻️ Duplicate comments (1)
scripts/functions/grafana/folders.py (1)
32-32:⚠️ Potential issue | 🟠 MajorAdd request timeouts to all Grafana HTTP calls
requests.post/get/deletestill have notimeout, so provisioning can hang indefinitely on network stalls. This was already flagged previously and appears unresolved.Proposed fix
+REQUEST_TIMEOUT_SECONDS = 30 + - result = requests.post(GRAFANA_FOLDERS_URL, json=json_data, headers=constant.GRAFANA_HEADERS, cookies=constant.GRAFANA_COOKIES) + result = requests.post( + GRAFANA_FOLDERS_URL, + json=json_data, + headers=constant.GRAFANA_HEADERS, + cookies=constant.GRAFANA_COOKIES, + timeout=REQUEST_TIMEOUT_SECONDS, + ) - result = requests.get(GRAFANA_FOLDER_URL.format(folderUuid=folder_uuid), headers=constant.GRAFANA_HEADERS, cookies=constant.GRAFANA_COOKIES) + result = requests.get( + GRAFANA_FOLDER_URL.format(folderUuid=folder_uuid), + headers=constant.GRAFANA_HEADERS, + cookies=constant.GRAFANA_COOKIES, + timeout=REQUEST_TIMEOUT_SECONDS, + ) - result = requests.get(GRAFANA_FOLDER_URL.format(folderUuid=folder_uid), headers=constant.GRAFANA_HEADERS, cookies=constant.GRAFANA_COOKIES) + result = requests.get( + GRAFANA_FOLDER_URL.format(folderUuid=folder_uid), + headers=constant.GRAFANA_HEADERS, + cookies=constant.GRAFANA_COOKIES, + timeout=REQUEST_TIMEOUT_SECONDS, + ) - result = requests.delete(GRAFANA_FOLDER_URL.format(folderUuid=folder_uid), headers=constant.GRAFANA_HEADERS, cookies=constant.GRAFANA_COOKIES) + result = requests.delete( + GRAFANA_FOLDER_URL.format(folderUuid=folder_uid), + headers=constant.GRAFANA_HEADERS, + cookies=constant.GRAFANA_COOKIES, + timeout=REQUEST_TIMEOUT_SECONDS, + )Also applies to: 44-44, 54-54, 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/functions/grafana/folders.py` at line 32, Add a request timeout to every Grafana HTTP call to avoid hangs: update the requests.post/get/delete calls that interact with GRAFANA_FOLDERS_URL (and the other calls on the same file) to pass a sensible timeout parameter (e.g., timeout=10 or use a new constant like GRAFANA_REQUEST_TIMEOUT) alongside json=headers=cookies; ensure the timeout is applied to the calls that use GRAFANA_FOLDERS_URL, constant.GRAFANA_HEADERS, and constant.GRAFANA_COOKIES (and the other requests in this module at the indicated lines).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/functions/grafana/folders.py`:
- Around line 35-39: The except RequestException handlers (e.g., in the folder
creation/update functions around the "except RequestException as e:" blocks)
assume a response object `result` always exists; when a request fails before a
response `result` can be None and `result.status_code` or `result.content` will
raise and mask the real error. Fix by guarding accesses: test `if result is not
None and getattr(result, "status_code", None) == requests.codes.conflict:` to
detect conflicts, and when logging/raising include safe fallbacks (use
`result.content` only if `result` is not None, otherwise include `str(e)` or
`repr(e)`). Apply the same guard pattern to the other except blocks noted (lines
~47-48 and ~61-62).
- Around line 22-27: The create_folder function removed its return value causing
callers that assign its result (e.g. storing into folders_uids from
create_folder(...)) to receive None; restore the API by returning the computed
folder_uid at the end of create_folder (after the __create_folder call) so
callers relying on the folder UID continue to work; ensure the returned value is
the same symbol folder_uid used inside the function.
---
Duplicate comments:
In `@scripts/functions/grafana/folders.py`:
- Line 32: Add a request timeout to every Grafana HTTP call to avoid hangs:
update the requests.post/get/delete calls that interact with GRAFANA_FOLDERS_URL
(and the other calls on the same file) to pass a sensible timeout parameter
(e.g., timeout=10 or use a new constant like GRAFANA_REQUEST_TIMEOUT) alongside
json=headers=cookies; ensure the timeout is applied to the calls that use
GRAFANA_FOLDERS_URL, constant.GRAFANA_HEADERS, and constant.GRAFANA_COOKIES (and
the other requests in this module at the indicated lines).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e862bb9-350d-4fc3-b6b1-00a87a1f2732
📒 Files selected for processing (1)
scripts/functions/grafana/folders.py
- Ensure `create_folder` returns the folder UID. - Update print statements to log the correct UID. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
…reation: - Clarify "uid" description and its usage. - Refine JSON field modifications for dashboard creation. Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
|



PR Summary
resources/toscripts/resources/and added a missing print statement.