Skip to content

Commit

Permalink
Fixed deploy command failure (#1915)
Browse files Browse the repository at this point in the history
* fixed deploy commands failing in case of non-deployable plugin installation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fixed cli test

* fixed cli test

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
kessler-frost and pre-commit-ci[bot] authored Jan 21, 2024
1 parent 5700337 commit 3e8aa09
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 35 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Removed unused file transfer how to guides
- Removed `pennylane` as a requirement from notebooks' requirements.txt as it comes with `covalent`
- Removed `validate_args` and `validate_region` method from `deploy_group` CLI as they were specific to AWS

### Docs

- Added voice cloning tutorial

### Fixed

- Fixed the scenario where any deploy commands would fail if the user had a non deploy compatible plugin installed

## [0.233.0-rc.0] - 2024-01-07

### Authors
Expand Down
54 changes: 29 additions & 25 deletions covalent_dispatcher/_cli/groups/deploy_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from pathlib import Path
from typing import Callable, Dict, Tuple

import boto3
import click
from rich.console import Console
from rich.table import Table
Expand Down Expand Up @@ -176,12 +175,20 @@ def up(executor_name: str, vars: Dict, help: bool, dry_run: bool, verbose: bool)
$ covalent deploy up awslambda --verbose --region=us-east-1 --instance-type=t2.micro
"""

cmd_options = {key[2:]: value for key, value in (var.split("=") for var in vars)}
if msg := validate_args(cmd_options):
# Message is not None, so there was an error.
click.echo(msg)

try:
crm = get_crm_object(executor_name, cmd_options)
except (KeyError, AttributeError):
click.echo(
click.style(
f"Warning: '{executor_name}' is not a valid executor for deployment.",
fg="yellow",
)
)
sys.exit(1)
crm = get_crm_object(executor_name, cmd_options)

if help:
click.echo(Console().print(get_up_help_table(crm)))
sys.exit(0)
Expand Down Expand Up @@ -212,7 +219,18 @@ def down(executor_name: str, verbose: bool) -> None:
$ covalent deploy down ecs --verbose
"""
crm = get_crm_object(executor_name)

try:
crm = get_crm_object(executor_name)
except (KeyError, AttributeError):
click.echo(
click.style(
f"Warning: '{executor_name}' is not a valid executor for deployment.",
fg="yellow",
)
)
sys.exit(1)

_command = partial(crm.down)
_run_command_and_show_output(_command, "Destroying resources...", verbose=verbose)

Expand Down Expand Up @@ -247,7 +265,7 @@ def status(executor_names: Tuple[str]) -> None:
for name in _executor_manager.executor_plugins_map
if name not in ["dask", "local", "remote_executor"]
]
click.echo(f"Executors: {', '.join(executor_names)}")
click.echo(f"Installed executors: {', '.join(executor_names)}")

table = Table()
table.add_column("Executor", justify="center")
Expand All @@ -260,31 +278,17 @@ def status(executor_names: Tuple[str]) -> None:
crm = get_crm_object(executor_name)
crm_status = crm.status()
table.add_row(executor_name, crm_status, description[crm_status])
except KeyError:
except (KeyError, AttributeError):
# Added the AttributeError here as well in case the executor does not
# have the ExecutorPluginDefaults or ExecutorInfraDefaults classes.
invalid_executor_names.append(executor_name)

click.echo(Console().print(table))

if invalid_executor_names:
click.echo(
click.style(
f"Warning: {', '.join(invalid_executor_names)} are not valid executors.",
f"Warning: Invalid executors for deployment -> '{', '.join(invalid_executor_names)}'",
fg="yellow",
)
)


def validate_args(args: dict):
message = None
if len(args) == 0:
return message
if "region" in args and args["region"] != "":
if not validate_region(args["region"]):
return f"Unable to find the provided region: {args['region']}"


def validate_region(region_name: str):
ec2_client = boto3.client("ec2")
response = ec2_client.describe_regions()
exists = region_name in [item["RegionName"] for item in response["Regions"]]
return exists
21 changes: 11 additions & 10 deletions tests/covalent_dispatcher_tests/_cli/cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,22 +184,14 @@ def test_deploy_up(mocker):
"covalent_dispatcher._cli.groups.deploy_group._run_command_and_show_output",
)

# Fail with invalid command options.
mocker.patch(
"covalent_dispatcher._cli.groups.deploy_group.validate_args",
return_value="Non-empty msg",
)
# Fail with invalid executor name
with pytest.raises(SystemExit) as exc_info:
ctx = click.Context(up)
ctx.invoke(up)
ctx.invoke(up, executor_name="invalid")

assert exc_info.value.code == 1

# Succeed but exit after help message.
mocker.patch(
"covalent_dispatcher._cli.groups.deploy_group.validate_args",
return_value=None,
)
mocker.patch(
"covalent_dispatcher._cli.groups.deploy_group.get_crm_object",
)
Expand Down Expand Up @@ -227,6 +219,15 @@ def test_deploy_down(mocker):
mock_run_command_and_show_output = mocker.patch(
"covalent_dispatcher._cli.groups.deploy_group._run_command_and_show_output",
)

# Fail with invalid executor name
with pytest.raises(SystemExit) as exc_info:
ctx = click.Context(down)
ctx.invoke(down, executor_name="invalid")

assert exc_info.value.code == 1

# Succeed with valid command options.
mocker.patch(
"covalent_dispatcher._cli.groups.deploy_group.get_crm_object",
)
Expand Down

0 comments on commit 3e8aa09

Please sign in to comment.