Skip to content

Commit

Permalink
Fixing command line interface for the run command. (#348)
Browse files Browse the repository at this point in the history 
This commit fixes bugs associated with the MLCube `run` command:
- It now uses correct option decorators from the `cli` module (class `Options`) instead of deprecated decorators from the `__main__.py` file.
- It now uses the `-P` option to correctly parse MLCube parameters that users provide on a command line.

The latter particularly fixes the #347 issue.
  • Loading branch information
@sergey-serebryakov
sergey-serebryakov authored Dec 11, 2023
1 parent 9881ff6 commit 4938fe6
Showing 1 changed file with 34 additions and 73 deletions.
107 changes: 34 additions & 73 deletions mlcube/mlcube/__main__.py
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
"""This requires the MLCube 2.0 that's located somewhere in one of dev branches."""
import logging
import os
from pathlib import Path
import shutil
import sys
import typing as t
from pathlib import Path

import click
import coloredlogs
Expand Down Expand Up @@ -41,9 +41,7 @@ def parser_process(value: str, state: click.parser.ParsingState):

super(MultiValueOption, self).add_to_parser(parser, ctx)
for opt_name in self.opts:
our_parser: t.Optional[click.parser.Option] = parser._long_opt.get(
opt_name
) or parser._short_opt.get(opt_name)
our_parser: t.Optional[click.parser.Option] = parser._long_opt.get(opt_name) or parser._short_opt.get(opt_name)
if our_parser:
self._eat_all_parser = our_parser
self._previous_parser_process = our_parser.process
Expand Down Expand Up @@ -238,9 +236,7 @@ def configure(mlcube: t.Optional[str], platform: str, p: t.Tuple[str]) -> None:
p: Additional MLCube configuration parameters (these parameters are those parameters that normally start with
`-P` prefix). Here, due to original implementation, we need to `unparse` by adding `-P` prefix.
"""
logger.debug(
"mlcube::configure, mlcube=%s, platform=%s, p=%s", mlcube, platform, str(p)
)
logger.debug("mlcube::configure, mlcube=%s, platform=%s, p=%s", mlcube, platform, str(p))
if mlcube is None:
mlcube = os.getcwd()
logger.info(
Expand Down Expand Up @@ -280,16 +276,17 @@ def configure(mlcube: t.Optional[str], platform: str, p: t.Tuple[str]) -> None:
"max_content_width": _TERMINAL_WIDTH,
},
)
@mlcube_option
@platform_option
@task_option
@workspace_option
@Options.mlcube
@Options.platform
@Options.task
@Options.workspace
@network_option
@security_option
@gpus_option
@memory_option
@cpu_option
@mount_option
@Options.parameter
@Options.help
@click.pass_context
def run(
Expand All @@ -304,6 +301,7 @@ def run(
memory: str,
cpu: str,
mount: str,
p: t.Tuple[str],
) -> None:
"""Run MLCube task(s).
Expand All @@ -321,11 +319,12 @@ def run(
cpu: CPU options defined during MLCube container execution.
mount: Mount (global) options defined for all input parameters in all tasks to be executed. They override any
mount options defined for individual parameters.
p: Additional MLCube configuration parameters (these parameters are those parameters that normally start with
`-P` prefix). Here, due to original implementation, we need to `unparse` by adding `-P` prefix.
"""
logger.info(
"run input_arg mlcube=%s, platform=%s, task=%s, workspace=%s, network=%s, security=%s, gpus=%s, "
"memory=%s, mount=%s"
"cpu=%s",
"memory=%s, mount=%s, cpu=%s, p=%s",
mlcube,
platform,
task,
Expand All @@ -336,9 +335,10 @@ def run(
memory,
cpu,
mount,
str(p),
)
runner_cls, mlcube_config = parse_cli_args(
unparsed_args=ctx.args,
unparsed_args=ctx.args + ["-P" + param for param in p],
parsed_args={
"mlcube": mlcube,
"platform": platform,
Expand All @@ -352,17 +352,11 @@ def run(
},
resolve=True,
)
mlcube_tasks: t.List[str] = list(
(mlcube_config.get("tasks", None) or {}).keys()
) # Tasks in this MLCube.
tasks: t.List[str] = CliParser.parse_list_arg(
task, default=None
) # Requested tasks.
mlcube_tasks: t.List[str] = list((mlcube_config.get("tasks", None) or {}).keys()) # Tasks in this MLCube.
tasks: t.List[str] = CliParser.parse_list_arg(task, default=None) # Requested tasks.

if len(tasks) == 0:
logger.warning(
"Missing required task name (--task=COMMA_SEPARATED_LIST_OF_TASK_NAMES)."
)
logger.warning("Missing required task name (--task=COMMA_SEPARATED_LIST_OF_TASK_NAMES).")
if len(mlcube_tasks) != 1:
logger.error(
"Task name could not be automatically resolved (supported tasks = %s).",
Expand All @@ -379,8 +373,7 @@ def run(
unknown_tasks: t.List[str] = [name for name in tasks if name not in mlcube_tasks]
if len(unknown_tasks) > 0:
logger.error(
"Unknown tasks have been requested: supported tasks = %s, requested tasks = %s, "
"unknown tasks = %s.",
"Unknown tasks have been requested: supported tasks = %s, requested tasks = %s, " "unknown tasks = %s.",
str(mlcube_tasks),
str(tasks),
str(unknown_tasks),
Expand Down Expand Up @@ -419,9 +412,7 @@ def describe(mlcube: t.Optional[str]) -> None:
"""
if mlcube is None:
mlcube = os.getcwd()
_, mlcube_config = parse_cli_args(
unparsed_args=[], parsed_args={"mlcube": mlcube}, resolve=True
)
_, mlcube_config = parse_cli_args(unparsed_args=[], parsed_args={"mlcube": mlcube}, resolve=True)
print("MLCube")
print(f" path = {mlcube_config.runtime.root}")
print(f" name = {mlcube_config.name}:{mlcube_config.get('version', 'latest')}")
Expand All @@ -433,25 +424,17 @@ def describe(mlcube: t.Optional[str]) -> None:
for task_name, task_def in mlcube_config.tasks.items():
description = f"name = {task_name}"
if len(task_def.parameters.inputs) > 0:
description = (
f"{description}, inputs = {list(task_def.parameters.inputs.keys())}"
)
description = f"{description}, inputs = {list(task_def.parameters.inputs.keys())}"
if len(task_def.parameters.outputs) > 0:
description = (
f"{description}, outputs = {list(task_def.parameters.outputs.keys())}"
)
description = f"{description}, outputs = {list(task_def.parameters.outputs.keys())}"
print(f" {description}")
print()
print("Run this MLCube:")
print(" Configure MLCube:")
print(
f" mlcube configure --mlcube={mlcube_config.runtime.root} --platform=docker"
)
print(f" mlcube configure --mlcube={mlcube_config.runtime.root} --platform=docker")
print(" Run MLCube tasks:")
for task_name in mlcube_config.tasks.keys():
print(
f" mlcube run --mlcube={mlcube_config.runtime.root} --task={task_name} --platform=docker"
)
print(f" mlcube run --mlcube={mlcube_config.runtime.root} --task={task_name} --platform=docker")
print()


Expand Down Expand Up @@ -543,19 +526,11 @@ def config(
ctx: click.core.Context,
list_all: bool, # mlcube config --list
get: t.Optional[str], # mlcube config --get KEY
create_platform: t.Optional[
t.Tuple
], # mlcube config --create-platform RUNNER PLATFORM
create_platform: t.Optional[t.Tuple], # mlcube config --create-platform RUNNER PLATFORM
remove_platform: t.Optional[str], # mlcube config --remove-platform NAME
rename_platform: t.Optional[
t.Tuple
], # mlcube config --rename-platform OLD_NAME NEW_NAME
copy_platform: t.Optional[
t.Tuple
], # mlcube config --copy-platform EXISTING_PLATFORM NEW_PLATFORM
rename_runner: t.Optional[
t.Tuple
], # mlcube config --rename-runner OLD_NAME NEW_NAME
rename_platform: t.Optional[t.Tuple], # mlcube config --rename-platform OLD_NAME NEW_NAME
copy_platform: t.Optional[t.Tuple], # mlcube config --copy-platform EXISTING_PLATFORM NEW_PLATFORM
rename_runner: t.Optional[t.Tuple], # mlcube config --rename-runner OLD_NAME NEW_NAME
remove_runner: t.Optional[str], # mlcube config --remove-runner NAME
) -> None:
"""Display or change MLCube system settings.
Expand All @@ -570,49 +545,35 @@ def config(
print(f"System settings file path = {SystemSettings.system_settings_file()}")
settings = SystemSettings()

def _check_tuple(
_tuple: t.Tuple, _name: str, _expected_size: int, _expected_value: str
) -> None:
def _check_tuple(_tuple: t.Tuple, _name: str, _expected_size: int, _expected_value: str) -> None:
if len(_tuple) != _expected_size:
raise IllegalParameterValueError(
f"--{_name}", " ".join(_tuple), f'"{_expected_value}"'
)
raise IllegalParameterValueError(f"--{_name}", " ".join(_tuple), f'"{_expected_value}"')

try:
if list_all:
print(OmegaConf.to_yaml(settings.settings))
elif get:
print(OmegaConf.to_yaml(OmegaConf.select(settings.settings, get)))
elif create_platform:
_check_tuple(
create_platform, "create_platform", 2, "RUNNER_NAME PLATFORM_NAME"
)
_check_tuple(create_platform, "create_platform", 2, "RUNNER_NAME PLATFORM_NAME")
settings.create_platform(create_platform)
elif remove_platform:
settings.remove_platform(remove_platform)
elif rename_platform:
_check_tuple(rename_platform, "rename_platform", 2, "OLD_NAME NEW_NAME")
settings.copy_platform(rename_platform, delete_source=True)
elif copy_platform:
_check_tuple(
copy_platform, "copy_platform", 2, "EXISTING_PLATFORM NEW_PLATFORM"
)
_check_tuple(copy_platform, "copy_platform", 2, "EXISTING_PLATFORM NEW_PLATFORM")
settings.copy_platform(rename_platform, delete_source=False)
elif rename_runner:
_check_tuple(rename_runner, "rename_runner", 2, "OLD_NAME NEW_NAME")
update_platforms: bool = (
"--update-platforms" in ctx.args or "--update_platforms" in ctx.args
)
update_platforms: bool = "--update-platforms" in ctx.args or "--update_platforms" in ctx.args
settings.rename_runner(rename_runner, update_platforms=update_platforms)
elif remove_runner:
remove_platforms: bool = (
"--remove-platforms" in ctx.args or "--remove_platforms" in ctx.args
)
remove_platforms: bool = "--remove-platforms" in ctx.args or "--remove_platforms" in ctx.args
settings.remove_runner(remove_runner, remove_platforms=remove_platforms)
except MLCubeError as e:
logger.error(
"Command failed, command = '%s' error = '%s'", " ".join(sys.argv), str(e)
)
logger.error("Command failed, command = '%s' error = '%s'", " ".join(sys.argv), str(e))


@cli.command(
Expand Down

0 comments on commit 4938fe6

Please sign in to comment.