ERA-12674: Add alert rules and notification method CRUD#36
Open
JoshuaVulcan wants to merge 6 commits intomainfrom
Open
ERA-12674: Add alert rules and notification method CRUD#36JoshuaVulcan wants to merge 6 commits intomainfrom
JoshuaVulcan wants to merge 6 commits intomainfrom
Conversation
JoshuaVulcan
added
autoreviewing
…lients
Implements full CRUD operations for alert rules and notification methods:
Sync ERClient methods:
- get_alerts, get_alert, post_alert, patch_alert, delete_alert
- get_alert_conditions
- get_notification_methods, get_notification_method
- post_notification_method, patch_notification_method, delete_notification_method
Async ERClient methods:
- Same set of async methods for AsyncERClient
- Adds async _delete helper that properly handles 204 No Content responses
(avoids JSON parsing on empty DELETE responses)
Endpoints covered:
- activity/alerts (list/create)
- activity/alert/{id} (get/patch/delete)
- activity/alerts/conditions (list)
- activity/notificationmethods (list/create)
- activity/notificationmethod/{id} (get/patch/delete)
Test coverage:
- Comprehensive async tests using respx for all alert and notification methods
- Comprehensive sync tests using unittest.mock for all alert and notification methods
- Success, not found, forbidden, unauthorized, timeout, and empty response cases
ERA-12674
Co-authored-by: Cursor <cursoragent@cursor.com>
…st modules The test_alerts_notifications.py files were duplicates of the more thorough test_alerts.py and test_notification_methods.py files which provide better coverage with additional error cases and edge cases. Co-authored-by: Cursor <cursoragent@cursor.com>
JoshuaVulcan
force-pushed
the
ERA-12674/alerts-notifications
branch
from
d14182e to
35389f9
Compare
This was referenced Feb 11, 2026
# Conflicts: # tests/sync_client/conftest.py
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds full CRUD operations for alert rules and notification methods to both the synchronous ERClient and asynchronous AsyncERClient classes, along with comprehensive test coverage for both clients.
Changes:
- Adds
get_alerts,get_alert,post_alert,patch_alert,delete_alert,get_alert_conditions,get_notification_methods,get_notification_method,post_notification_method,patch_notification_method, anddelete_notification_methodmethods to bothERClientandAsyncERClient - Adds
get_alert_conditionswith optionalevent_typeandonly_common_factorsquery parameters - Adds comprehensive sync and async test coverage for all new endpoints including success, error, and edge case scenarios
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| erclient/client.py | Adds alert rule, alert condition, and notification method CRUD methods to both ERClient (sync) and AsyncERClient (async) classes |
| tests/sync_client/test_alerts.py | Sync client tests for alert rule CRUD and alert conditions |
| tests/sync_client/test_notification_methods.py | Sync client tests for notification method CRUD |
| tests/async_client/test_alerts.py | Async client tests for alert rule CRUD and alert conditions |
| tests/async_client/test_notification_methods.py | Async client tests for notification method CRUD |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+7
to
+27
|
|
||
|
|
||
| ALERT_ID = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" | ||
|
|
||
|
|
||
| def _mock_response(status_code, json_data=None, ok=None, text=None): | ||
| """Create a mock requests.Response.""" | ||
| mock_resp = MagicMock() | ||
| mock_resp.status_code = status_code | ||
| if ok is None: | ||
| mock_resp.ok = 200 <= status_code < 300 | ||
| else: | ||
| mock_resp.ok = ok | ||
| if json_data is not None: | ||
| mock_resp.text = json.dumps(json_data) | ||
| mock_resp.json.return_value = json_data | ||
| elif text is not None: | ||
| mock_resp.text = text | ||
| else: | ||
| mock_resp.text = "{}" | ||
| return mock_resp |
| from erclient import (ERClientBadCredentials, ERClientBadRequest, | ||
| ERClientException, ERClientInternalError, | ||
| ERClientNotFound, ERClientPermissionDenied, | ||
| ERClientRateLimitExceeded, ERClientServiceUnreachable) |
Comment on lines
+1119
to
+1121
| if kwargs.get('event_type'): | ||
| params['event_type'] = kwargs['event_type'] | ||
| if kwargs.get('only_common_factors'): |
Comment on lines
+1549
to
+1551
| if kwargs.get('event_type'): | ||
| params['event_type'] = kwargs['event_type'] | ||
| if kwargs.get('only_common_factors'): |
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.
Summary
ERClient(sync) andAsyncERClient(async) classes_deletehelper method that properly handles HTTP 204 No Content responses (avoids JSON parsing on empty DELETE response bodies)Endpoints Covered
activity/alertsactivity/alert/{id}activity/alerts/conditionsactivity/notificationmethodsactivity/notificationmethod/{id}Test Plan
pytest tests/ -v)Truewithout JSON parsingResolves ERA-12674