Repository: run-llama/llama_deploy

Issue:
Use `Client` in the CLI instead of manual http calls
The CLI is basically a frontend to the apiserver, so it could benefit from the `Client` introduced in #290, but currently we make http calls manually, which is prone to error and introduces duplicated code. Let's consolidate using `Client` across the board.

Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/70853997/badge)](https://coveralls.io/builds/70853997)

coverage: 72.018% (-0.1%) from 72.127%
when pulling **7e656c7212679725b8643103f64d36da9ad3339d on massi/336**
into **4449d9f4629009f39ee31ed89a074c283b32f85f on main**.


Patch:
diff --git a/llama_deploy/cli/__init__.py b/llama_deploy/cli/__init__.py
index 1affa74..b49e1c9 100644
--- a/llama_deploy/cli/__init__.py
+++ b/llama_deploy/cli/__init__.py
@@ -1,8 +1,8 @@
 import click
 
-from llama_deploy.cli.deploy import deploy
-from llama_deploy.cli.run import run
-from llama_deploy.cli.status import status
+from .deploy import deploy as deploy_cmd
+from .run import run as run_cmd
+from .status import status as status_cmd
 
 
 @click.group(
@@ -21,19 +21,17 @@ from llama_deploy.cli.status import status
 @click.option(
     "-t",
     "--timeout",
-    default=None,
+    default=120.0,
     type=float,
     help="Timeout on apiserver HTTP requests",
 )
 @click.pass_context
-def llamactl(
-    ctx: click.Context, server: str, insecure: bool, timeout: float | None
-) -> None:
+def llamactl(ctx: click.Context, server: str, insecure: bool, timeout: float) -> None:
     ctx.obj = server, insecure, timeout
     if ctx.invoked_subcommand is None:
         click.echo(ctx.get_help())  # show the help if no subcommand was provided
 
 
-llamactl.add_command(deploy)
-llamactl.add_command(run)
-llamactl.add_command(status)
+llamactl.add_command(deploy_cmd)
+llamactl.add_command(run_cmd)
+llamactl.add_command(status_cmd)
diff --git a/llama_deploy/cli/deploy.py b/llama_deploy/cli/deploy.py
index 5de962c..025d05c 100644
--- a/llama_deploy/cli/deploy.py
+++ b/llama_deploy/cli/deploy.py
@@ -2,7 +2,7 @@ from typing import IO
 
 import click
 
-from llama_deploy.cli.utils import request
+from llama_deploy import Client
 
 
 @click.command()
@@ -10,18 +10,11 @@ from llama_deploy.cli.utils import request
 @click.argument("deployment_config_file", type=click.File("rb"))
 def deploy(global_config: tuple, deployment_config_file: IO) -> None:
     server_url, disable_ssl, timeout = global_config
-    deploy_url = f"{server_url}/deployments/create"
+    client = Client(api_server_url=server_url, disable_ssl=disable_ssl, timeout=timeout)
 
-    files = {"config_file": deployment_config_file.read()}
-    resp = request(
-        "POST", deploy_url, files=files, verify=not disable_ssl, timeout=timeout
-    )
+    try:
+        deployment = client.sync.apiserver.deployments.create(deployment_config_file)
+    except Exception as e:
+        raise click.ClickException(str(e))
 
-    if resp.status_code >= 400:
-        try:
-            raise click.ClickException(resp.json().get("detail", resp.text))
-        except ValueError:
-            raise click.ClickException(resp.text)
-
-    else:
-        click.echo(f"Deployment successful: {resp.json().get('name')}")
+    click.echo(f"Deployment successful: {deployment.id}")
diff --git a/llama_deploy/cli/run.py b/llama_deploy/cli/run.py
index 8c68404..f0cc791 100644
--- a/llama_deploy/cli/run.py
+++ b/llama_deploy/cli/run.py
@@ -1,7 +1,9 @@
 import json
 
 import click
-import httpx
+
+from llama_deploy import Client
+from llama_deploy.types import TaskDefinition
 
 
 @click.command()
@@ -27,14 +29,15 @@ def run(
     service: str,
 ) -> None:
     server_url, disable_ssl, timeout = global_config
-    deploy_url = f"{server_url}/deployments/{deployment}/tasks/run"
+    client = Client(api_server_url=server_url, disable_ssl=disable_ssl, timeout=timeout)
+
     payload = {"input": json.dumps(dict(arg))}
     if service:
         payload["agent_id"] = service
 
-    resp = httpx.post(deploy_url, verify=not disable_ssl, json=payload, timeout=timeout)
+    try:
+        result = client.sync.apiserver.deployments.tasks.run(TaskDefinition(**payload))
+    except Exception as e:
+        raise click.ClickException(str(e))
 
-    if resp.status_code >= 400:
-        raise click.ClickException(resp.json().get("detail"))
-    else:
-        click.echo(resp.json())
+    click.echo(result)
diff --git a/llama_deploy/cli/status.py b/llama_deploy/cli/status.py
index 04953c2..af536c1 100644
--- a/llama_deploy/cli/status.py
+++ b/llama_deploy/cli/status.py
@@ -1,28 +1,27 @@
 import click
 
-
-from llama_deploy.cli.utils import request
+from llama_deploy import Client
+from llama_deploy.types.apiserver import StatusEnum
 
 
 @click.command()
 @click.pass_obj  # global_config
 def status(global_config: tuple) -> None:
     server_url, disable_ssl, timeout = global_config
-    status_url = f"{server_url}/status/"
+    client = Client(api_server_url=server_url, disable_ssl=disable_ssl, timeout=timeout)
 
-    r = request("GET", status_url, verify=not disable_ssl, timeout=timeout)
-    if r.status_code >= 400:
-        body = r.json()
-        click.echo(
-            f"Llama Deploy is unhealthy: [{r.status_code}] {r.json().get('detail')}"
-        )
-        return
+    try:
+        status = client.sync.apiserver.status()
+    except Exception as e:
+        raise click.ClickException(str(e))
 
-    click.echo("Llama Deploy is up and running.")
-    body = r.json()
-    if deployments := body.get("deployments"):
-        click.echo("\nActive deployments:")
-        for d in deployments:
-            click.echo(f"- {d}")
+    if status.status == StatusEnum.HEALTHY:
+        click.echo("Llama Deploy is up and running.")
+        if status.deployments:
+            click.echo("\nActive deployments:")
+            for d in status.deployments:
+                click.echo(f"- {d}")
+        else:
+            click.echo("\nCurrently there are no active deployments")
     else:
-        click.echo("\nCurrently there are no active deployments")
+        click.echo(f"Llama Deploy is unhealthy: {status.status_message}")
diff --git a/llama_deploy/cli/utils.py b/llama_deploy/cli/utils.py
deleted file mode 100644
index 616dc2b..0000000
--- a/llama_deploy/cli/utils.py
+++ /dev/null
@@ -1,18 +0,0 @@
-from typing import Any
-from urllib.parse import urlparse
-
-import click
-import httpx
-
-
-def request(
-    method: str, url: str | httpx.URL, *args: Any, **kwargs: Any
-) -> httpx.Response:
-    try:
-        return httpx.request(method, url, *args, **kwargs)
-    except httpx.ConnectError:
-        parsed_url = urlparse(str(url))
-        raise click.ClickException(
-            "Llama Deploy is not responding, check that the apiserver "
-            f"is running at {parsed_url.scheme}://{parsed_url.netloc} and try again."
-        )
diff --git a/llama_deploy/client/models/apiserver.py b/llama_deploy/client/models/apiserver.py
index 638ea80..e69a795 100644
--- a/llama_deploy/client/models/apiserver.py
+++ b/llama_deploy/client/models/apiserver.py
@@ -211,7 +211,14 @@ class DeploymentCollection(Collection):
     """A model representing a collection of deployments currently active."""
 
     async def create(self, config: TextIO) -> Deployment:
-        """Creates a new deployment from a deployment file."""
+        """Creates a new deployment from a deployment file.
+
+        Example:
+            ```
+            with open("deployment.yml") as f:
+                await client.apiserver.deployments.create(f)
+            ```
+        """
         create_url = f"{self.client.api_server_url}/deployments/create"
 
         files = {"config_file": config.read()}


Test patch:
diff --git a/tests/cli/test_deploy.py b/tests/cli/test_deploy.py
index c958474..ed636af 100644
--- a/tests/cli/test_deploy.py
+++ b/tests/cli/test_deploy.py
@@ -1,6 +1,7 @@
 from pathlib import Path
 from unittest import mock
 
+import httpx
 from click.testing import CliRunner
 
 from llama_deploy.cli import llamactl
@@ -8,29 +9,34 @@ from llama_deploy.cli import llamactl
 
 def test_deploy(runner: CliRunner, data_path: Path) -> None:
     test_config_file = data_path / "deployment.yaml"
-    mocked_response = mock.MagicMock(status_code=200, json=lambda: {})
-    with mock.patch("llama_deploy.cli.utils.httpx.request") as mocked_httpx:
-        mocked_httpx.return_value = mocked_response
+    mocked_result = mock.MagicMock(id="test_deployment")
+    with mock.patch("llama_deploy.cli.deploy.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.deployments.create.return_value = (
+            mocked_result
+        )
+
         result = runner.invoke(llamactl, ["-t", "5.0", "deploy", str(test_config_file)])
 
         assert result.exit_code == 0
-        with open(test_config_file, "rb") as f:
-            mocked_httpx.assert_called_with(
-                "POST",
-                "http://localhost:4501/deployments/create",
-                files={"config_file": f.read()},
-                verify=True,
-                timeout=5,
-            )
+        assert result.output == "Deployment successful: test_deployment\n"
+        mocked_client.assert_called_with(
+            api_server_url="http://localhost:4501", disable_ssl=False, timeout=5.0
+        )
+        file_arg = (
+            mocked_client.return_value.sync.apiserver.deployments.create.call_args
+        )
+        assert str(test_config_file) == file_arg.args[0].name
 
 
 def test_deploy_failed(runner: CliRunner, data_path: Path) -> None:
     test_config_file = data_path / "deployment.yaml"
-    mocked_response = mock.MagicMock(
-        status_code=401, json=lambda: {"detail": "Unauthorized!"}
-    )
-    with mock.patch("llama_deploy.cli.utils.httpx.request") as mocked_httpx:
-        mocked_httpx.return_value = mocked_response
+    with mock.patch("llama_deploy.cli.deploy.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.deployments.create.side_effect = (
+            httpx.HTTPStatusError(
+                "Unauthorized!", response=mock.MagicMock(), request=mock.MagicMock()
+            )
+        )
+
         result = runner.invoke(llamactl, ["deploy", str(test_config_file)])
         assert result.exit_code == 1
         assert result.output == "Error: Unauthorized!\n"
diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py
index 01d1319..61b6748 100644
--- a/tests/cli/test_run.py
+++ b/tests/cli/test_run.py
@@ -1,41 +1,58 @@
 from unittest import mock
 
+import httpx
 from click.testing import CliRunner
 
 from llama_deploy.cli import llamactl
+from llama_deploy.types import TaskDefinition
 
 
 def test_run(runner: CliRunner) -> None:
-    mocked_response = mock.MagicMock(status_code=200, json=lambda: {})
-    with mock.patch("httpx.post") as mocked_post:
-        mocked_post.return_value = mocked_response
+    mocked_result = mock.MagicMock(id="test_deployment")
+    with mock.patch("llama_deploy.cli.run.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.deployments.tasks.run.return_value = (
+            mocked_result
+        )
+
         result = runner.invoke(
-            llamactl, ["run", "-d", "deployment_name", "-s", "service_name"]
+            llamactl,
+            ["run", "-d", "deployment_name", "-s", "service_name"],
         )
-        mocked_post.assert_called_with(
-            "http://localhost:4501/deployments/deployment_name/tasks/run",
-            verify=True,
-            json={"input": "{}", "agent_id": "service_name"},
-            timeout=None,
+
+        mocked_client.assert_called_with(
+            api_server_url="http://localhost:4501", disable_ssl=False, timeout=120.0
         )
+
+        args = mocked_client.return_value.sync.apiserver.deployments.tasks.run.call_args
+        actual = args[0][0]
+        expected = TaskDefinition(agent_id="service_name", input="{}")
+        assert expected.input == actual.input
+        assert expected.agent_id == actual.agent_id
+        assert actual.session_id is None
         assert result.exit_code == 0
 
 
 def test_run_error(runner: CliRunner) -> None:
-    mocked_response = mock.MagicMock(
-        status_code=500, json=lambda: {"detail": "test error"}
-    )
-    with mock.patch("httpx.post") as mocked_post:
-        mocked_post.return_value = mocked_response
+    with mock.patch("llama_deploy.cli.run.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.deployments.tasks.run.side_effect = (
+            httpx.HTTPStatusError(
+                "test error", response=mock.MagicMock(), request=mock.MagicMock()
+            )
+        )
+
         result = runner.invoke(llamactl, ["run", "-d", "deployment_name"])
+
         assert result.exit_code == 1
         assert result.output == "Error: test error\n"
 
 
 def test_run_args(runner: CliRunner) -> None:
-    mocked_response = mock.MagicMock(status_code=200, json=lambda: {})
-    with mock.patch("httpx.post") as mocked_post:
-        mocked_post.return_value = mocked_response
+    mocked_result = mock.MagicMock(id="test_deployment")
+    with mock.patch("llama_deploy.cli.run.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.deployments.tasks.run.return_value = (
+            mocked_result
+        )
+
         result = runner.invoke(
             llamactl,
             [
@@ -50,12 +67,13 @@ def test_run_args(runner: CliRunner) -> None:
                 '"second value with spaces"',
             ],
         )
-        mocked_post.assert_called_with(
-            "http://localhost:4501/deployments/deployment_name/tasks/run",
-            verify=True,
-            json={
-                "input": '{"first_arg": "first_value", "second_arg": "\\"second value with spaces\\""}',
-            },
-            timeout=None,
+
+        args = mocked_client.return_value.sync.apiserver.deployments.tasks.run.call_args
+        actual = args[0][0]
+        expected = TaskDefinition(
+            input='{"first_arg": "first_value", "second_arg": "\\"second value with spaces\\""}',
         )
+        assert expected.input == actual.input
+        assert expected.agent_id == actual.agent_id
+        assert actual.session_id is None
         assert result.exit_code == 0
diff --git a/tests/cli/test_status.py b/tests/cli/test_status.py
index 54534ff..a1d1599 100644
--- a/tests/cli/test_status.py
+++ b/tests/cli/test_status.py
@@ -3,28 +3,44 @@ from unittest import mock
 from click.testing import CliRunner
 
 from llama_deploy.cli import llamactl
+from llama_deploy.types.apiserver import Status, StatusEnum
+
+
+def test_status_raised(runner: CliRunner) -> None:
+    with mock.patch("llama_deploy.cli.status.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.status.side_effect = Exception()
+        result = runner.invoke(llamactl, ["-s", "https://test", "status"])
+        assert result.exit_code == 1
 
 
 def test_status_server_down(runner: CliRunner) -> None:
-    result = runner.invoke(llamactl, ["-s", "https://test", "status"])
-    assert result.exit_code == 1
-    print(result.output)
-    assert "Error: Llama Deploy is not responding" in result.output
+    with mock.patch("llama_deploy.cli.status.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.status.return_value = Status(
+            status=StatusEnum.DOWN, status_message="API Server is down for tests"
+        )
+        result = runner.invoke(llamactl, ["-s", "https://test", "status"])
+        assert result.exit_code == 0
+        assert (
+            "Llama Deploy is unhealthy: API Server is down for tests" in result.output
+        )
 
 
 def test_status_unhealthy(runner: CliRunner) -> None:
-    mocked_response = mock.MagicMock(status_code=500)
-    with mock.patch("llama_deploy.cli.utils.httpx.request") as mocked_httpx:
-        mocked_httpx.return_value = mocked_response
+    with mock.patch("llama_deploy.cli.status.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.status.return_value = Status(
+            status=StatusEnum.UNHEALTHY, status_message="test_message"
+        )
+
         result = runner.invoke(llamactl, ["status"])
         assert result.exit_code == 0
-        assert "Llama Deploy is unhealthy: [500]" in result.output
+        assert "Llama Deploy is unhealthy: test_message" in result.output
 
 
 def test_status(runner: CliRunner) -> None:
-    mocked_response = mock.MagicMock(status_code=200, json=lambda: {})
-    with mock.patch("llama_deploy.cli.utils.httpx.request") as mocked_httpx:
-        mocked_httpx.return_value = mocked_response
+    with mock.patch("llama_deploy.cli.status.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.status.return_value = Status(
+            status=StatusEnum.HEALTHY, status_message="test_message"
+        )
         result = runner.invoke(llamactl, ["status"])
         assert result.exit_code == 0
         assert (
@@ -34,11 +50,15 @@ def test_status(runner: CliRunner) -> None:
 
 
 def test_status_with_deployments(runner: CliRunner) -> None:
-    mocked_response = mock.MagicMock(status_code=200)
-    mocked_response.json.return_value = {"deployments": ["foo", "bar"]}
-    with mock.patch("llama_deploy.cli.utils.httpx.request") as mocked_httpx:
-        mocked_httpx.return_value = mocked_response
+    with mock.patch("llama_deploy.cli.status.Client") as mocked_client:
+        mocked_client.return_value.sync.apiserver.status.return_value = Status(
+            status=StatusEnum.HEALTHY,
+            status_message="test_message",
+            deployments=["foo", "bar"],
+        )
+
         result = runner.invoke(llamactl, ["status"])
+
         assert result.exit_code == 0
         assert result.output == (
             "Llama Deploy is up and running.\n\nActive deployments:\n- foo\n- bar\n"