ERA-12654: add get_event() single-event detail to sync and async clients#26
Open
JoshuaVulcan wants to merge 6 commits intomainfrom
Open
ERA-12654: add get_event() single-event detail to sync and async clients#26JoshuaVulcan wants to merge 6 commits intomainfrom
JoshuaVulcan wants to merge 6 commits intomainfrom
Conversation
Add get_event(event_id) method to both ERClient (sync) and AsyncERClient
that hits the activity/event/{id} GET endpoint. Supports query params:
include_details, include_updates, include_notes, include_related_events,
and include_files.
Includes comprehensive test coverage:
- 10 sync tests (basic retrieval, default params, all includes, notes,
404/401/403 error handling, URL construction)
- 7 async tests mirroring the same scenarios using respx mocking
Also establishes tests/sync_client/ test directory with conftest fixtures.
Co-authored-by: Cursor <cursoragent@cursor.com>
JoshuaVulcan
added
autoreviewing
- get_event(*, event_id, ...) in sync and async clients - Update all test call sites to use event_id= keyword - Aligns with style guide (PR #23) for merge compatibility Co-authored-by: Cursor <cursoragent@cursor.com>
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 single-event retrieval support (get_event(event_id, ...)) to both the sync and async ER clients, along with new test coverage to ensure parity and correct query param behavior.
Changes:
- Add
ERClient.get_event(...)to retrieve a single event viaGET activity/event/{id}with include-* query params. - Add
AsyncERClient.get_event(...)with matching behavior for async usage. - Add/expand sync + async tests covering success, include flags, and error handling (401/403/404).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
erclient/client.py |
Introduces/relocates get_event() for both sync and async clients, wiring include flags into _get(...). |
tests/sync_client/test_get_event.py |
Reworks sync get_event tests to cover defaults, include flags, and common error responses. |
tests/async_client/test_get_event.py |
Adds async get_event tests using respx mocking for success and error scenarios. |
💡 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
+141
to
+143
| async with respx.mock( | ||
| base_url=er_client.service_root, assert_all_called=False | ||
| ) as respx_mock: |
Comment on lines
+162
to
+164
| async with respx.mock( | ||
| base_url=er_client.service_root, assert_all_called=False | ||
| ) as respx_mock: |
| @@ -4,69 +4,175 @@ | |||
| import pytest | |||
|
|
|||
| from erclient.client import ERClient | |||
Comment on lines
+11
to
+16
| async with respx.mock( | ||
| base_url=er_client.service_root, assert_all_called=False | ||
| ) as respx_mock: | ||
| route = respx_mock.get(f'activity/event/{event_id}') | ||
| route.return_value = httpx.Response( | ||
| httpx.codes.OK, |
Comment on lines
+34
to
+36
| async with respx.mock( | ||
| base_url=er_client.service_root, assert_all_called=False | ||
| ) as respx_mock: |
Comment on lines
+69
to
+71
| async with respx.mock( | ||
| base_url=er_client.service_root, assert_all_called=False | ||
| ) as respx_mock: |
Comment on lines
+95
to
+97
| async with respx.mock( | ||
| base_url=er_client.service_root, assert_all_called=False | ||
| ) as respx_mock: |
Comment on lines
+118
to
+120
| async with respx.mock( | ||
| base_url=er_client.service_root, assert_all_called=False | ||
| ) as respx_mock: |
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
get_event(event_id)method to ERClient (sync) for retrieving a single event by ID viaactivity/event/{id}GET endpointget_event(event_id)to AsyncERClient for sync/async parityinclude_details,include_updates,include_notes,include_related_events,include_filestests/sync_client/directory with conftest fixtures for future sync client testsJira
ERA-12654
Test Plan
respxmockingMerge order
Merge after PR #23 (API versioning support) is merged. This PR aligns with PR #23's
get_event(*, event_id, ...)signature; merging #23 first avoids conflicts and keeps versioned URL behavior consistent.