From 7ae82d40e339b5a8aee81494cf764b1f19306d3e Mon Sep 17 00:00:00 2001 From: Hugo Durand Date: Tue, 22 Sep 2026 17:16:36 -0400 Subject: [PATCH] fix(cli): look up an explicit listener by id instead of searching a page --- libs/cli/langgraph_cli/deploy.py | 57 +++++---- libs/cli/langgraph_cli/host_backend.py | 3 + .../unit_tests/cli/test_deploy_command.py | 114 +++++++++++++++++- .../tests/unit_tests/test_deploy_helpers.py | 85 ++++++++----- 4 files changed, 198 insertions(+), 61 deletions(-) diff --git a/libs/cli/langgraph_cli/deploy.py b/libs/cli/langgraph_cli/deploy.py index 7252864c1..5223b4088 100644 --- a/libs/cli/langgraph_cli/deploy.py +++ b/libs/cli/langgraph_cli/deploy.py @@ -105,6 +105,10 @@ _OPERATOR_DEFAULT_RESOURCE_SPEC: Mapping[str, object] = {} _CUSTOMER_REGISTRY_SOURCE: SourceName = "external_docker" _LISTENER_REQUIRED_MARKER = "listener_id' is required" _LISTENERS_SHOWN = 10 +_NO_LISTENERS = ( + "This workspace has no listeners, so --listener-id and --k8s-namespace " + "do not apply." +) _TERMINAL_STATUSES = frozenset( @@ -234,32 +238,20 @@ class RequestedPlacement: "--name." ) - def resolve(self, listeners: Sequence[Listener]) -> Placement: - if not listeners: - if self.requested: - raise click.UsageError( - "This workspace has no listeners, so --listener-id and " - "--k8s-namespace do not apply." - ) - return Unplaced() - listener = self._listener(listeners) + def on(self, listener: Listener) -> Placement: return OnListener(listener.id, self._namespace(listener)) - def _listener(self, listeners: Sequence[Listener]) -> Listener: - if self.listener_id is None: - if len(listeners) == 1: - return listeners[0] + def among(self, listeners: Sequence[Listener]) -> Placement: + if not listeners: + if self.requested: + raise click.UsageError(_NO_LISTENERS) + return Unplaced() + if len(listeners) > 1: raise click.UsageError( "This workspace has several listeners. Choose one with " f"--listener-id:\n{_describe_listeners(listeners)}" ) - for listener in listeners: - if listener.id == self.listener_id: - return listener - raise click.UsageError( - f"Listener {self.listener_id} was not found in this workspace. " - f"Available listeners:\n{_describe_listeners(listeners)}" - ) + return self.on(listeners[0]) def _namespace(self, listener: Listener) -> str: if not listener.namespaces: @@ -1514,6 +1506,24 @@ def _needs_a_listener(err: HostBackendError) -> bool: ) +def _requested_listener(client: HostBackendClient, listener_id: str) -> Listener: + try: + resource = _call_host_backend_with_optional_tenant( + client, lambda c: c.get_listener(listener_id) + ) + except HostBackendError as err: + if err.status_code != 404: + raise + available = _available_listeners(client) + if not available: + raise click.UsageError(_NO_LISTENERS) from None + raise click.UsageError( + f"Listener {listener_id} was not found in this workspace. " + f"Available listeners:\n{_describe_listeners(available)}" + ) from None + return Listener.from_resource(resource) + + def _available_listeners(client: HostBackendClient) -> tuple[Listener, ...]: resources = _call_host_backend_with_optional_tenant( client, lambda c: c.list_listeners() @@ -1620,9 +1630,12 @@ class CustomerRegistrySource: ) def _resolve_placement(self, ctx: DeployContext) -> Placement: - if not (ctx.endpoints.is_cloud or self.requested_placement.requested): + requested = self.requested_placement + if requested.listener_id is not None: + return requested.on(_requested_listener(ctx.client, requested.listener_id)) + if not (ctx.endpoints.is_cloud or requested.requested): return Unplaced() - return self.requested_placement.resolve(_available_listeners(ctx.client)) + return requested.among(_available_listeners(ctx.client)) def _announce(self, placement: Placement) -> None: if isinstance(placement, OnListener): diff --git a/libs/cli/langgraph_cli/host_backend.py b/libs/cli/langgraph_cli/host_backend.py index 007d48a7a..4f2afe42a 100644 --- a/libs/cli/langgraph_cli/host_backend.py +++ b/libs/cli/langgraph_cli/host_backend.py @@ -234,6 +234,9 @@ class HostBackendClient: params = {key: value for key, value in given if value is not None} return _resources(self._request("GET", "/v2/deployments", params=params)) + def get_listener(self, listener_id: str) -> dict[str, Any]: + return self._request("GET", f"/v2/listeners/{listener_id}") + def list_listeners(self) -> list[dict[str, Any]]: return _resources( self._request("GET", "/v2/listeners", params={"limit": MAX_PAGE_SIZE}) diff --git a/libs/cli/tests/unit_tests/cli/test_deploy_command.py b/libs/cli/tests/unit_tests/cli/test_deploy_command.py index ffd423474..5fa6770c3 100644 --- a/libs/cli/tests/unit_tests/cli/test_deploy_command.py +++ b/libs/cli/tests/unit_tests/cli/test_deploy_command.py @@ -78,6 +78,7 @@ class ControlPlaneDouble: push_token_status: int = 200 create_error: str | None = None listeners: list[dict] = field(default_factory=list) + listeners_by_id: dict[str, dict] = field(default_factory=dict) bodies: dict[str, dict] = field(default_factory=dict) def handle(self, request: httpx.Request) -> httpx.Response: @@ -91,6 +92,15 @@ class ControlPlaneDouble: method, path = request.method, request.url.path if (method, path) == ("GET", "/v2/listeners"): return httpx.Response(200, json={"resources": self.listeners}) + if method == "GET" and path.startswith("/v2/listeners/"): + listener_id = path.rsplit("/", 1)[-1] + known = {listener["id"]: listener for listener in self.listeners} + known.update(self.listeners_by_id) + if listener_id not in known: + return httpx.Response( + 404, json={"detail": f"Listener ID {listener_id} not found."} + ) + return httpx.Response(200, json=known[listener_id]) if (method, path) == ("GET", "/v2/deployments"): name = request.url.params.get("name") return httpx.Response( @@ -727,12 +737,6 @@ def test_push_to_places_a_new_deployment_on_the_chosen_listener( pytest.param( [TWO_NAMESPACE_LISTENER], (), "--k8s-namespace", id="two_namespaces" ), - pytest.param( - [LISTENER], - ("--listener-id", "listener-9"), - "was not found", - id="unknown_listener", - ), pytest.param( [LISTENER], ("--k8s-namespace", "nope"), @@ -951,3 +955,101 @@ def test_a_listener_without_an_id_is_reported_rather_than_ignored( assert result.exit_code != 0 assert "without an id" in result.output assert deploy_project.docker.verbs() == [] + + +def _listener_route(listener_id: str) -> str: + return f"GET /v2/listeners/{listener_id}" + + +def test_an_explicit_listener_is_fetched_by_id_not_searched( + deploy_project: DeployProject, +) -> None: + deploy_project.control_plane.listeners = [LISTENER, OTHER_LISTENER] + + result = deploy_project.run( + "--push-to", + PUSH_REPOSITORY, + "--listener-id", + "listener-2", + host_url=CLOUD_CONTROL_PLANE_URL, + ) + + assert result.exit_code == 0, result.output + assert _listener_route("listener-2") in deploy_project.timeline + assert LIST_LISTENERS not in deploy_project.timeline + assert deploy_project.control_plane.bodies[CREATE_DEPLOYMENT]["source_config"] == { + "resource_spec": {}, + "listener_id": "listener-2", + "listener_config": {"k8s_namespace": "agents"}, + } + + +def test_an_explicit_listener_beyond_the_first_page_still_works( + deploy_project: DeployProject, +) -> None: + deploy_project.control_plane.listeners = [ + { + "id": f"listener-{index}", + "compute_id": "cluster", + "compute_config": {"k8s_namespaces": ["agents"]}, + } + for index in range(100) + ] + deploy_project.control_plane.listeners_by_id = { + "listener-on-page-two": { + "id": "listener-on-page-two", + "compute_id": "far-cluster", + "compute_config": {"k8s_namespaces": ["agents"]}, + } + } + + result = deploy_project.run( + "--push-to", + PUSH_REPOSITORY, + "--listener-id", + "listener-on-page-two", + host_url=CLOUD_CONTROL_PLANE_URL, + ) + + assert result.exit_code == 0, result.output + assert deploy_project.control_plane.bodies[CREATE_DEPLOYMENT]["source_config"] == { + "resource_spec": {}, + "listener_id": "listener-on-page-two", + "listener_config": {"k8s_namespace": "agents"}, + } + + +def test_an_unknown_listener_names_the_ones_that_exist( + deploy_project: DeployProject, +) -> None: + deploy_project.control_plane.listeners = [LISTENER] + + result = deploy_project.run( + "--push-to", + PUSH_REPOSITORY, + "--listener-id", + "listener-9", + host_url=CLOUD_CONTROL_PLANE_URL, + ) + + assert result.exit_code != 0 + assert "was not found" in result.output + assert "listener-1" in result.output + assert "prod-cluster" in result.output + assert deploy_project.docker.verbs() == [] + + +def test_an_explicit_listener_in_a_workspace_without_any_is_refused( + deploy_project: DeployProject, +) -> None: + result = deploy_project.run( + "--push-to", + PUSH_REPOSITORY, + "--listener-id", + "listener-1", + host_url=CLOUD_CONTROL_PLANE_URL, + ) + + assert result.exit_code != 0 + assert "no listeners" in result.output + assert deploy_project.docker.verbs() == [] diff --git a/libs/cli/tests/unit_tests/test_deploy_helpers.py b/libs/cli/tests/unit_tests/test_deploy_helpers.py index e2e522e75..02dce8679 100644 --- a/libs/cli/tests/unit_tests/test_deploy_helpers.py +++ b/libs/cli/tests/unit_tests/test_deploy_helpers.py @@ -993,22 +993,10 @@ class TestRequestedPlacement: OnListener("listener-2", "agents-staging"), id="namespace_alone_picks_the_only_listener", ), - pytest.param( - RequestedPlacement(listener_id="listener-1"), - (ONE_NAMESPACE, TWO_NAMESPACES), - OnListener("listener-1", "agents"), - id="listener_alone_picks_its_only_namespace", - ), - pytest.param( - RequestedPlacement(listener_id="listener-2", k8s_namespace="agents"), - (ONE_NAMESPACE, TWO_NAMESPACES), - OnListener("listener-2", "agents"), - id="both_given", - ), ], ) def test_resolves_to_a_placement(self, request_, listeners, expected): - assert request_.resolve(listeners) == expected + assert request_.among(listeners) == expected @pytest.mark.parametrize( ("request_", "listeners", "message"), @@ -1037,39 +1025,21 @@ class TestRequestedPlacement: "no listeners", id="namespace_without_any_listener", ), - pytest.param( - RequestedPlacement(listener_id="listener-9"), - (ONE_NAMESPACE,), - "was not found", - id="unknown_listener", - ), pytest.param( RequestedPlacement(), (TWO_NAMESPACES,), "--k8s-namespace", id="several_namespaces_need_a_choice", ), - pytest.param( - RequestedPlacement(listener_id="listener-2", k8s_namespace="nope"), - (TWO_NAMESPACES,), - "does not serve namespace", - id="unknown_namespace", - ), - pytest.param( - RequestedPlacement(listener_id="listener-3"), - (NO_NAMESPACE,), - "serves no namespaces", - id="listener_without_namespaces", - ), ], ) def test_refuses_and_names_the_choices(self, request_, listeners, message): with pytest.raises(click.UsageError, match=message): - request_.resolve(listeners) + request_.among(listeners) def test_the_error_lists_every_listener_with_its_cluster_and_namespaces(self): with pytest.raises(click.UsageError) as error: - RequestedPlacement().resolve((ONE_NAMESPACE, TWO_NAMESPACES)) + RequestedPlacement().among((ONE_NAMESPACE, TWO_NAMESPACES)) assert "listener-1" in error.value.message assert "prod-cluster" in error.value.message @@ -1200,3 +1170,52 @@ def test_a_deployment_id_with_listener_flags_is_refused_without_probing_docker( placement=RequestedPlacement(listener_id="listener-1"), selector=ById("dep-1"), ) + + +class TestPlacementOnAKnownListener: + @pytest.mark.parametrize( + ("request_", "listener", "expected"), + [ + pytest.param( + RequestedPlacement(listener_id="listener-1"), + ONE_NAMESPACE, + OnListener("listener-1", "agents"), + id="the_only_namespace_is_used", + ), + pytest.param( + RequestedPlacement(listener_id="listener-2", k8s_namespace="agents"), + TWO_NAMESPACES, + OnListener("listener-2", "agents"), + id="the_chosen_namespace_is_used", + ), + ], + ) + def test_places_on_the_listener(self, request_, listener, expected): + assert request_.on(listener) == expected + + @pytest.mark.parametrize( + ("request_", "listener", "message"), + [ + pytest.param( + RequestedPlacement(listener_id="listener-2"), + TWO_NAMESPACES, + "--k8s-namespace", + id="several_namespaces_need_a_choice", + ), + pytest.param( + RequestedPlacement(listener_id="listener-2", k8s_namespace="nope"), + TWO_NAMESPACES, + "does not serve namespace", + id="unknown_namespace", + ), + pytest.param( + RequestedPlacement(listener_id="listener-3"), + NO_NAMESPACE, + "serves no namespaces", + id="listener_without_namespaces", + ), + ], + ) + def test_refuses_and_names_the_namespaces(self, request_, listener, message): + with pytest.raises(click.UsageError, match=message): + request_.on(listener)