Skip to content

[ISSUE] get_service_edge returns a bare ServiceEdge instead of the (object, response, error) tuple every sibling getter returns #578

Description

@hackerboey

[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.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions