fix(metrics): warn when the packaged Xenna Grafana dashboard is missing (#2189) - #2371
Open
chethanuk wants to merge 1 commit into
Open
fix(metrics): warn when the packaged Xenna Grafana dashboard is missing (#2189)#2371chethanuk wants to merge 1 commit into
chethanuk wants to merge 1 commit into
Conversation
write_grafana_configs folded the source check into the copy condition, so "package data missing" and "already provisioned" shared one silent branch: a missing xenna_grafana_dashboard.json produced no log at any level and the user had to inspect site-packages to find out why the dashboard never appeared in Grafana. Guard on the destination first, then warn only when the packaged source is genuinely absent. A dashboard already present in dashboards_path is provisioned by Grafana regardless of the package, so re-runs stay silent. Adds a parametrized test over the four (src present, dst present) states. The missing-from-package case fails without this fix. Fixes NVIDIA-NeMo#2189 Signed-off-by: ChethanUK <chethanuk@outlook.com>
chethanuk
requested review from
oyilmaz-nvidia
and removed request for
a team
September 5, 2026 04:50
Contributor
Greptile SummaryThis PR distinguishes an already-provisioned Xenna dashboard from a missing packaged dashboard.
Confidence Score: 5/5The PR appears safe to merge, with the intended missing-dashboard warning and preservation behavior covered by tests. No actionable correctness, security, or repository-rule violations remain; the changed branch behaves correctly for realistic callers and all relevant file-presence states. Important Files Changed
Reviews (1): Last reviewed commit: "fix(metrics): warn when the packaged Xen..." | Re-trigger Greptile |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
write_grafana_configs()folded the packaged-file check into the copy condition:so "the packaged dashboard is missing" and "it is already provisioned" took the same silent
branch. When the JSON was absent from the install the function returned normally, having written
only the Ray dashboards, with no log at any level. The reporter had to list
site-packagestowork out why the Xenna dashboard never appeared in Grafana.
This guards on the destination first and warns only when the packaged source is genuinely
absent, so an already-provisioned dashboard stays quiet and is never overwritten.
Packaging itself is untouched: the missing-from-the-wheel half of the report was already fixed
by ad58743 (#1911), which added
recursive-include nemo_curator ... *.jsontoMANIFEST.in.What was left is the silent skip, which is what the issue's "Suggested fix" asks for.
closes #2189
Usage
No API change. When the packaged dashboard is missing,
write_grafana_configs()now logs itinstead of returning quietly:
Tests
Added a parametrized test in
tests/metrics/test_metrics_utils.pycovering the four(source present, destination present) states for
write_grafana_configs; only the"source missing, destination missing" case fails on
maintoday, with the reported symptom(no warning logged). All four pass with this fix, and the rest of
tests/metrics/(28 tests)is unaffected.
pytest tests/metrics/test_metrics_utils.py -k WriteGrafanaConfigs— 4 passedpytest tests/metrics/— 28 passedruff check/ruff format --checkon both changed files — cleanChecklist