fix: added method for removal of notification policy tree #236
Conversation
|
Warning Rate limit exceeded@serhiy-yevtushenko has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughAdds synchronous and asynchronous Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant AP as AlertingProvisioning (sync/async)
participant G as Grafana API
Caller->>AP: delete_notification_policy_tree()
AP->>G: DELETE /v1/provisioning/policies
G-->>AP: 2xx / 4xx / 5xx Response
AP-->>Caller: return response
note right of AP: new method added (sync & async)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #236 +/- ##
==========================================
+ Coverage 92.26% 92.27% +0.01%
==========================================
Files 27 27
Lines 1822 1825 +3
==========================================
+ Hits 1681 1684 +3
Misses 141 141
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
grafana_client/elements/alertingprovisioning.py (1)
193-199: Bug: delete_mute_timing uses GET instead of DELETE.This will not delete the resource and is likely a copy/paste error.
- return self.client.GET(delete_mute_timing_path) + return self.client.DELETE(delete_mute_timing_path)
🧹 Nitpick comments (4)
grafana_client/elements/alertingprovisioning.py (4)
148-155: Nit: align docstring wording with surrounding methods.Prefer “Deletes notification policy tree” for consistency with other “Deletes …” docstrings.
- Removes notification policy tree + Deletes notification policy tree
51-57: Docstring param mismatch in get_rule_group.The docstring mentions disable_provenance, but the method doesn’t accept it.
""" :param folder_uid: :param group_uid: - :param disable_provenance: :return: """
239-240: Nit: variable name says “create” in a delete method.Rename for clarity.
- create_template_path = "/v1/provisioning/templates/%s" % template_name - return self.client.DELETE(create_template_path) + delete_template_path = "/v1/provisioning/templates/%s" % template_name + return self.client.DELETE(delete_template_path)
148-155: Tests and docs follow-upPlease add a unit test that asserts client.DELETE is called with “/v1/provisioning/policies” and update docs/readme to mention the new API. I can draft both if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
grafana_client/elements/alertingprovisioning.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (ubuntu-22.04, pypy-3.9)
🔇 Additional comments (1)
grafana_client/elements/alertingprovisioning.py (1)
148-155: Good addition: DELETE endpoint wired to existing policies path.Method name, path, and return pattern are consistent with adjacent GET/PUT methods for the same resource. Nice, lean change.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
grafana_client/elements/_async/alertingprovisioning.py (1)
193-199: Bug: delete_mute_timing uses GET instead of DELETEUnrelated to this PR but nearby; this likely prevents deletions.
Apply:
- return await self.client.GET(delete_mute_timing_path) + return await self.client.DELETE(delete_mute_timing_path)
🧹 Nitpick comments (1)
grafana_client/elements/_async/alertingprovisioning.py (1)
148-155: Async delete_notification_policy_tree: endpoint/method look correct; please add tests/docsMatches existing get/set pattern and uses the right path. Suggest adding a unit test asserting a DELETE to "/v1/provisioning/policies" is issued, and updating docs/changelog accordingly.
Example test sketch (pytest + mock):
import pytest import asyncio @pytest.mark.asyncio async def test_delete_notification_policy_tree_calls_correct_endpoint(mocker): client = mocker.Mock() client.DELETE = mocker.AsyncMock(return_value={"status": 204}) ap = AlertingProvisioning(client) resp = await ap.delete_notification_policy_tree() client.DELETE.assert_awaited_once_with("/v1/provisioning/policies") assert resp == {"status": 204}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
grafana_client/elements/_async/alertingprovisioning.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
grafana_client/elements/_async/alertingprovisioning.py (1)
grafana_client/elements/alertingprovisioning.py (1)
delete_notification_policy_tree(148-154)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test (ubuntu-22.04, pypy-3.9)
- GitHub Check: test (ubuntu-22.04, pypy-3.10)
2 participants
Description
Fixes the issue #234 - added a method delete_notification_policy_tree, which performs call to the existing Grafana End-Point
Checklist