-
Notifications
You must be signed in to change notification settings - Fork 30
refactor(set): refactor the update flow to construct PATCH request bodies correctly #111
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
Conversation
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.
Pull request overview
This PR refactors the set command implementation to properly align with REST PATCH semantics by sending only the updated properties rather than the entire resource payload. Previously, the CLI would fetch the full resource via GET, modify a single field, and send the entire object back via PATCH, which doesn't align with PATCH semantics and could cause validation errors.
Key changes:
- Extracted
update_item_definitionfromupdate_fabric_elementfor item-specific base64 handling - Added
extract_updated_onlyparameter to control payload extraction behavior - Introduced
update_cachehelper to reduce code duplication - Updated tests to reflect new payload structure and cache behavior
Reviewed changes
Copilot reviewed 41 out of 45 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/fabric_cli/utils/fab_cmd_set_utils.py |
Core refactoring: split item definition handling, added extract_updated_only flag, new update_cache helper |
src/fabric_cli/errors/common.py |
Added gateway-specific error messages for unsupported operations |
tests/test_utils/test_fab_cmd_set_utils.py |
Added test for update_item_definition, updated assertions for extracted properties |
tests/test_commands/test_set.py |
Updated cache assertion logic to only expect cache updates for displayName or name changes |
tests/test_commands/recordings/**/*.yaml |
Re-recorded VCR tests showing minimal PATCH payloads instead of full resource objects |
tests/test_commands/api_processors/capacities_api_processor.py |
Added defensive checks for optional fields in request/response handling |
.changes/unreleased/optimization-20260101-115402.yaml |
Changelog entry for the optimization |
Summary
Public Fabric APIs for updating resources (capacities, workspaces, items, etc.) are exposed as REST PATCH operations and expect request bodies to include only the fields being changed.
Our CLI set command previously fetched the full resource payload via GET, modified a single field (for example, displayName), and sent the entire object back to the update endpoint. This behavior does not align with PATCH semantics and can result in unexpected behavior or validation errors.
This PR refactors the update flow to construct PATCH request bodies correctly by extracting only the updated properties from the GET response.
Changes
fab_cmd_set_utils:
fab_fs_set_*
Removed data.pop(...) calls. Since the payload now includes only the target path being updated, fields like id are no longer present and do not need to be stripped.
fab_fs_set_connection
Added connectivityType to the payload, as required by the Update Gateway API documentation.
fab_fs_set_gateway
Added the required type field, per the [Update Gateway API documentation.]((https://learn.microsoft.com/en-us/rest/api/fabric/core/connections/update-gateway).
Tests
Because this change affects how request payloads are constructed for update APIs, all set command tests were re-recorded to reflect the new behavior.