[ISSUE] get_service_edge returns a bare ServiceEdge instead of the (object, response, error) tuple every sibling getter returns
Environment
|
|
| SDK version |
zscaler-sdk-python 1.9.44 (editable install of master @ e7f5f7ef) |
| Python |
3.11.15 |
| OS |
macOS (arm64) |
| Affected product / resource |
ZPA / service_edges |
Found by reading the SDK source and confirmed with the offline reproduction below (a stub
request executor, so no credentials and no network are involved). All identifiers used are
synthetic.
Description
ServiceEdgeControllerAPI.get_service_edge returns a bare model on success and a bare None
on failure:
# zscaler/zpa/service_edges.py:130-138
request, error = self._request_executor.create_request(http_method, api_url, params=params)
if error:
return None
response, error = self._request_executor.execute(request)
if error:
return None
return ServiceEdge(response.get_body())
Its declared return type is APIResult[dict], which zscaler/types.py:39 defines as
Tuple[T, ZscalerAPIResponse, Optional[Exception]], and its own docstring example unpacks
three values (zscaler/zpa/service_edges.py:116):
>>> fetched_service_edge, _, err = client.zpa.service_edges.get_service_edge('999999')
That example cannot run. Unpacking a ServiceEdge into three names hits
ZscalerObject.__getitem__ with integer indices, so it raises
TypeError: attribute name must be string, not 'int'; on the error path it raises
TypeError: cannot unpack non-iterable NoneType object. Either way the caller also loses the
response object and the error object entirely — there is no way to read a status code or an
error body from this method.
As far as I can tell this is the only method in the SDK shaped this way:
$ grep -rn 'return [A-Z][A-Za-z]*(response.get_body())' zscaler/
zscaler/zpa/service_edges.py:138: return ServiceEdge(response.get_body())
Sibling getters all return the triple, including update_service_edge in the very same class:
| Method |
Final return |
zscaler/zpa/service_edges.py:199 (update_service_edge) |
return (result, response, None) |
zscaler/zpa/service_edges.py:101 (list_service_edges) |
return (result, response, None) |
zscaler/zpa/app_connectors.py:147 (get_connector) |
return (result, response, None) |
zscaler/zpa/service_edge_group.py:146 (get_service_edge_group) |
return (result, response, None) |
zscaler/zpa/segment_groups.py:150 (get_group) |
return (result, response, None) |
zscaler/zpa/server_groups.py:148 (get_group) |
return (result, response, None) |
zscaler/zpa/servers.py:208 (get_server) |
return (result, response, None) |
zscaler/zpa/private_cloud_controller.py:147 (get_cloud_controller) |
return (result, response, None) |
This class was audited once before in #475, but that was about which endpoint attribute the
methods referenced; the return shape was not part of it.
Reproduction
Fully offline — no credentials, no network.
from zscaler.zpa.service_edges import ServiceEdgeControllerAPI
CONFIG = {"client": {"customerId": "72058304855015000"}}
class StubResponse:
def __init__(self, body):
self._body = body
def get_body(self):
return self._body
def get_status(self):
return 200
class StubExecutor:
"""Stands in for RequestExecutor."""
def create_request(self, method, endpoint, body=None, headers=None, params=None,
use_raw_data_for_body=False):
return {"method": method, "endpoint": endpoint.strip(),
"body": {} if body is None else body, "params": params or {}}, None
def execute(self, request, response_type=None, return_raw_response=False):
return StubResponse({"id": "72058304855099500", "name": "edge-a", "enabled": True}), None
edges = ServiceEdgeControllerAPI(StubExecutor(), CONFIG)
raw = edges.get_service_edge("72058304855099500")
print("get_service_edge(...) returned type:", type(raw).__name__)
try:
obj, resp, err = edges.get_service_edge("72058304855099500")
except TypeError as exc:
print("3-tuple unpack (the form in this method's own docstring) raises:")
print(" TypeError:", exc)
obj, resp, err = edges.update_service_edge("72058304855099500", name="edge-a")
print("update_service_edge in the same class unpacks fine ->",
type(obj).__name__, type(resp).__name__, err)
Expected behaviour
get_service_edge behaves like every other getter in the SDK — returns
(ServiceEdge | None, response, error) — so the documented obj, _, err = ... call form works
and errors are reachable.
Actual behaviour
get_service_edge(...) returned type: ServiceEdge
3-tuple unpack (the form in this method's own docstring) raises:
TypeError: attribute name must be string, not 'int'
update_service_edge in the same class unpacks fine -> ServiceEdge StubResponse None
Root cause
zscaler/zpa/service_edges.py:132, :136 (return None on the two error paths) and
zscaler/zpa/service_edges.py:138 (return ServiceEdge(response.get_body())).
This is a breaking change, so I'd rather ask than propose
Changing the return shape would break any caller currently doing
edge = client.zpa.service_edges.get_service_edge(...), so I don't want to assume how you'd
want it handled. Some options, without a preference:
- Change it to the triple and note it in
UPGRADE_GUIDE.md as a fix aligning one outlier with
the documented APIResult contract.
- Change it to the triple only on the next major, and in the meantime fix the docstring so it
matches the code.
- Keep the current shape and correct the type annotation and docstring instead.
Adjacent, and mentioning it only because you may want to decide the two together:
delete_service_edge (:236) and bulk_delete_service_edges (:266) return
response.get_status() — a bare int — while their docstrings also show _, _, err = ....
Happy to open a PR for whichever direction you choose.
[ISSUE] get_service_edge returns a bare ServiceEdge instead of the (object, response, error) tuple every sibling getter returns
Environment
zscaler-sdk-python1.9.44 (editable install ofmaster@e7f5f7ef)service_edgesFound by reading the SDK source and confirmed with the offline reproduction below (a stub
request executor, so no credentials and no network are involved). All identifiers used are
synthetic.
Description
ServiceEdgeControllerAPI.get_service_edgereturns a bare model on success and a bareNoneon failure:
Its declared return type is
APIResult[dict], whichzscaler/types.py:39defines asTuple[T, ZscalerAPIResponse, Optional[Exception]], and its own docstring example unpacksthree values (
zscaler/zpa/service_edges.py:116):That example cannot run. Unpacking a
ServiceEdgeinto three names hitsZscalerObject.__getitem__with integer indices, so it raisesTypeError: attribute name must be string, not 'int'; on the error path it raisesTypeError: cannot unpack non-iterable NoneType object. Either way the caller also loses theresponse object and the error object entirely — there is no way to read a status code or an
error body from this method.
As far as I can tell this is the only method in the SDK shaped this way:
Sibling getters all return the triple, including
update_service_edgein the very same class:zscaler/zpa/service_edges.py:199(update_service_edge)return (result, response, None)zscaler/zpa/service_edges.py:101(list_service_edges)return (result, response, None)zscaler/zpa/app_connectors.py:147(get_connector)return (result, response, None)zscaler/zpa/service_edge_group.py:146(get_service_edge_group)return (result, response, None)zscaler/zpa/segment_groups.py:150(get_group)return (result, response, None)zscaler/zpa/server_groups.py:148(get_group)return (result, response, None)zscaler/zpa/servers.py:208(get_server)return (result, response, None)zscaler/zpa/private_cloud_controller.py:147(get_cloud_controller)return (result, response, None)This class was audited once before in #475, but that was about which endpoint attribute the
methods referenced; the return shape was not part of it.
Reproduction
Fully offline — no credentials, no network.
Expected behaviour
get_service_edgebehaves like every other getter in the SDK — returns(ServiceEdge | None, response, error)— so the documentedobj, _, err = ...call form worksand errors are reachable.
Actual behaviour
Root cause
zscaler/zpa/service_edges.py:132,:136(return Noneon the two error paths) andzscaler/zpa/service_edges.py:138(return ServiceEdge(response.get_body())).This is a breaking change, so I'd rather ask than propose
Changing the return shape would break any caller currently doing
edge = client.zpa.service_edges.get_service_edge(...), so I don't want to assume how you'dwant it handled. Some options, without a preference:
UPGRADE_GUIDE.mdas a fix aligning one outlier withthe documented
APIResultcontract.matches the code.
Adjacent, and mentioning it only because you may want to decide the two together:
delete_service_edge(:236) andbulk_delete_service_edges(:266) returnresponse.get_status()— a bareint— while their docstrings also show_, _, err = ....Happy to open a PR for whichever direction you choose.