feat: add delete_event_type() to sync and async clients (ERA-12656)#30
Open
JoshuaVulcan wants to merge 27 commits intomainfrom
Open
feat: add delete_event_type() to sync and async clients (ERA-12656)#30JoshuaVulcan wants to merge 27 commits intomainfrom
JoshuaVulcan wants to merge 27 commits intomainfrom
Conversation
JoshuaVulcan
added a commit
that referenced
this pull request
Feb 11, 2026
…ia _call + 204 - Remove .cursor/plans/purely_async_er2er_f2b9c60a.plan.md (IDE artifact) - Add .cursor/ to .gitignore to prevent recurrence - Async _delete: delegate to _call() with 204 and DELETE 409 handling in _call - Update async test for 500 to accept ERClientInternalError message Co-authored-by: Cursor <cursoragent@cursor.com>
…R2Er event sharing)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Implements the v2 event type DELETE endpoint for both ERClient and
AsyncERClient. The method defaults to v2.0 (activity/eventtypes/{slug})
since the destroy action is primarily supported on the v2 viewset.
Changes:
- Add event_type_delete_path() helper to api_paths.py
- Add base_url parameter to sync _delete() for versioned endpoints
- Add base_url parameter to async _delete() for versioned endpoints
- Add 409 Conflict handling in both sync/async _delete() methods
- Add delete_event_type() to ERClient (sync)
- Add delete_event_type() to AsyncERClient (async)
- Add comprehensive tests: 9 async, 10 sync, 4 api_paths (23 new tests)
All 177 tests pass.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
a603ea0 to
1565e44
Compare
…n message Client raises ERClientInternalError with 'Internal Server Error' in the message, not 'Failed to delete'. Update pytest.raises match regex accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds client support for deleting Event Types via the ER API (defaulting to v2.0), including centralized path generation and comprehensive sync/async tests.
Changes:
- Introduces
delete_event_type(value, version)on bothERClientandAsyncERClient(defaultv2.0). - Adds
event_type_delete_path()toerclient/api_paths.pyfor version-aware delete endpoint construction. - Adds new unit tests for the path helper and new sync/async delete behavior across success and error scenarios.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
erclient/api_paths.py |
Adds event_type_delete_path() to build versioned delete paths. |
erclient/client.py |
Adds delete_event_type() to sync/async clients; updates sync _delete() to accept base_url + handle 409 conflicts. |
tests/test_api_paths.py |
Adds unit tests for the new delete-path helper across versions/aliases/error. |
tests/sync_client/test_delete_event_type.py |
Adds sync delete_event_type tests for v2 default, v1, alias, and error handling. |
tests/async_client/test_delete_event_type.py |
Adds async delete_event_type tests for v2 default, v1, alias, and error handling. |
.cursor/plans/purely_async_er2er_f2b9c60a.plan.md |
Adds a Cursor plan document (non-runtime artifact). |
💡 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.
| self.logger.debug('Deleting event type: %s (version %s)', value, version) | ||
| path = event_type_delete_path(version, value) | ||
| base_url = self._api_root(version) | ||
| result = await self._delete(path, base_url=base_url) |
Comment on lines
+1579
to
+1600
| async def delete_event_type(self, value, version=VERSION_2_0): | ||
| """ | ||
| Delete an event type by its slug (value). | ||
|
|
||
| The das server returns 204 No Content on success, or 409 Conflict if | ||
| the event type has associated events or alert rules. | ||
|
|
||
| :param value: The event type slug/value (e.g. "immobility_rep"). | ||
| :param version: API version segment. Defaults to v2.0 because the | ||
| destroy action is primarily supported on the v2 endpoint. | ||
| :returns: True on successful deletion. | ||
| :raises ERClientNotFound: If the event type does not exist. | ||
| :raises ERClientException: If the server returns 409 Conflict (event | ||
| type is in use) or another error. | ||
| """ | ||
| self.logger.debug('Deleting event type: %s (version %s)', value, version) | ||
| path = event_type_delete_path(version, value) | ||
| base_url = self._api_root(version) | ||
| result = await self._delete(path, base_url=base_url) | ||
| self.logger.debug('Result of event type delete: %s', result) | ||
| return result | ||
|
|
Comment on lines
+1
to
+41
| --- | ||
| name: Purely async er2er | ||
| overview: "Remove the sync fallback so the connector uses only the async path: require AsyncERClient, drop asyncio.to_thread for client build and run_sync, and optionally remove now-unused sync-only code from er2er_syncher." | ||
| todos: [] | ||
| isProject: false | ||
| --- | ||
|
|
||
| # Purely async er2er | ||
|
|
||
| ## Current flow | ||
|
|
||
| ```mermaid | ||
| flowchart LR | ||
| subgraph main [er2er_main extract] | ||
| A{ASYNC_ER_CLIENT_AVAILABLE?} | ||
| A -->|yes| B[build_async_er_clients] | ||
| A -->|no| C[asyncio.to_thread build_er_clients] | ||
| B --> D[async_run_sync] | ||
| C --> D | ||
| D --> E{Both AsyncERClient?} | ||
| E -->|yes| F[_async_run_sync_impl] | ||
| E -->|no| G[asyncio.to_thread run_sync] | ||
| end | ||
| ``` | ||
|
|
||
|
|
||
|
|
||
| After the change: only the top/left path (async clients + `_async_run_sync_impl`). No `to_thread`, no sync fallback. | ||
|
|
||
| ## 1. Require AsyncERClient in [er2er_main.py](cdip-integrations/er2er/er2er_main.py) | ||
|
|
||
| - **Remove** the `else` branch (lines 91–99) that calls `asyncio.to_thread(build_er_clients, ...)`. | ||
| - **Require async:** When `ASYNC_ER_CLIENT_AVAILABLE` is false, raise a clear error before building clients (e.g. `ER2ERSync_ConfigError` or `RuntimeError`) with a message that AsyncERClient is required (e.g. "er-client async support is required; AsyncERClient could not be imported"). | ||
| - **Remove** the `build_er_clients` import from `er2er_syncher`. | ||
| - **Keep** the `if ASYNC_ER_CLIENT_AVAILABLE:` block but change it to: always call `build_async_er_clients`; if the flag is false, raise instead of falling back. | ||
|
|
||
| Result: connector only builds async clients and fails fast if async is unavailable. | ||
|
|
||
| ## 2. Require AsyncERClient in [er2er_syncher.py](cdip-integrations/er2er/er2er_syncher.py) `async_run_sync` | ||
|
|
||
| - **Remove** the fallback that runs `run_sync` via `asyncio.to_thread` (lines 1500–1508). |
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
delete_event_type(value, version)method to bothERClient(sync) andAsyncERClient(async), targeting the v2.0activity/eventtypes/{slug}endpoint by default.event_type_delete_path()helper toapi_paths.pyfor centralized path resolution.base_urlparameter to both sync and async_delete()methods to support versioned endpoints, plus 409 Conflict handling for the server's dependency-check response.Motivation
The das server's
EventTypesViewSetsupports adestroy()action (v2.0) that returns 204 on success or 409 Conflict when the event type has associated events or alert rules. The er-client was missing this capability entirely.Jira: ERA-12656
Test plan
event_type_delete_path()helperNote
This branch builds on top of the API versioning infrastructure from PR #23 (
cd/support-api-versions), which was merged locally. Once that PR lands inmain, this PR should merge cleanly.