Open
Conversation
Sync ERClient: - Add post_message() for creating messages - Add get_message(id) for fetching a single message by ID - Fix get_messages() pagination bug (undefined variable 'p') - Accept **kwargs in get_messages() for query param pass-through Async AsyncERClient: - Add get_messages() as an async generator with automatic pagination - Add get_message(id) for fetching a single message by ID - Add delete_message(id) for deleting messages - Add _delete() convenience method to async client - Handle HTTP 204 No Content responses in async _call() Tests: - Add sync test suite (tests/sync_client/) with conftest and fixtures - Add comprehensive tests for all new sync methods (post, get, get_list, delete) - Add comprehensive tests for all new async methods (get_messages, get_message, delete_message) - Cover success, pagination, empty results, not-found, and forbidden cases Co-authored-by: Cursor <cursoragent@cursor.com>
JoshuaVulcan
added
autoreviewing
JoshuaVulcan
added a commit
that referenced
this pull request
Feb 11, 2026
…g to _call() - Async _delete() now delegates to _call(path=path, payload=None, method='DELETE', params=params) - Add 204 / empty-body handling in async _call() so DELETE responses don't trigger JSON parse - Aligns with PR review: use _call() delegation pattern like PRs #28, #29, #34; reduces merge conflict Co-authored-by: Cursor <cursoragent@cursor.com>
# Conflicts: # erclient/client.py # tests/sync_client/conftest.py
…; fix message test URLs and 204 assert 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 missing Messages endpoints to both sync and async ER clients and introduces new test coverage to ensure sync/async parity for messages operations.
Changes:
- Add sync
ERClient.post_message()andERClient.get_message(), and fixERClient.get_messages()pagination while allowing**kwargspassthrough. - Add async
AsyncERClient.get_messages()(async generator), plusget_message()anddelete_message(). - Add new sync and async tests for message create/list/get/delete scenarios (including pagination and error cases).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| erclient/client.py | Adds messages methods for sync/async clients and updates sync messages pagination behavior. |
| tests/sync_client/test_post_message.py | Sync tests for posting messages (success + error scenarios). |
| tests/sync_client/test_get_messages.py | Sync tests for listing messages (single page, pagination, empty, kwargs passthrough). |
| tests/sync_client/test_get_message.py | Sync tests for getting a single message (success + 404). |
| tests/sync_client/test_delete_message.py | Sync tests for deleting a message (success + error scenarios). |
| tests/async_client/test_get_messages.py | Async tests for listing messages via async generator (single page, pagination, empty). |
| tests/async_client/test_get_message.py | Async tests for getting a single message (success + 404). |
| tests/async_client/test_delete_message.py | Async tests for deleting a message (success + 404). |
💡 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
+73
to
+78
| call_kwargs = mock_get.call_args | ||
| assert call_kwargs is not None | ||
| assert "params" in call_kwargs.kwargs or ( | ||
| len(call_kwargs.args) > 1 | ||
| or (call_kwargs.kwargs.get("params", {}).get("page_size") == 50) | ||
| ) |
Comment on lines
+96
to
+99
| route = respx_mock.get("messages").return_value = httpx.Response( | ||
| httpx.codes.OK, | ||
| json={"data": get_messages_single_page_response}, | ||
| ) |
| """ | ||
| Delete a message by ID. | ||
| :param message_id: UUID of the message | ||
| :return: response data |
Comment on lines
+653
to
+655
| url, query_params = split_link(results['next']) | ||
| params['page'] = query_params['page'] | ||
| results = self._get(path='messages', params=params) |
| @@ -0,0 +1,78 @@ | |||
| import json | |||
| from unittest.mock import patch, MagicMock, call | |||
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
Achieves full sync/async parity for the messages interface in er-client (ERA-12660).
Sync
ERClientpost_message(message)— POST tomessagesendpointget_message(message_id)— GET single message by ID frommessages/{id}get_messages()pagination — the existing method had a bug referencing undefined variablep; now properly carries pagination params forward**kwargsinget_messages()for query param pass-through (e.g.page_size)Async
AsyncERClientget_messages(**kwargs)— async generator with automatic pagination via_get_data()get_message(message_id)— GET single message by IDdelete_message(message_id)— DELETE message by ID_delete()convenience method — enables DELETE operations in the async client_call()now returnsNonefor 204 responses instead of failing on empty JSON parseTests
tests/sync_client/test suite with conftest, fixtures, and testsMerge coordination
This PR adds
async _delete()and 204 handling in_call(). PRs #28, #32, and #34 add the same. Whichever merges first, the others should rebase to avoid duplicate/conflicting edits.tests/sync_client/conftest.pymay also conflict with other PRs; rebase as needed.Test plan
pytest tests/ -v)post_message()against a live ER instance (sync)get_messages()pagination works correctly with real paginated dataget_message(id)returns expected detail for a known messagedelete_message(id)successfully removes a message (async)