Skip to content

Commit

Permalink
Move from --<configname> <config> to --config <configname> <config>
Browse files Browse the repository at this point in the history
  • Loading branch information
romain-intel committed Aug 22, 2024
1 parent 99d06ae commit de97829
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 127 deletions.
23 changes: 9 additions & 14 deletions metaflow/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
resolve_identity,
write_latest_run_id,
)
from .user_configs import LocalFileInput
from .user_configs import LocalFileInput, config_options

ERASE_TO_EOL = "\033[K"
HIGHLIGHT = "red"
Expand Down Expand Up @@ -845,7 +845,10 @@ def version(obj):


@tracing.cli_entrypoint("cli/start")
@decorators.add_decorator_and_config_options
# NOTE: add_decorator_options should be TL because it checks to make sure
# that no option conflict with the ones below
@decorators.add_decorator_options
@config_options
@click.command(
cls=click.CommandCollection,
sources=[cli] + plugins.get_plugin_cli(),
Expand Down Expand Up @@ -936,7 +939,8 @@ def start(
event_logger=None,
monitor=None,
local_info_file=None,
**deco_and_config_options
config_options=None,
**deco_options
):
global echo
if quiet:
Expand All @@ -957,7 +961,7 @@ def start(
# process all those decorators that the user added that will modify the flow based
# on those configurations. It is important to do this as early as possible since it
# actually modifies the flow itself
ctx.obj.flow = ctx.obj.flow._process_config_funcs(deco_and_config_options)
ctx.obj.flow = ctx.obj.flow._process_config_funcs(config_options)

cli_args._set_top_kwargs(ctx.params)
ctx.obj.echo = echo
Expand Down Expand Up @@ -1015,16 +1019,7 @@ def start(
ctx.obj.monitor,
)

ctx.obj.config_options = {
k: v
for k, v in deco_and_config_options.items()
if k in ctx.command.config_options
}
deco_options = {
k: v
for k, v in deco_and_config_options.items()
if k not in ctx.command.config_options
}
ctx.obj.config_options = config_options

decorators._resolve_configs(ctx.obj.flow)

Expand Down
27 changes: 6 additions & 21 deletions metaflow/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,27 +227,13 @@ def get_top_level_options(self):


# compare this to parameters.add_custom_parameters
def add_decorator_and_config_options(cmd):
config_seen = {}
def add_decorator_options(cmd):
flow_cls = getattr(current_flow, "flow_cls", None)
if flow_cls is None:
return cmd

parameters = [p for _, p in flow_cls._get_parameters() if p.IS_FLOW_PARAMETER]
# Add configuration options
for arg in parameters[::-1]:
kwargs = arg.option_kwargs(False)
if arg.name in config_seen:
msg = (
"Multiple configurations use the same name '%s'. Please change the "
"names of some of your configurations" % arg.name
)
raise MetaflowException(msg)
config_seen[arg.name] = arg
cmd.params.insert(0, click.Option(("--" + arg.name,), **kwargs))

cmd.config_options = set(config_seen.keys())
seen = {}
existing_params = set(p.name.lower() for p in cmd.params)
# Add decorator options
for deco in flow_decorators(flow_cls):
for option, kwargs in deco.options.items():
Expand All @@ -259,11 +245,10 @@ def add_decorator_and_config_options(cmd):
% (deco.name, option, seen[option])
)
raise MetaflowInternalError(msg)
elif option in config_seen:
msg = (
"Flow decorator '%s' uses an option '%s' which is also "
"used by a configuration. Please change the name of the "
"configuration" % (deco.name, option)
elif deco.name.lower() in existing_params:
raise MetaflowInternalError(
"Flow decorator '%s' uses an option '%s' which is a reserved "
"keyword. Please use a different option name." % (deco.name, option)
)
else:
kwargs["envvar"] = "METAFLOW_FLOW_%s" % option.upper()
Expand Down
8 changes: 5 additions & 3 deletions metaflow/runner/click_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
UUIDParameterType,
)
from metaflow._vendor.typeguard import TypeCheckError, check_type
from metaflow.decorators import add_decorator_and_config_options
from metaflow.decorators import add_decorator_options
from metaflow.exception import MetaflowException
from metaflow.includefile import FilePathClass
from metaflow.parameters import JSONTypeClass, flow_context
Expand Down Expand Up @@ -184,9 +184,11 @@ def chain(self):
@classmethod
def from_cli(cls, flow_file: str, cli_collection: Callable) -> Callable:
flow_cls = extract_flow_class_from_file(flow_file)
flow_parameters = [p for _, p in flow_cls._get_parameters()]
flow_parameters = [
p for _, p in flow_cls._get_parameters() if not p.IS_FLOW_PARAMETER
]
with flow_context(flow_cls) as _:
add_decorator_and_config_options(cli_collection)
add_decorator_options(cli_collection)

class_dict = {"__module__": "metaflow", "_API_NAME": flow_file}
command_groups = cli_collection.sources
Expand Down
11 changes: 7 additions & 4 deletions metaflow/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -1468,9 +1468,10 @@ def __init__(self, task):
# We also pass configuration options using the kv.<name> syntax which will cause
# the configuration options to be loaded from the INFO file (or local-info-file
# in the case of the local runtime)
self.top_level_options.update(
{k: ConfigInput.make_key_name(k) for k in self.task.flow._user_configs}
)
if self.task.flow._user_configs:
self.top_level_options["config"] = [
(k, ConfigInput.make_key_name(k)) for k in self.task.flow._user_configs
]

self.commands = ["step"]
self.command_args = [self.task.step]
Expand Down Expand Up @@ -1505,7 +1506,9 @@ def _options(mapping):
for value in v:
yield "--%s" % k
if not isinstance(value, bool):
yield to_unicode(value)
value = value if isinstance(value, tuple) else (value,)
for vv in value:
yield to_unicode(vv)

args = list(self.entrypoint)
args.extend(_options(self.top_level_options))
Expand Down
Loading

0 comments on commit de97829

Please sign in to comment.