Fix deploy timeout and improve error messages (#770)
- update_deployment() was using httpx default timeout (5s) instead of the 360s used by deploy_server_to_engine(), causing "The write operation timed out" errors on larger packages - Catch httpx.TimeoutException in both deploy paths with an actionable error message that points to package size as the likely cause - Add proper error handling (ConnectError, HTTPStatusError) and client.close() to update_deployment(), matching deploy_server_to_engine() - Add unit tests covering timeout handling and timeout constant usage
This commit is contained in:
parent
4d48bb765d
commit
3e9ffb6bd9
2 changed files with 167 additions and 10 deletions
|
|
@ -40,6 +40,10 @@ from arcade_cli.utils import (
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
# Constants
|
||||||
|
|
||||||
|
DEPLOY_TIMEOUT_SECONDS = 360
|
||||||
|
|
||||||
# Models
|
# Models
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -115,7 +119,7 @@ def _get_deployment_status(engine_url: str, server_name: str) -> str:
|
||||||
Possible values are: "pending", "updating", "unknown", "running", "failed".
|
Possible values are: "pending", "updating", "unknown", "running", "failed".
|
||||||
"""
|
"""
|
||||||
url = get_org_scoped_url(engine_url, f"/deployments/{server_name}/status")
|
url = get_org_scoped_url(engine_url, f"/deployments/{server_name}/status")
|
||||||
client = httpx.Client(headers=get_auth_headers(), timeout=360)
|
client = httpx.Client(headers=get_auth_headers(), timeout=DEPLOY_TIMEOUT_SECONDS)
|
||||||
response = client.get(url)
|
response = client.get(url)
|
||||||
response.raise_for_status()
|
response.raise_for_status()
|
||||||
status = cast(str, response.json().get("status", "unknown"))
|
status = cast(str, response.json().get("status", "unknown"))
|
||||||
|
|
@ -304,12 +308,48 @@ def update_deployment(
|
||||||
engine_url: str,
|
engine_url: str,
|
||||||
server_name: str,
|
server_name: str,
|
||||||
update_deployment_request: dict,
|
update_deployment_request: dict,
|
||||||
|
debug: bool = False,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Update a deployment in the Arcade Engine."""
|
"""Update a deployment in the Arcade Engine.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
engine_url: The base URL of the Arcade Engine
|
||||||
|
server_name: The name of the server to update
|
||||||
|
update_deployment_request: The update deployment request payload
|
||||||
|
debug: Whether to show debug information
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
httpx.HTTPStatusError: If the deployment request fails
|
||||||
|
httpx.ConnectError: If connection to the engine fails
|
||||||
|
"""
|
||||||
url = get_org_scoped_url(engine_url, f"/deployments/{server_name}")
|
url = get_org_scoped_url(engine_url, f"/deployments/{server_name}")
|
||||||
client = httpx.Client(headers=get_auth_headers())
|
client = httpx.Client(headers=get_auth_headers(), timeout=DEPLOY_TIMEOUT_SECONDS)
|
||||||
response = client.put(url, json=update_deployment_request)
|
|
||||||
response.raise_for_status()
|
try:
|
||||||
|
response = client.put(url, json=update_deployment_request)
|
||||||
|
response.raise_for_status()
|
||||||
|
except httpx.TimeoutException as e:
|
||||||
|
raise ValueError(
|
||||||
|
f"Deployment update timed out. This is often caused by a large deployment "
|
||||||
|
f"package. Try reducing package size by removing unnecessary files "
|
||||||
|
f"(data files, large assets, etc.) from your project directory. "
|
||||||
|
f"Current timeout: {DEPLOY_TIMEOUT_SECONDS}s. Details: {e}"
|
||||||
|
) from e
|
||||||
|
except httpx.ConnectError as e:
|
||||||
|
raise ValueError(f"Failed to connect to Arcade Engine at {engine_url}: {e}") from e
|
||||||
|
except httpx.HTTPStatusError as e:
|
||||||
|
error_detail = ""
|
||||||
|
try:
|
||||||
|
error_json = e.response.json()
|
||||||
|
error_detail = f": {error_json}"
|
||||||
|
except Exception:
|
||||||
|
error_detail = f": {e.response.text}"
|
||||||
|
|
||||||
|
raise ValueError(
|
||||||
|
f"Deployment update failed with HTTP {e.response.status_code}{error_detail}"
|
||||||
|
) from e
|
||||||
|
finally:
|
||||||
|
client.close()
|
||||||
|
|
||||||
|
|
||||||
def create_package_archive(package_dir: Path) -> str:
|
def create_package_archive(package_dir: Path) -> str:
|
||||||
|
|
@ -734,12 +774,19 @@ def deploy_server_to_engine(
|
||||||
httpx.ConnectError: If connection to the engine fails
|
httpx.ConnectError: If connection to the engine fails
|
||||||
"""
|
"""
|
||||||
url = get_org_scoped_url(engine_url, "/deployments")
|
url = get_org_scoped_url(engine_url, "/deployments")
|
||||||
client = httpx.Client(headers=get_auth_headers(), timeout=360)
|
client = httpx.Client(headers=get_auth_headers(), timeout=DEPLOY_TIMEOUT_SECONDS)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
response = client.post(url, json=deployment_request)
|
response = client.post(url, json=deployment_request)
|
||||||
response.raise_for_status()
|
response.raise_for_status()
|
||||||
return cast(dict, response.json())
|
return cast(dict, response.json())
|
||||||
|
except httpx.TimeoutException as e:
|
||||||
|
raise ValueError(
|
||||||
|
f"Deployment request timed out. This is often caused by a large deployment "
|
||||||
|
f"package. Try reducing package size by removing unnecessary files "
|
||||||
|
f"(data files, large assets, etc.) from your project directory. "
|
||||||
|
f"Current timeout: {DEPLOY_TIMEOUT_SECONDS}s. Details: {e}"
|
||||||
|
) from e
|
||||||
except httpx.ConnectError as e:
|
except httpx.ConnectError as e:
|
||||||
raise ValueError(f"Failed to connect to Arcade Engine at {engine_url}: {e}") from e
|
raise ValueError(f"Failed to connect to Arcade Engine at {engine_url}: {e}") from e
|
||||||
except httpx.HTTPStatusError as e:
|
except httpx.HTTPStatusError as e:
|
||||||
|
|
@ -902,7 +949,7 @@ def deploy_server_logic(
|
||||||
description="MCP Server deployed via CLI",
|
description="MCP Server deployed via CLI",
|
||||||
toolkits=deployment_toolkits,
|
toolkits=deployment_toolkits,
|
||||||
)
|
)
|
||||||
update_deployment(engine_url, server_name, update_request.model_dump())
|
update_deployment(engine_url, server_name, update_request.model_dump(), debug)
|
||||||
else:
|
else:
|
||||||
create_request = CreateDeploymentRequest(
|
create_request = CreateDeploymentRequest(
|
||||||
name=server_name,
|
name=server_name,
|
||||||
|
|
|
||||||
|
|
@ -6,12 +6,16 @@ import tarfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import MagicMock, patch
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
import httpx
|
||||||
import pytest
|
import pytest
|
||||||
from arcade_cli.deploy import (
|
from arcade_cli.deploy import (
|
||||||
|
DEPLOY_TIMEOUT_SECONDS,
|
||||||
create_package_archive,
|
create_package_archive,
|
||||||
|
deploy_server_to_engine,
|
||||||
get_required_secrets,
|
get_required_secrets,
|
||||||
get_server_info,
|
get_server_info,
|
||||||
start_server_process,
|
start_server_process,
|
||||||
|
update_deployment,
|
||||||
verify_server_and_get_metadata,
|
verify_server_and_get_metadata,
|
||||||
wait_for_health,
|
wait_for_health,
|
||||||
)
|
)
|
||||||
|
|
@ -309,6 +313,114 @@ def test_verify_server_and_get_metadata_with_debug(valid_server_path: str, capsy
|
||||||
assert "MY_SECRET_KEY" in required_secrets
|
assert "MY_SECRET_KEY" in required_secrets
|
||||||
|
|
||||||
|
|
||||||
|
# Tests for deploy_server_to_engine
|
||||||
|
|
||||||
|
|
||||||
|
@patch("arcade_cli.deploy.get_auth_headers", return_value={"Authorization": "Bearer test"})
|
||||||
|
@patch(
|
||||||
|
"arcade_cli.deploy.get_org_scoped_url",
|
||||||
|
return_value="http://engine/v1/orgs/1/projects/1/deployments",
|
||||||
|
)
|
||||||
|
def test_deploy_server_to_engine_timeout_raises_helpful_error(
|
||||||
|
mock_url: MagicMock, mock_auth: MagicMock
|
||||||
|
) -> None:
|
||||||
|
"""Test that a timeout during deployment raises a clear, actionable error."""
|
||||||
|
with patch("arcade_cli.deploy.httpx.Client") as mock_client_cls:
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_client_cls.return_value = mock_client
|
||||||
|
mock_client.post.side_effect = httpx.WriteTimeout("The write operation timed out")
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="Deployment request timed out"):
|
||||||
|
deploy_server_to_engine("http://engine", {"test": "payload"})
|
||||||
|
|
||||||
|
|
||||||
|
@patch("arcade_cli.deploy.get_auth_headers", return_value={"Authorization": "Bearer test"})
|
||||||
|
@patch(
|
||||||
|
"arcade_cli.deploy.get_org_scoped_url",
|
||||||
|
return_value="http://engine/v1/orgs/1/projects/1/deployments",
|
||||||
|
)
|
||||||
|
def test_deploy_server_to_engine_timeout_mentions_package_size(
|
||||||
|
mock_url: MagicMock, mock_auth: MagicMock
|
||||||
|
) -> None:
|
||||||
|
"""Test that the timeout error message mentions package size as a likely cause."""
|
||||||
|
with patch("arcade_cli.deploy.httpx.Client") as mock_client_cls:
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_client_cls.return_value = mock_client
|
||||||
|
mock_client.post.side_effect = httpx.ReadTimeout("timed out")
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="large deployment package"):
|
||||||
|
deploy_server_to_engine("http://engine", {"test": "payload"})
|
||||||
|
|
||||||
|
|
||||||
|
@patch("arcade_cli.deploy.get_auth_headers", return_value={"Authorization": "Bearer test"})
|
||||||
|
@patch(
|
||||||
|
"arcade_cli.deploy.get_org_scoped_url",
|
||||||
|
return_value="http://engine/v1/orgs/1/projects/1/deployments",
|
||||||
|
)
|
||||||
|
def test_deploy_server_to_engine_uses_deploy_timeout(
|
||||||
|
mock_url: MagicMock, mock_auth: MagicMock
|
||||||
|
) -> None:
|
||||||
|
"""Test that deploy_server_to_engine uses the DEPLOY_TIMEOUT_SECONDS constant."""
|
||||||
|
with patch("arcade_cli.deploy.httpx.Client") as mock_client_cls:
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_client_cls.return_value = mock_client
|
||||||
|
mock_response = MagicMock()
|
||||||
|
mock_response.json.return_value = {"status": "ok"}
|
||||||
|
mock_client.post.return_value = mock_response
|
||||||
|
|
||||||
|
deploy_server_to_engine("http://engine", {"test": "payload"})
|
||||||
|
|
||||||
|
mock_client_cls.assert_called_once_with(
|
||||||
|
headers={"Authorization": "Bearer test"},
|
||||||
|
timeout=DEPLOY_TIMEOUT_SECONDS,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# Tests for update_deployment
|
||||||
|
|
||||||
|
|
||||||
|
@patch("arcade_cli.deploy.get_auth_headers", return_value={"Authorization": "Bearer test"})
|
||||||
|
@patch(
|
||||||
|
"arcade_cli.deploy.get_org_scoped_url",
|
||||||
|
return_value="http://engine/v1/orgs/1/projects/1/deployments/myserver",
|
||||||
|
)
|
||||||
|
def test_update_deployment_timeout_raises_helpful_error(
|
||||||
|
mock_url: MagicMock, mock_auth: MagicMock
|
||||||
|
) -> None:
|
||||||
|
"""Test that a timeout during deployment update raises a clear, actionable error."""
|
||||||
|
with patch("arcade_cli.deploy.httpx.Client") as mock_client_cls:
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_client_cls.return_value = mock_client
|
||||||
|
mock_client.put.side_effect = httpx.WriteTimeout("The write operation timed out")
|
||||||
|
|
||||||
|
with pytest.raises(ValueError, match="Deployment update timed out"):
|
||||||
|
update_deployment("http://engine", "myserver", {"test": "payload"})
|
||||||
|
|
||||||
|
|
||||||
|
@patch("arcade_cli.deploy.get_auth_headers", return_value={"Authorization": "Bearer test"})
|
||||||
|
@patch(
|
||||||
|
"arcade_cli.deploy.get_org_scoped_url",
|
||||||
|
return_value="http://engine/v1/orgs/1/projects/1/deployments/myserver",
|
||||||
|
)
|
||||||
|
def test_update_deployment_uses_deploy_timeout(mock_url: MagicMock, mock_auth: MagicMock) -> None:
|
||||||
|
"""Test that update_deployment uses the DEPLOY_TIMEOUT_SECONDS constant."""
|
||||||
|
with patch("arcade_cli.deploy.httpx.Client") as mock_client_cls:
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_client_cls.return_value = mock_client
|
||||||
|
|
||||||
|
update_deployment("http://engine", "myserver", {"test": "payload"})
|
||||||
|
|
||||||
|
mock_client_cls.assert_called_once_with(
|
||||||
|
headers={"Authorization": "Bearer test"},
|
||||||
|
timeout=DEPLOY_TIMEOUT_SECONDS,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_deploy_timeout_constant() -> None:
|
||||||
|
"""Test that the deploy timeout constant is correctly defined."""
|
||||||
|
assert DEPLOY_TIMEOUT_SECONDS == 360
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Debug-aware error messages
|
# Debug-aware error messages
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
@ -331,9 +443,7 @@ def test_start_server_process_non_debug_message(
|
||||||
|
|
||||||
@patch("arcade_cli.deploy.find_python_interpreter")
|
@patch("arcade_cli.deploy.find_python_interpreter")
|
||||||
@patch("arcade_cli.deploy.subprocess.Popen")
|
@patch("arcade_cli.deploy.subprocess.Popen")
|
||||||
def test_start_server_process_debug_message(
|
def test_start_server_process_debug_message(mock_popen: MagicMock, mock_python: MagicMock) -> None:
|
||||||
mock_popen: MagicMock, mock_python: MagicMock
|
|
||||||
) -> None:
|
|
||||||
"""Debug mode error should NOT tell user to run with --debug (already in debug mode)."""
|
"""Debug mode error should NOT tell user to run with --debug (already in debug mode)."""
|
||||||
mock_python.return_value = Path("python3")
|
mock_python.return_value = Path("python3")
|
||||||
mock_proc = MagicMock()
|
mock_proc = MagicMock()
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue