Skip to content

Commit 619c2ff

Browse files
nicolai86tchaton
andauthored
[CLI] fix cluster creation CLI requiring instance-type selection (#14056)
fix cluster creation CLI requiring instace-type selection we've marked `instance_types` as `required=False`, but the CLI calls `split` on the value. So if nothing is provided, we'll actually receive a runtime error, effectively rendering the flag as required. Co-authored-by: thomas chaton <thomas@grid.ai>
1 parent d850854 commit 619c2ff

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

src/lightning_app/cli/lightning_cli_create.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def create_cluster(
7979
region=region,
8080
role_arn=role_arn,
8181
external_id=external_id,
82-
instance_types=instance_types.split(","),
82+
instance_types=instance_types.split(",") if instance_types is not None else None,
8383
edit_before_creation=edit_before_creation,
8484
cost_savings=cost_savings,
8585
wait=wait,

tests/tests_app/cli/test_cli.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,15 @@ def test_main_lightning_cli_help():
7070

7171
@mock.patch("lightning_cloud.login.Auth.authenticate", MagicMock())
7272
@mock.patch("lightning_app.cli.cmd_clusters.AWSClusterManager.create")
73-
def test_create_cluster(create: mock.MagicMock):
73+
@pytest.mark.parametrize(
74+
"instance_types,expected_instance_types",
75+
[
76+
(["--instance-types", "t3.xlarge"], ["t3.xlarge"]),
77+
(["--instance-types", "t3.xlarge,t3.2xlarge"], ["t3.xlarge", "t3.2xlarge"]),
78+
([], None),
79+
],
80+
)
81+
def test_create_cluster(create_command: mock.MagicMock, instance_types, expected_instance_types):
7482
runner = CliRunner()
7583
runner.invoke(
7684
create_cluster,
@@ -82,17 +90,16 @@ def test_create_cluster(create: mock.MagicMock):
8290
"dummy",
8391
"--role-arn",
8492
"arn:aws:iam::1234567890:role/lai-byoc",
85-
"--instance-types",
86-
"t2.small",
87-
],
93+
]
94+
+ instance_types,
8895
)
8996

90-
create.assert_called_once_with(
97+
create_command.assert_called_once_with(
9198
cluster_name="test-7",
9299
region="us-east-1",
93100
role_arn="arn:aws:iam::1234567890:role/lai-byoc",
94101
external_id="dummy",
95-
instance_types=["t2.small"],
102+
instance_types=expected_instance_types,
96103
edit_before_creation=False,
97104
cost_savings=False,
98105
wait=False,

0 commit comments

Comments
 (0)