Skip to content

Commit 0a4c969

Browse files
committed
fix: address second round of review comments
- Remove unused gpu_count from llm-d template context - Use distinct gpu_count/tensor_parallel values in test fixture to catch regressions - Mock ModelCatalog and use tmp_path in client fixture to avoid disk side-effects Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Jing Chen <jing.chen2@ibm.com>
1 parent ac1690a commit 0a4c969

2 files changed

Lines changed: 11 additions & 8 deletions

File tree

src/planner/configuration/llmd_generator.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ def _prepare_context(
6363
"deployment_id": deployment_id,
6464
"namespace": namespace,
6565
"model_id": model_id,
66-
"gpu_count": gpu_config.gpu_count if gpu_config else 1,
6766
"tensor_parallel": tensor_parallel,
6867
"gpus_per_replica": tensor_parallel,
6968
"replicas": gpu_config.replicas if gpu_config else 1,

tests/unit/test_llmd_generator.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from __future__ import annotations
44

5-
from unittest.mock import MagicMock
5+
from unittest.mock import MagicMock, patch
66

77
import pytest
88
import yaml
@@ -21,11 +21,15 @@
2121

2222

2323
@pytest.fixture
24-
def client() -> TestClient:
25-
"""Create a test client with mocked app state (no DB required)."""
24+
def client(tmp_path) -> TestClient:
25+
"""Create a test client with mocked app state (no DB or disk side-effects)."""
2626
app = FastAPI()
27-
app.state.deployment_generator = DeploymentGenerator(simulator_mode=False)
28-
app.state.llmd_deployment_generator = LlmdDeploymentGenerator()
27+
28+
with patch("planner.configuration.generator.ModelCatalog"):
29+
app.state.deployment_generator = DeploymentGenerator(
30+
output_dir=str(tmp_path / "vllm"), simulator_mode=False
31+
)
32+
app.state.llmd_deployment_generator = LlmdDeploymentGenerator(output_dir=str(tmp_path / "llmd"))
2933
app.state.yaml_validator = YAMLValidator()
3034
app.state.cluster_managers = {}
3135
app.state.cluster_manager_lock = MagicMock()
@@ -55,7 +59,7 @@ def sample_recommendation() -> DeploymentRecommendation:
5559
meets_slo=True,
5660
gpu_config=GPUConfig(
5761
gpu_type="NVIDIA-A100-80GB",
58-
gpu_count=2,
62+
gpu_count=6,
5963
tensor_parallel=2,
6064
replicas=3,
6165
),
@@ -251,7 +255,7 @@ def test_sets_gpu_resources_per_replica(
251255
parsed = yaml.safe_load(result["contents"]["patch_vllm"])
252256

253257
container = parsed["spec"]["template"]["spec"]["containers"][0]
254-
# tensor_parallel=2, so each replica gets 2 GPUs (not gpu_count which is total)
258+
# gpu_count=6, tensor_parallel=2, replicas=3 — each pod gets tensor_parallel GPUs
255259
assert container["resources"]["requests"]["nvidia.com/gpu"] == "2"
256260
assert container["resources"]["limits"]["nvidia.com/gpu"] == "2"
257261

0 commit comments

Comments
 (0)