From 207e341db4dded083e24b7891b4c9ec21ca1dab8 Mon Sep 17 00:00:00 2001 From: Luca Furst Date: Tue, 25 Oct 2022 13:59:03 -0400 Subject: [PATCH 1/2] Remove instance types from cluster creation --- src/lightning_app/cli/cmd_clusters.py | 4 ---- src/lightning_app/cli/lightning_cli_create.py | 10 ---------- tests/tests_app/cli/test_cli.py | 13 ++++--------- tests/tests_app/cli/test_cmd_clusters.py | 3 --- 4 files changed, 4 insertions(+), 26 deletions(-) diff --git a/src/lightning_app/cli/cmd_clusters.py b/src/lightning_app/cli/cmd_clusters.py index 0cff6eb83a854..4bb8b1fdb793f 100644 --- a/src/lightning_app/cli/cmd_clusters.py +++ b/src/lightning_app/cli/cmd_clusters.py @@ -15,7 +15,6 @@ V1ClusterState, V1ClusterType, V1CreateClusterRequest, - V1InstanceSpec, V1KubernetesClusterDriver, ) from rich.console import Console @@ -86,7 +85,6 @@ def create( role_arn: str = None, region: str = "us-east-1", external_id: str = None, - instance_types: List[str] = [], edit_before_creation: bool = False, wait: bool = False, ) -> None: @@ -98,7 +96,6 @@ def create( role_arn: AWS IAM Role ARN used to provision resources region: AWS region containing compute resources external_id: AWS IAM Role external ID - instance_types: AWS instance types supported by the cluster edit_before_creation: Enables interactive editing of requests before submitting it to Lightning AI. wait: Waits for the cluster to be in a RUNNING state. Only use this for debugging. """ @@ -119,7 +116,6 @@ def create( region=region, role_arn=role_arn, external_id=external_id, - instance_types=[V1InstanceSpec(name=x) for x in instance_types], ) ) ), diff --git a/src/lightning_app/cli/lightning_cli_create.py b/src/lightning_app/cli/lightning_cli_create.py index 55ef9a453c255..8b5c179777555 100644 --- a/src/lightning_app/cli/lightning_cli_create.py +++ b/src/lightning_app/cli/lightning_cli_create.py @@ -26,14 +26,6 @@ def create() -> None: default="us-east-1", help="AWS region that is used to host the associated resources.", ) -@click.option( - "--instance-types", - "instance_types", - type=str, - required=False, - default=None, - help="Instance types that you want to support, for computer jobs within the cluster.", -) @click.option( "--enable-performance", "enable_performance", @@ -65,7 +57,6 @@ def create_cluster( role_arn: str, external_id: str, provider: str, - instance_types: str, edit_before_creation: bool, enable_performance: bool, wait: bool, @@ -81,7 +72,6 @@ def create_cluster( region=region, role_arn=role_arn, external_id=external_id, - instance_types=instance_types.split(",") if instance_types is not None else [], edit_before_creation=edit_before_creation, cost_savings=not enable_performance, wait=wait, diff --git a/tests/tests_app/cli/test_cli.py b/tests/tests_app/cli/test_cli.py index fdd3031be2692..c87bcd24a3c33 100644 --- a/tests/tests_app/cli/test_cli.py +++ b/tests/tests_app/cli/test_cli.py @@ -94,17 +94,13 @@ def test_main_lightning_cli_help(): @mock.patch("lightning_cloud.login.Auth.authenticate", MagicMock()) @mock.patch("lightning_app.cli.cmd_clusters.AWSClusterManager.create") @pytest.mark.parametrize( - "extra_arguments,expected_instance_types,expected_cost_savings_mode", + "extra_arguments,expected_cost_savings_mode", [ - (["--instance-types", "t3.xlarge"], ["t3.xlarge"], True), - (["--instance-types", "t3.xlarge,t3.2xlarge"], ["t3.xlarge", "t3.2xlarge"], True), - ([], [], True), - (["--enable-performance"], [], False), + ([], True), + (["--enable-performance"], False), ], ) -def test_create_cluster( - create_command: mock.MagicMock, extra_arguments, expected_instance_types, expected_cost_savings_mode -): +def test_create_cluster(create_command: mock.MagicMock, extra_arguments, expected_cost_savings_mode): runner = CliRunner() runner.invoke( create_cluster, @@ -125,7 +121,6 @@ def test_create_cluster( region="us-east-1", role_arn="arn:aws:iam::1234567890:role/lai-byoc", external_id="dummy", - instance_types=expected_instance_types, edit_before_creation=False, cost_savings=expected_cost_savings_mode, wait=False, diff --git a/tests/tests_app/cli/test_cmd_clusters.py b/tests/tests_app/cli/test_cmd_clusters.py index 3063ffe742cb1..92df6c172c9f0 100644 --- a/tests/tests_app/cli/test_cmd_clusters.py +++ b/tests/tests_app/cli/test_cmd_clusters.py @@ -13,7 +13,6 @@ V1ClusterStatus, V1ClusterType, V1CreateClusterRequest, - V1InstanceSpec, V1KubernetesClusterDriver, V1ListClustersResponse, ) @@ -43,7 +42,6 @@ def test_create_cluster(api: mock.MagicMock): cluster_name="test-7", external_id="dummy", role_arn="arn:aws:iam::1234567890:role/lai-byoc", - instance_types=["t2.small"], region="us-west-2", ) @@ -59,7 +57,6 @@ def test_create_cluster(api: mock.MagicMock): region="us-west-2", role_arn="arn:aws:iam::1234567890:role/lai-byoc", external_id="dummy", - instance_types=[V1InstanceSpec(name="t2.small")], ) ) ), From e8c15d72be6033f9e0263668b0287e2309d94a07 Mon Sep 17 00:00:00 2001 From: Luca Furst Date: Tue, 25 Oct 2022 20:09:58 -0400 Subject: [PATCH 2/2] Add changelog entry --- src/lightning_app/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lightning_app/CHANGELOG.md b/src/lightning_app/CHANGELOG.md index 99631b750530b..1dce95967d46c 100644 --- a/src/lightning_app/CHANGELOG.md +++ b/src/lightning_app/CHANGELOG.md @@ -23,6 +23,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Added support for adding requirements to commands and installing them when missing when running an app command ([#15198](https://github.com/Lightning-AI/lightning/pull/15198) - Added Lightning CLI Connection to be terminal session instead of global ([#15241](https://github.com/Lightning-AI/lightning/pull/15241) +## Changed + +- Removed the `--instance-types` option when creating clusters ([#15314](https://github.com/Lightning-AI/lightning/pull/15314)) + ### Fixed - Fixed an issue when using the CLI without arguments ([#14877](https://github.com/Lightning-AI/lightning/pull/14877))