TPT-4428: Change assignments type from dict to list in ip_addresses_assign#687
Conversation
mawilk90
requested review from
psnoch-akamai and
yec-akamai
and removed request for
a team,
psnoch-akamai and
yec-akamai
There was a problem hiding this comment.
Pull request overview
Updates the NetworkingGroup.ip_addresses_assign API to send assignments as an array/list (matching the Linode API contract) instead of a dict wrapper, and aligns unit tests accordingly.
Changes:
- Update
ip_addresses_assignto iterate over and submit a list of assignment objects. - Update unit test to pass and assert list-shaped
assignments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for a in assignments: | ||
| if not "address" in a or not "linode_id" in a: |
There was a problem hiding this comment.
Changing ip_addresses_assign to accept a bare list is a breaking change for any callers that were previously passing {"assignments": [...]}. Consider supporting both shapes for backward compatibility (e.g., if assignments is a dict with an assignments key, unwrap it) or at least raise a clearer error when a dict is provided (current loop will iterate dict keys and raise a generic Invalid assignment).
| for a in assignments: | |
| if not "address" in a or not "linode_id" in a: | |
| if isinstance(assignments, dict): | |
| if "assignments" in assignments: | |
| assignments = assignments["assignments"] | |
| else: | |
| raise ValueError( | |
| "Invalid assignments payload: expected a list of assignments " | |
| "or a dict containing an 'assignments' key, got {}".format( | |
| assignments | |
| ) | |
| ) | |
| if not isinstance(assignments, list): | |
| raise ValueError( | |
| "Invalid assignments payload: expected a list of assignments, got {}".format( | |
| type(assignments).__name__ | |
| ) | |
| ) | |
| for a in assignments: | |
| if not isinstance(a, dict) or "address" not in a or "linode_id" not in a: |
| :param assignments: Any number of assignments to make. See | ||
| :any:`IPAddress.to` for details on how to construct | ||
| assignments. |
There was a problem hiding this comment.
The docstring still says "Any number of assignments to make" but the method now takes a single list of assignment objects. Please update the wording to reflect the actual expected argument shape to avoid confusing API consumers.
📝 Description
Current type of "assignments" passed into "ip_addresses_assign" method is dict what causes an API error during IP assignment as it requires array/list.
https://techdocs.akamai.com/linode-api/reference/post-assign-ips
✔️ How to Test
make test-unit