From 04a78aafadc121f70026cf87ad7e5a8cd8b0cc00 Mon Sep 17 00:00:00 2001 From: Hugo Durand Date: Tue, 22 Sep 2026 14:35:31 -0400 Subject: [PATCH] refactor(cli): return resource lists from the control plane client --- libs/cli/langgraph_cli/deploy.py | 46 ++++----------- libs/cli/langgraph_cli/host_backend.py | 33 ++++++++--- libs/cli/tests/unit_tests/cli/test_cli.py | 58 +++++++++---------- .../tests/unit_tests/test_deploy_helpers.py | 6 +- .../cli/tests/unit_tests/test_host_backend.py | 33 ++++++++++- 5 files changed, 95 insertions(+), 81 deletions(-) diff --git a/libs/cli/langgraph_cli/deploy.py b/libs/cli/langgraph_cli/deploy.py index 16439d549..70111116c 100644 --- a/libs/cli/langgraph_cli/deploy.py +++ b/libs/cli/langgraph_cli/deploy.py @@ -375,14 +375,8 @@ def _source_of(resource: object) -> str | None: def find_deployment_by_name( client: HostBackendClient, name: str ) -> ExistingDeployment | None: - listed = client.list_deployments(name_contains=name) - resources = listed.get("resources", []) if isinstance(listed, dict) else [] - for resource in resources: - if ( - isinstance(resource, dict) - and resource.get("name") == name - and resource.get("id") - ): + for resource in client.list_deployments(name_contains=name): + if resource.get("name") == name and resource.get("id"): return ExistingDeployment(str(resource["id"]), _source_of(resource)) return None @@ -750,14 +744,11 @@ def _poll_revision_status( ) -> tuple[str, str | None]: """Poll latest revision status until terminal status or timeout.""" em = _get_emitter() - revisions_resp = client.list_revisions(deployment_id, limit=1) - resources = ( - revisions_resp.get("resources", []) if isinstance(revisions_resp, dict) else [] - ) - if not resources: + revisions = client.list_revisions(deployment_id, limit=1) + if not revisions: return "", None - revision_id = str(resources[0]["id"]) + revision_id = str(revisions[0]["id"]) last_status = "" deadline = time.time() + timeout_seconds start_time = time.monotonic() @@ -2052,16 +2043,10 @@ def _deploy_cmd( @deploy.command("list", help="[Beta] List LangSmith Deployments.") def deploy_list(api_key: str | None, host_url: str | None, name_contains: str) -> None: client = _create_host_backend_client(host_url, api_key) - response = _call_host_backend_with_optional_tenant( + deployments = _call_host_backend_with_optional_tenant( client, lambda c: c.list_deployments(name_contains=name_contains), ) - resources = response.get("resources") if isinstance(response, dict) else None - deployments = ( - [item for item in resources if isinstance(item, dict)] - if isinstance(resources, list) - else [] - ) if not deployments: click.echo("No deployments found.") return @@ -2101,16 +2086,10 @@ def deploy_revisions_list( api_key: str | None, host_url: str | None, limit: int, deployment_id: str ) -> None: client = _create_host_backend_client(host_url, api_key) - response = _call_host_backend_with_optional_tenant( + revisions = _call_host_backend_with_optional_tenant( client, lambda c: c.list_revisions(deployment_id, limit=limit), ) - resources = response.get("resources") if isinstance(response, dict) else None - revisions = ( - [item for item in resources if isinstance(item, dict)] - if isinstance(resources, list) - else [] - ) if not revisions: click.echo(f"No revisions found for deployment {deployment_id}.") return @@ -2260,17 +2239,12 @@ def deploy_logs( dep_id = found.id if log_type == "build" and not revision_id: - revisions_resp = client.list_revisions(dep_id, limit=1) - resources = ( - revisions_resp.get("resources", []) - if isinstance(revisions_resp, dict) - else [] - ) - if not resources: + revisions = client.list_revisions(dep_id, limit=1) + if not revisions: raise click.ClickException( "No revisions found for this deployment. Cannot fetch build logs." ) - revision_id = str(resources[0]["id"]) + revision_id = str(revisions[0]["id"]) click.secho(f"Using latest revision: {revision_id}", fg="cyan") payload: dict = {"limit": limit, "order": "desc"} diff --git a/libs/cli/langgraph_cli/host_backend.py b/libs/cli/langgraph_cli/host_backend.py index ebfd3f9d0..77e13be41 100644 --- a/libs/cli/langgraph_cli/host_backend.py +++ b/libs/cli/langgraph_cli/host_backend.py @@ -83,6 +83,15 @@ def _without_api_path(path: str) -> str: return path +def _resources(payload: object) -> list[dict[str, Any]]: + if not isinstance(payload, dict): + return [] + resources = payload.get("resources") + if not isinstance(resources, list): + return [] + return [item for item in resources if isinstance(item, dict)] + + class HostBackendError(click.ClickException): """Raised when the host backend returns an error response.""" @@ -172,11 +181,13 @@ class HostBackendClient: payload["secrets"] = secrets return self._request("POST", "/v2/deployments", payload) - def list_deployments(self, name_contains: str = "") -> dict[str, Any]: - return self._request( - "GET", - "/v2/deployments", - params={"name_contains": name_contains}, + def list_deployments(self, name_contains: str = "") -> list[dict[str, Any]]: + return _resources( + self._request( + "GET", + "/v2/deployments", + params={"name_contains": name_contains}, + ) ) def get_deployment(self, deployment_id: str) -> dict[str, Any]: @@ -251,10 +262,14 @@ class HostBackendClient: payload["secrets"] = secrets return self._request("PATCH", f"/v2/deployments/{deployment_id}", payload) - def list_revisions(self, deployment_id: str, limit: int = 1) -> dict[str, Any]: - return self._request( - "GET", - f"/v2/deployments/{deployment_id}/revisions?limit={limit}", + def list_revisions( + self, deployment_id: str, limit: int = 1 + ) -> list[dict[str, Any]]: + return _resources( + self._request( + "GET", + f"/v2/deployments/{deployment_id}/revisions?limit={limit}", + ) ) def get_revision(self, deployment_id: str, revision_id: str) -> dict[str, Any]: diff --git a/libs/cli/tests/unit_tests/cli/test_cli.py b/libs/cli/tests/unit_tests/cli/test_cli.py index ca0ec7a04..8cce48db8 100644 --- a/libs/cli/tests/unit_tests/cli/test_cli.py +++ b/libs/cli/tests/unit_tests/cli/test_cli.py @@ -382,20 +382,18 @@ def test_deploy_list_command(monkeypatch) -> None: def list_deployments(self, name_contains: str = ""): captured["name_contains"] = name_contains - return { - "resources": [ - { - "id": "dep-123", - "name": "alpha", - "source_config": {"custom_url": "https://alpha.example.com"}, - }, - { - "id": "dep-456", - "name": "beta", - "source_config": {"custom_url": "https://beta.example.com"}, - }, - ] - } + return [ + { + "id": "dep-123", + "name": "alpha", + "source_config": {"custom_url": "https://alpha.example.com"}, + }, + { + "id": "dep-456", + "name": "beta", + "source_config": {"custom_url": "https://beta.example.com"}, + }, + ] monkeypatch.setattr(deploy_module, "HostBackendClient", FakeClient) @@ -435,7 +433,7 @@ def test_deploy_list_command_no_results(monkeypatch) -> None: pass def list_deployments(self, name_contains: str = ""): - return {"resources": []} + return [] monkeypatch.setattr(deploy_module, "HostBackendClient", FakeClient) @@ -468,20 +466,18 @@ def test_deploy_revisions_list_command(monkeypatch) -> None: def list_revisions(self, deployment_id: str, limit: int = 1): captured["deployment_id"] = deployment_id captured["limit"] = str(limit) - return { - "resources": [ - { - "id": "rev-123", - "status": "CREATING", - "created_at": "2023-11-07T05:31:56Z", - }, - { - "id": "rev-456", - "status": "DEPLOYED", - "created_at": "2023-11-08T10:00:00Z", - }, - ] - } + return [ + { + "id": "rev-123", + "status": "CREATING", + "created_at": "2023-11-07T05:31:56Z", + }, + { + "id": "rev-456", + "status": "DEPLOYED", + "created_at": "2023-11-08T10:00:00Z", + }, + ] monkeypatch.setattr(deploy_module, "HostBackendClient", FakeClient) @@ -522,7 +518,7 @@ def test_deploy_revisions_list_command_no_results(monkeypatch) -> None: pass def list_revisions(self, deployment_id: str, limit: int = 1): - return {"resources": []} + return [] monkeypatch.setattr(deploy_module, "HostBackendClient", FakeClient) @@ -555,7 +551,7 @@ def test_deploy_revisions_list_command_with_explicit_limit(monkeypatch) -> None: def list_revisions(self, deployment_id: str, limit: int = 1): captured["deployment_id"] = deployment_id captured["limit"] = str(limit) - return {"resources": []} + return [] monkeypatch.setattr(deploy_module, "HostBackendClient", FakeClient) diff --git a/libs/cli/tests/unit_tests/test_deploy_helpers.py b/libs/cli/tests/unit_tests/test_deploy_helpers.py index 62c34625b..8bf7cd673 100644 --- a/libs/cli/tests/unit_tests/test_deploy_helpers.py +++ b/libs/cli/tests/unit_tests/test_deploy_helpers.py @@ -280,11 +280,13 @@ class TestCallHostBackendWithOptionalTenant: return c def test_success_passes_through(self): - client = self._make_client(lambda req: httpx.Response(200, json={"ok": True})) + client = self._make_client( + lambda req: httpx.Response(200, json={"resources": [{"id": "dep-1"}]}) + ) result = _call_host_backend_with_optional_tenant( client, lambda c: c.list_deployments() ) - assert result == {"ok": True} + assert result == [{"id": "dep-1"}] def test_403_not_enabled_gives_actionable_error(self): detail = ( diff --git a/libs/cli/tests/unit_tests/test_host_backend.py b/libs/cli/tests/unit_tests/test_host_backend.py index 686b516c3..f3bd482a9 100644 --- a/libs/cli/tests/unit_tests/test_host_backend.py +++ b/libs/cli/tests/unit_tests/test_host_backend.py @@ -88,8 +88,7 @@ def test_list_deployments_sends_query_params(): c = HostBackendClient( "https://api.example.com", "test-key", transport=httpx.MockTransport(handler) ) - result = c.list_deployments("my app") - assert result == {"ok": True} + assert c.list_deployments("my app") == [] def _capturing_client(captured: dict) -> HostBackendClient: @@ -421,7 +420,7 @@ def test_injected_transport_receives_requests_under_the_prefixed_base_url(): transport=httpx.MockTransport(handler), ) - assert c.list_revisions("dep-1", limit=2) == {"ok": True} + assert c.list_revisions("dep-1", limit=2) == [] assert seen == { "url": "https://smith.example.com/api-host/v2/deployments/dep-1/revisions?limit=2", "api_key": "key", @@ -546,3 +545,31 @@ def test_control_plane_endpoints_resolve(host_url, langsmith_endpoint, expected) endpoints = ControlPlaneEndpoints.resolve(host_url, langsmith_endpoint) assert (endpoints.control_plane_url, endpoints.dashboard_url) == expected + + +@pytest.mark.parametrize( + ("payload", "expected"), + [ + pytest.param( + {"resources": [{"id": "a"}, {"id": "b"}]}, + [{"id": "a"}, {"id": "b"}], + id="list_returns_the_resources", + ), + pytest.param({"resources": []}, [], id="empty_list"), + pytest.param({}, [], id="missing_key"), + pytest.param({"resources": None}, [], id="null_resources"), + pytest.param( + {"resources": ["nope", {"id": "a"}]}, [{"id": "a"}], id="skips_non_objects" + ), + pytest.param([], [], id="unexpected_envelope"), + ], +) +def test_list_endpoints_return_resource_objects(payload, expected): + def handler(req: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=payload) + + c = HostBackendClient( + "https://api.example.com", "key", transport=httpx.MockTransport(handler) + ) + + assert c.list_deployments() == expected