Skip to content

Commit

Permalink
Fix case where the decorator's init function could be called twice. (N…
Browse files Browse the repository at this point in the history
…etflix#2167)

In some cases, specifically when using argo/airflow/sfn, the decorator's
init function could be called twice which caused issues with the conda/pypi
decorator.
  • Loading branch information
romain-intel authored Dec 8, 2024
1 parent b782b9e commit e2114f5
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 11 deletions.
2 changes: 1 addition & 1 deletion metaflow/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ def start(
)
if all_decospecs:
decorators._attach_decorators(ctx.obj.flow, all_decospecs)
decorators._init(ctx.obj.flow, only_non_static=True)
decorators._init(ctx.obj.flow)
# Regenerate graph if we attached more decorators
ctx.obj.graph = FlowGraph(ctx.obj.flow.__class__)

Expand Down
2 changes: 1 addition & 1 deletion metaflow/cli_components/run_cmds.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def before_run(obj, tags, decospecs):
)
if all_decospecs:
decorators._attach_decorators(obj.flow, all_decospecs)
decorators._init(obj.flow, only_non_static=True)
decorators._init(obj.flow)
obj.graph = FlowGraph(obj.flow.__class__)

obj.check(obj.graph, obj.flow, obj.environment, pylint=obj.pylint)
Expand Down
2 changes: 1 addition & 1 deletion metaflow/cli_components/step_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def step(

if decospecs:
decorators._attach_decorators_to_step(func, decospecs)
decorators._init(ctx.obj.flow, only_non_static=True)
decorators._init(ctx.obj.flow)

step_kwargs = ctx.params
# Remove argument `step_name` from `step_kwargs`.
Expand Down
17 changes: 13 additions & 4 deletions metaflow/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@
unicode = str
basestring = str

# Contains the decorators on which _init was called. We want to ensure it is called
# only once on each decorator and, as the _init() function below can be called in
# several places, we need to track which decorator had their init function called
_inited_decorators = set()


class BadStepDecoratorException(MetaflowException):
headline = "Syntax error"
Expand Down Expand Up @@ -553,12 +558,16 @@ def _attach_decorators_to_step(step, decospecs):
def _init(flow, only_non_static=False):
for decorators in flow._flow_decorators.values():
for deco in decorators:
if not only_non_static or not deco.statically_defined:
deco.init()
if deco in _inited_decorators:
continue
deco.init()
_inited_decorators.add(deco)
for flowstep in flow:
for deco in flowstep.decorators:
if not only_non_static or not deco.statically_defined:
deco.init()
if deco in _inited_decorators:
continue
deco.init()
_inited_decorators.add(deco)


def _init_flow_decorators(
Expand Down
2 changes: 1 addition & 1 deletion metaflow/plugins/airflow/airflow_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ def make_flow(
):
# Attach @kubernetes.
decorators._attach_decorators(obj.flow, [KubernetesDecorator.name])
decorators._init(obj.flow, only_non_static=True)
decorators._init(obj.flow)

decorators._init_step_decorators(
obj.flow, obj.graph, obj.environment, obj.flow_datastore, obj.logger
Expand Down
2 changes: 1 addition & 1 deletion metaflow/plugins/argo/argo_workflows_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ def make_flow(
decorators._attach_decorators(
obj.flow, [KubernetesDecorator.name, EnvironmentDecorator.name]
)
decorators._init(obj.flow, only_non_static=True)
decorators._init(obj.flow)

decorators._init_step_decorators(
obj.flow, obj.graph, obj.environment, obj.flow_datastore, obj.logger
Expand Down
2 changes: 1 addition & 1 deletion metaflow/plugins/aws/step_functions/step_functions_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def make_flow(

# Attach AWS Batch decorator to the flow
decorators._attach_decorators(obj.flow, [BatchDecorator.name])
decorators._init(obj.flow, only_non_static=True)
decorators._init(obj.flow)
decorators._init_step_decorators(
obj.flow, obj.graph, obj.environment, obj.flow_datastore, obj.logger
)
Expand Down
2 changes: 1 addition & 1 deletion metaflow/plugins/pypi/pypi_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def flow_init(
from metaflow import decorators

decorators._attach_decorators(flow, ["pypi"])
decorators._init(flow, only_non_static=True)
decorators._init(flow)

# @pypi uses a conda environment to create a virtual environment.
# The conda environment can be created through micromamba.
Expand Down

0 comments on commit e2114f5

Please sign in to comment.