Skip to content

Commit 532de71

Browse files
michaelshinmichaelshin
committed
address comments
1 parent 70c5acb commit 532de71

5 files changed

Lines changed: 105 additions & 85 deletions

File tree

‎centml/cli/cluster.py‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,8 @@ def _get_replica_info(deployment):
7575
return {"min": "N/A", "max": "N/A"}
7676

7777

78-
def _get_ready_status(cclient, deployment):
78+
def _get_ready_status(deployment, service_status):
7979
api_status = deployment.status
80-
service_status = (
81-
cclient.get_status(deployment.id).service_status if deployment.status == DeploymentStatus.ACTIVE else None
82-
)
8380

8481
status_styles = {
8582
(DeploymentStatus.PAUSED, None): ("paused", "yellow", "black"),
@@ -103,37 +100,32 @@ def _get_ready_status(cclient, deployment):
103100
return click.style(style[0], fg=style[1], bg=style[2])
104101

105102

106-
def _get_status_error_messages(cclient, deployment):
107-
if deployment.status != DeploymentStatus.ACTIVE:
108-
return []
103+
def _append_status_error_message(messages, seen_messages, label, error_message):
104+
if not error_message or error_message in seen_messages:
105+
return
109106

110-
try:
111-
status = cclient.get_status_v3(deployment.id)
112-
except ApiException as e:
113-
if e.status not in [400, 404]:
114-
raise
107+
seen_messages.add(error_message)
108+
messages.append(f"{label}: {error_message}")
115109

116-
status = cclient.get_status(deployment.id)
117-
error_message = getattr(status, "error_message", None)
118-
return [error_message] if error_message else []
119110

120-
messages = []
121-
seen_messages = set()
111+
def _get_status_error_messages(status_response):
112+
if status_response is None:
113+
return []
122114

123-
def add_message(label, error_message):
124-
if not error_message or error_message in seen_messages:
125-
return
115+
error_message = getattr(status_response, "error_message", None)
116+
if error_message:
117+
return [error_message]
126118

127-
seen_messages.add(error_message)
128-
messages.append(f"{label}: {error_message}")
119+
messages = []
120+
seen_messages = set()
129121

130-
for revision in status.revision_pod_details_list or []:
122+
for revision in getattr(status_response, "revision_pod_details_list", None) or []:
131123
revision_label = f"revision {revision.revision_number}" if revision.revision_number is not None else "revision"
132-
add_message(revision_label, revision.error_message)
124+
_append_status_error_message(messages, seen_messages, revision_label, revision.error_message)
133125

134-
for pod in revision.pod_details_list or []:
126+
for pod in getattr(revision, "pod_details_list", None) or []:
135127
pod_label = pod.name or "pod"
136-
add_message(f"{revision_label} / {pod_label}", pod.error_message)
128+
_append_status_error_message(messages, seen_messages, f"{revision_label} / {pod_label}", pod.error_message)
137129

138130
return messages
139131

@@ -184,8 +176,10 @@ def get(type, id):
184176
else:
185177
sys.exit("Please enter correct deployment type")
186178

187-
ready_status = _get_ready_status(cclient, deployment)
188-
status_error_messages = _get_status_error_messages(cclient, deployment)
179+
deployment_status = cclient.get_status(deployment) if deployment.status == DeploymentStatus.ACTIVE else None
180+
service_status = deployment_status.service_status if deployment_status is not None else None
181+
ready_status = _get_ready_status(deployment, service_status)
182+
status_error_messages = _get_status_error_messages(deployment_status)
189183
_, id_to_hw_map = _get_hw_to_id_map(cclient, deployment.cluster_id)
190184
hw = id_to_hw_map[deployment.hardware_instance_id]
191185
detail_rows = [

‎centml/sdk/api.py‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
from centml.sdk.config import settings
1717

1818

19+
STATUS_V3_DEPLOYMENT_TYPES = {DeploymentType.INFERENCE_V3, DeploymentType.CSERVE_V3}
20+
21+
1922
class CentMLClient:
2023
def __init__(self, api):
2124
self._api: platform_api_python_client.EXTERNALApi = api
@@ -25,11 +28,18 @@ def get(self, depl_type):
2528
deployments = sorted(results, reverse=True, key=lambda d: d.created_at)
2629
return deployments
2730

28-
def get_status(self, id):
29-
return self._api.get_deployment_status_deployments_status_deployment_id_get(id)
31+
def get_status(self, deployment):
32+
deployment_id = getattr(deployment, "id", deployment)
33+
deployment_type = getattr(deployment, "type", None)
34+
35+
if deployment_type in STATUS_V3_DEPLOYMENT_TYPES:
36+
return self._api.get_deployment_status_v3_deployments_status_v3_deployment_id_get(deployment_id)
37+
38+
status = self._api.get_deployment_status_deployments_status_deployment_id_get(deployment_id)
39+
if deployment_type is None and getattr(status, "type", None) in STATUS_V3_DEPLOYMENT_TYPES:
40+
return self._api.get_deployment_status_v3_deployments_status_v3_deployment_id_get(deployment_id)
3041

31-
def get_status_v3(self, deployment_id):
32-
return self._api.get_deployment_status_v3_deployments_status_v3_deployment_id_get(deployment_id)
42+
return status
3343

3444
def get_inference(self, id):
3545
"""Get Inference deployment details - automatically handles both V2 and V3 deployments"""

‎centml/sdk/shell/session.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ def build_ws_url(api_url, deployment_id, pod_name, shell_type=None):
3737

3838

3939
def get_running_pods(cclient, deployment_id) -> list[str]:
40-
status = cclient.get_status_v3(deployment_id)
40+
status = cclient.get_status(deployment_id)
4141
running_pods = []
42-
for revision in status.revision_pod_details_list or []:
42+
for revision in getattr(status, "revision_pod_details_list", None) or []:
4343
for pod in revision.pod_details_list or []:
4444
if pod.status == PodStatus.RUNNING and pod.name:
4545
running_pods.append(pod.name)

‎tests/test_cli_cluster.py‎

Lines changed: 25 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,43 @@
11
from types import SimpleNamespace
22

3-
import pytest
4-
5-
from centml.cli.cluster import DeploymentStatus, _get_status_error_messages
6-
from centml.sdk import ApiException
3+
from centml.cli.cluster import _get_status_error_messages
74

85

96
def test_status_error_messages_include_revision_and_pod_messages():
10-
cclient = SimpleNamespace(
11-
get_status_v3=lambda _id: SimpleNamespace(
12-
revision_pod_details_list=[
13-
SimpleNamespace(
14-
revision_number=3,
15-
error_message="revision failed",
16-
pod_details_list=[
17-
SimpleNamespace(name="pod-a", error_message="image pull failed"),
18-
SimpleNamespace(name="pod-b", error_message=None),
19-
],
20-
)
21-
]
22-
)
7+
status_response = SimpleNamespace(
8+
revision_pod_details_list=[
9+
SimpleNamespace(
10+
revision_number=3,
11+
error_message="revision failed",
12+
pod_details_list=[
13+
SimpleNamespace(name="pod-a", error_message="image pull failed"),
14+
SimpleNamespace(name="pod-b", error_message=None),
15+
],
16+
)
17+
]
2318
)
24-
deployment = SimpleNamespace(id=123, status=DeploymentStatus.ACTIVE)
2519

26-
messages = _get_status_error_messages(cclient, deployment)
20+
messages = _get_status_error_messages(status_response)
2721

2822
assert messages == ["revision 3: revision failed", "revision 3 / pod-a: image pull failed"]
2923

3024

3125
def test_status_error_messages_do_not_repeat_duplicate_messages():
3226
duplicate_message = "one or more objects failed to apply"
33-
cclient = SimpleNamespace(
34-
get_status_v3=lambda _id: SimpleNamespace(
35-
revision_pod_details_list=[
36-
SimpleNamespace(
37-
revision_number=None,
38-
error_message=duplicate_message,
39-
pod_details_list=[SimpleNamespace(name=None, error_message=duplicate_message)],
40-
)
41-
]
42-
)
27+
status_response = SimpleNamespace(
28+
revision_pod_details_list=[
29+
SimpleNamespace(
30+
revision_number=None,
31+
error_message=duplicate_message,
32+
pod_details_list=[SimpleNamespace(name=None, error_message=duplicate_message)],
33+
)
34+
]
4335
)
44-
deployment = SimpleNamespace(id=123, status=DeploymentStatus.ACTIVE)
45-
46-
assert _get_status_error_messages(cclient, deployment) == [f"revision: {duplicate_message}"]
47-
48-
49-
def test_status_error_messages_fall_back_to_legacy_status_message():
50-
legacy_status = SimpleNamespace(error_message="legacy service failure")
51-
52-
def get_status_v3(_id):
53-
raise ApiException(status=404)
54-
55-
cclient = SimpleNamespace(get_status_v3=get_status_v3, get_status=lambda _id: legacy_status)
56-
deployment = SimpleNamespace(id=123, status=DeploymentStatus.ACTIVE)
57-
58-
assert _get_status_error_messages(cclient, deployment) == ["legacy service failure"]
5936

37+
assert _get_status_error_messages(status_response) == [f"revision: {duplicate_message}"]
6038

61-
def test_status_error_messages_reraises_unexpected_v3_errors():
62-
def get_status_v3(_id):
63-
raise ApiException(status=500)
6439

65-
cclient = SimpleNamespace(get_status_v3=get_status_v3)
66-
deployment = SimpleNamespace(id=123, status=DeploymentStatus.ACTIVE)
40+
def test_status_error_messages_include_legacy_status_message():
41+
status_response = SimpleNamespace(error_message="legacy service failure")
6742

68-
with pytest.raises(ApiException):
69-
_get_status_error_messages(cclient, deployment)
43+
assert _get_status_error_messages(status_response) == ["legacy service failure"]

‎tests/test_sdk_api.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
from types import SimpleNamespace
2+
from unittest.mock import MagicMock
3+
4+
from centml.sdk import DeploymentType
5+
from centml.sdk.api import CentMLClient
6+
7+
8+
def test_get_status_uses_v3_endpoint_when_deployment_type_is_v3():
9+
api = MagicMock()
10+
deployment = SimpleNamespace(id=123, type=DeploymentType.INFERENCE_V3)
11+
expected_status = SimpleNamespace()
12+
api.get_deployment_status_v3_deployments_status_v3_deployment_id_get.return_value = expected_status
13+
14+
assert CentMLClient(api).get_status(deployment) is expected_status
15+
16+
api.get_deployment_status_v3_deployments_status_v3_deployment_id_get.assert_called_once_with(123)
17+
api.get_deployment_status_deployments_status_deployment_id_get.assert_not_called()
18+
19+
20+
def test_get_status_uses_legacy_endpoint_when_deployment_type_is_legacy():
21+
api = MagicMock()
22+
deployment = SimpleNamespace(id=123, type=DeploymentType.INFERENCE)
23+
expected_status = SimpleNamespace(type=DeploymentType.INFERENCE)
24+
api.get_deployment_status_deployments_status_deployment_id_get.return_value = expected_status
25+
26+
assert CentMLClient(api).get_status(deployment) is expected_status
27+
28+
api.get_deployment_status_deployments_status_deployment_id_get.assert_called_once_with(123)
29+
api.get_deployment_status_v3_deployments_status_v3_deployment_id_get.assert_not_called()
30+
31+
32+
def test_get_status_discovers_type_for_id_only_callers():
33+
api = MagicMock()
34+
legacy_status = SimpleNamespace(type=DeploymentType.CSERVE_V3)
35+
expected_status = SimpleNamespace()
36+
api.get_deployment_status_deployments_status_deployment_id_get.return_value = legacy_status
37+
api.get_deployment_status_v3_deployments_status_v3_deployment_id_get.return_value = expected_status
38+
39+
assert CentMLClient(api).get_status(123) is expected_status
40+
41+
api.get_deployment_status_deployments_status_deployment_id_get.assert_called_once_with(123)
42+
api.get_deployment_status_v3_deployments_status_v3_deployment_id_get.assert_called_once_with(123)

0 commit comments

Comments
 (0)