Skip to content

Add single-item convenience wrappers for bulk partial-success APIs#211

Open
fredvisser wants to merge 7 commits into
masterfrom
feature/python-modernization-phase2
Open

Add single-item convenience wrappers for bulk partial-success APIs#211
fredvisser wants to merge 7 commits into
masterfrom
feature/python-modernization-phase2

Conversation

@fredvisser
Copy link
Copy Markdown
Contributor

Summary

Several SystemLink client APIs only expose bulk operations that return partial-success envelopes (items + failed + error). When callers need to create or update a single item — by far the most common case — they must wrap the call in a list, then manually unpack the response and check for partial failures. This PR eliminates that boilerplate by adding singular convenience methods to six client classes.

What changed

Client New methods
TestMonitorClient create_result, create_step, update_result, update_step
ProductClient create_product, update_product
SpecClient create_spec, update_spec
TestPlanClient create_test_plan, update_test_plan, schedule_test_plan, create_test_plan_template
WorkItemClient create_work_item, update_work_item, schedule_work_item, create_work_item_template, update_work_item_template
AssetManagementClient create_asset

A shared helper unwrap_single_item_partial_success() (in nisystemlink/clients/core/helpers/_partial_success.py) centralizes the unwrap-or-raise logic so each wrapper stays minimal.

Why this is beneficial

  1. Reduces caller boilerplate — common single-item operations go from ~6 lines to 1.
  2. Safer error handling — partial-success failures are translated into ApiException with full error context, rather than silently returning empty lists.
  3. Consistent pattern — all wrappers follow the same contract (raise on failure, return typed item on success), making the API surface predictable.
  4. Non-breaking — the existing bulk methods remain unchanged; the new methods are purely additive.

Testing

  • 24 new unit tests covering the shared helper and all 18 convenience methods.
  • Full CI validation passes: poe check (formatting), poe lint, poe types (mypy, 379 source files), poe test (410 tests passed).

Introduce singular convenience methods (create_result, create_step,
update_result, update_step, create_product, update_product,
create_spec, update_spec, create_test_plan, update_test_plan,
schedule_test_plan, create_test_plan_template, create_asset,
create_work_item, update_work_item, schedule_work_item,
create_work_item_template, update_work_item_template) on their
respective client classes.

Each method wraps the existing bulk API with a one-element list, then
uses the shared unwrap_single_item_partial_success() helper to either
return the created/updated item or raise ApiException with the server
error details.

This eliminates repetitive boilerplate for the common single-item use
case while keeping the bulk methods available for batch operations.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds single-item convenience wrappers around existing bulk “partial-success” APIs across multiple SystemLink client classes, centralizing the unwrap-or-raise behavior in a shared helper to reduce caller boilerplate and standardize error handling.

Changes:

  • Added unwrap_single_item_partial_success() helper to consistently return the single created/updated item or raise ApiException with structured response context.
  • Introduced single-item convenience methods across TestMonitor, Product, Spec, TestPlan, WorkItem, and AssetManagement clients.
  • Added unit tests covering the helper and the new convenience methods.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
nisystemlink/clients/core/helpers/_partial_success.py Adds shared helper to unwrap single-item results from partial-success envelopes.
nisystemlink/clients/testmonitor/_test_monitor_client.py Adds create_result/create_step/update_result/update_step wrappers using the helper.
nisystemlink/clients/product/_product_client.py Adds create_product/update_product wrappers using the helper.
nisystemlink/clients/spec/_spec_client.py Adds create_spec/update_spec wrappers using the helper (including None response handling).
nisystemlink/clients/test_plan/_test_plan_client.py Adds create_test_plan/update_test_plan/schedule_test_plan/create_test_plan_template wrappers using the helper.
nisystemlink/clients/work_item/_work_item_client.py Adds create_work_item/update_work_item/schedule_work_item/create_work_item_template/update_work_item_template wrappers using the helper.
nisystemlink/clients/assetmanagement/_asset_management_client.py Adds create_asset wrapper using the helper.
tests/core/test_partial_success.py Adds tests for the shared partial-success unwrap helper.
tests/test_single_item_client_convenience_methods.py Adds tests verifying wrappers delegate to bulk APIs and return the unwrapped item.
tests/testmonitor/test_testmonitor_client.py Adds focused tests for TestMonitorClient.create_result success and failure behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread nisystemlink/clients/core/helpers/_partial_success.py
@fredvisser
Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in 4ca9b25. The shared single-item partial-success helper now raises ApiException when the service returns anything other than exactly one successful item, and I added a regression test for the multi-item case. Focused validation: pytest tests/core/test_partial_success.py.

@fredvisser fredvisser marked this pull request as ready for review June 2, 2026 13:49
if failed or error:
raise core.ApiException(
failure_message,
error=error,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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 (Skyline.OneOrMoreErrorsOccurred) or code (-251041) the check inner_errors for length one.

"""
...

def create_asset(self, asset: models.CreateAssetRequest) -> models.Asset:
Copy link
Copy Markdown
Collaborator

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.

Copy link
Copy Markdown
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants