Skip to content

SEP-1868: Report success when alerts push recreates a missing PMM rule - #1392

Open
olucasandrade wants to merge 3 commits into
mainfrom
SEP-1868
Open

SEP-1868: Report success when alerts push recreates a missing PMM rule#1392
olucasandrade wants to merge 3 commits into
mainfrom
SEP-1868

Conversation

@olucasandrade

@olucasandrade olucasandrade commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When POST /api/apps/alerts/push finds a template already registered in PMM, it no longer always reports skipped / Already present in PMM.
  • If create_rule succeeds (the rule was missing, including after an out-of-band delete), the item is success / Pushed successfully.
  • A genuine rule-name collision ("conflicts with existing" in the error detail) still reports skipped. Any other create_rule failure is error with the real detail, not swallowed.
  • Relates to SEP-1868. Out of scope: GET /api/apps/alerts/ in_pmm still means template presence only.

Tested

  • Log in at http://localhost:5174 (backend is SEP-1868 on :8000; Casdoor :9999; PMM :8443).
  • Open AlertsAlert Templates (/alerts/templates/). Confirm PMM is connected (no "PMM not connected" warning).
  • Check a template that is not already in PMM, click Push Selected, confirm Push to PMM. Expect a green check and Pushed successfully.
  • In PMM/Grafana, delete only the alert rule for that template (leave the template registered).
  • Back in SEP, the list still shows In PMM (template presence — unchanged). Select the same template and push again. Expect a green check and Pushed successfully, not a warning Already present in PMM.
  • Push the same template a third time without deleting the rule. Expect a warning icon and Already present in PMM.
  • Regression: a template that was never pushed still creates both template and rule and reports Pushed successfully.

Checklist

  • New/modified functions have type hints and rST docstrings
  • New tests added for new features or bug fixes
  • Covering tests pass locally (pytest tests/app/sep/apps/alerts/ — 189 passed). Full make test not re-run; no OpenAPI/schema change so make regen-specs was not run
  • Pre-commit hooks pass (ran on both commits)
  • Database migrations generated if models changed (make makemigrations) (N/A)
  • User-facing changes documented (README, inline help, UI text) (N/A — changelog fragment covers the push-result copy)
  • Configuration changes documented with examples (N/A)
  • Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket)

@olucasandrade olucasandrade self-assigned this Aug 21, 2026
@github-actions github-actions Bot added python app:alerts PR touches the alerts app slice labels Aug 21, 2026
@olucasandrade
olucasandrade marked this pull request as ready for review August 21, 2026 14:25
Copilot AI balanced review requested due to automatic review settings August 21, 2026 14:25
@olucasandrade
olucasandrade requested review from a team, maxbube and yyyyyyyan as code owners August 21, 2026 14:25
@olucasandrade olucasandrade added the qa passed Tests for this PR are completed and successful. label Aug 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates alert push results to distinguish recreated rules, existing-rule conflicts, and genuine failures.

Changes:

  • Reports successful missing-rule recreation.
  • Preserves skipped status for collisions and surfaces other errors.
  • Adds regression tests and a changelog fragment.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
app/sep/apps/alerts/api_routes.py Refines push-result handling.
tests/app/sep/apps/alerts/test_api_routes.py Tests revised result states.
changelog.d/SEP-1868.fixed.md Documents the fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/app/sep/apps/alerts/test_api_routes.py
Comment thread app/sep/apps/alerts/api_routes.py Outdated
Comment thread app/sep/apps/alerts/api_routes.py
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  app/sep
  inventory.py
  app/sep/apps/alerts
  api_routes.py
Project Total  

This report was generated by python-coverage-comment-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app:alerts PR touches the alerts app slice python qa passed Tests for this PR are completed and successful.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants