-
-
Notifications
You must be signed in to change notification settings - Fork 58
feat(client): return created Event(s) from insert_event/insert_events #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -177,15 +177,19 @@ def get_events( | |
| events = self._get(endpoint, params=params).json() | ||
| return [Event(**event) for event in events] | ||
|
|
||
| def insert_event(self, bucket_id: str, event: Event) -> None: | ||
| def insert_event(self, bucket_id: str, event: Event) -> Event: | ||
| endpoint = f"buckets/{bucket_id}/events" | ||
| data = [event.to_json_dict()] | ||
| self._post(endpoint, data) | ||
| response = self._post(endpoint, data) | ||
| created = response.json() | ||
| return Event(**created[0]) if created else event | ||
|
|
||
| def insert_events(self, bucket_id: str, events: List[Event]) -> None: | ||
| def insert_events(self, bucket_id: str, events: List[Event]) -> List[Event]: | ||
| endpoint = f"buckets/{bucket_id}/events" | ||
| data = [event.to_json_dict() for event in events] | ||
| self._post(endpoint, data) | ||
| response = self._post(endpoint, data) | ||
| created = response.json() | ||
| return [Event(**e) for e in created] | ||
|
Comment on lines
+187
to
+192
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the server returns fewer events than were passed in (e.g., partial insertion, empty response), the function silently returns a shorter list — including Consider validating that the returned count matches the input count: def insert_events(self, bucket_id: str, events: List[Event]) -> List[Event]:
endpoint = f"buckets/{bucket_id}/events"
data = [event.to_json_dict() for event in events]
response = self._post(endpoint, data)
created = response.json()
if len(created) != len(events):
raise Exception(
f"Server returned {len(created)} events, expected {len(events)}"
)
return [Event(**e) for e in created] |
||
|
|
||
| def delete_event(self, bucket_id: str, event_id: int) -> None: | ||
| endpoint = f"buckets/{bucket_id}/events/{event_id}" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent fallback returns event without server-assigned ID
When the server returns an empty list,
insert_eventsilently falls back to returning the originaleventobject (which hasid=Noneif not pre-assigned). This breaks the stated contract of the function — callers expect anEventwith a server-assigned ID, but they silently receive the input event instead.Any caller that does
client.delete_event(bucket_id, client.insert_event(bucket_id, e).id)would then calldelete_event(bucket_id, None), causing a confusing error downstream rather than a clear failure at the insertion step.The
insert_eventssibling doesn't have this fallback at all — it just returns[]on an empty response — creating an inconsistency in error handling between the two methods.A more defensive approach would be to raise a clear error if the server unexpectedly returns an empty list, rather than silently swallowing the failure: