-
Notifications
You must be signed in to change notification settings - Fork 31
Add single-item convenience wrappers for bulk partial-success APIs #211
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
base: master
Are you sure you want to change the base?
Changes from all commits
998db97
cdbb233
eb20730
f331485
80afc27
b88f82a
4ca9b25
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 |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| from typing import Any, Sequence, TypeVar | ||
|
|
||
| from nisystemlink.clients import core | ||
|
|
||
| _ItemT = TypeVar("_ItemT") | ||
|
|
||
|
|
||
| def unwrap_single_item_partial_success( | ||
| *, | ||
| response: Any | None, | ||
| items: Sequence[_ItemT] | None, | ||
| failed: Sequence[Any] | None, | ||
| error: core.ApiError | None, | ||
| failure_message: str, | ||
| empty_message: str, | ||
| ) -> _ItemT: | ||
| """Return the first successful item from a partial-success response. | ||
|
|
||
| Raises: | ||
| ApiException: if the response reports a failure or contains no successful item. | ||
| """ | ||
| response_data = ( | ||
| response.model_dump(mode="json", by_alias=True) | ||
| if response is not None | ||
| else None | ||
| ) | ||
|
|
||
| if failed or error: | ||
| raise core.ApiException( | ||
| failure_message, | ||
| error=error, | ||
|
Collaborator
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. The error here in the case of partial success on a single item request is very likely to be a OneOrMoreErrorsOccurred error with a single InnerErrors error. That inner error is the actually useful part to a caller. What you have to leave it as a one or more isn't wrong and it would be fine to submit like you have it. A nice to have in this unwrapping would be to also unwrap this one or many into just one error, if there is really only one inner error. You can identify one of these by name ( |
||
| response_data=response_data, | ||
| ) | ||
|
|
||
| if not items: | ||
| raise core.ApiException( | ||
| empty_message, | ||
| response_data=response_data, | ||
| ) | ||
|
|
||
| if len(items) != 1: | ||
| raise core.ApiException( | ||
| f"Expected exactly one successful item but received {len(items)}.", | ||
| response_data=response_data, | ||
| ) | ||
|
|
||
| return items[0] | ||
|
fredvisser marked this conversation as resolved.
|
||
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.
Your PR description claims that single item create is expected to be the most common. If so we should update the examples to use the new single item convenience functions where applicable. Assets create example is a good case for updating.
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.
I would be fine with this as a follow up PR as well.