fix: treat server_group_ids=None as not supplied instead of iterating it - #577
Open
hackerboey wants to merge 1 commit into
Open
hackerboey wants to merge 1 commit into
hackerboey wants to merge 1 commit into
Conversation
The ZPA application segment resources reformatted server_group_ids into
the wire-shaped serverGroups list behind a key-presence test:
if "server_group_ids" in body:
body["serverGroups"] = [{"id": gid} for gid in body.pop("server_group_ids")]
Presence is not the same as usability. Forwarding an optional keyword
argument whose default is None — the natural shape of a partial update
that does not intend to change server groups — put the key in body with
a None value, so the comprehension raised
TypeError: 'NoneType' object is not iterable
inside the SDK before any request was built. There was no way to perform
such a partial update at all: omitting the argument entirely required the
caller to branch on it and build kwargs conditionally.
Each site now pops the value first and only reformats when it is not
None, so None means "not supplied" and the snake_case key never leaks
into the request body. Callers passing a real list are unaffected, and an
explicitly supplied empty list is still sent as "serverGroups": [] —
these resources deliberately omit serverGroups when the caller does not
mention it, which makes [] the only way to clear the server groups on an
update, so it must keep its meaning.
Fixed at all eleven sites of the pattern, in both the create and update
paths of each resource:
- application_segment.py — add_segment, update_segment,
add_segment_provision
- app_segments_ba_v2.py — add_segment_ba, update_segment_ba
- app_segments_ba.py — add_segment_ba, update_segment_ba
- app_segments_inspection.py — add_segment_inspection,
update_segment_inspection
- app_segments_pra.py — add_segment_pra, update_segment_pra
tests/unit/zpa/test_app_segment_server_group_ids.py parametrizes all
eleven entry points over the three inputs: None is dropped, a real list
still produces serverGroups exactly as before, and [] is still sent. The
None cases are the eleven that failed beforehand; the other twenty-two
assertions pass both before and after and pin the unchanged behavior.
This branch has not been deployed
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
1 participant
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.
Fixes #575
Every ZPA application segment resource reshapes
server_group_idsinto the wire-shapedserverGroupslist behind a key-presence test:The test is for presence, not for a usable value. A caller who forwards an optional keyword
whose default is
None— the natural result of wrapping these methods — getsraised inside the SDK before any request is built. So a partial update that does not intend to
change server groups cannot be performed.
Scope
The pattern appears in five modules, so fixing one would leave the API inconsistent:
zscaler/zpa/application_segment.pyzscaler/zpa/app_segments_ba.pyzscaler/zpa/app_segments_ba_v2.pyzscaler/zpa/app_segments_pra.pyzscaler/zpa/app_segments_inspection.pyNoneis now treated as "not supplied". An explicitly-passed empty list is preserved asserverGroups: [], since that is a meaningful instruction to detach all server groups and isdistinguishable from omission — please tell me if you would rather it were normalised.
Test
Adds
tests/unit/test_zpa_app_segment_server_group_ids.py, matching the flattests/unit/convention. It asserts, per module, that
Noneno longer raises and that passing a real liststill produces
serverGroupsexactly as before. The request executor is mocked — no credentialsor network access required.
33 passed on this branch. Reverting the source makes the
Nonecases fail with theTypeErrorabove.ruffandblack --check --line-length 127clean.