Skip to content

Commit 2a77a7d

Browse files
author
Nissan Pow
committed
fix: skip config env var processing for flows without Config parameters
When a flow with a deploy-time Config triggers another flow via DeployedFlow, the parent flow's METAFLOW_FLOW_CONFIG_VALUE env var leaked into the child flow's click API, causing either "Options were not properly set" or an AttributeError on the False-valued config_input. Guard _compute_flow_parameters so the config-resolution block only runs when the flow actually declares Config parameters. Fixes #2717
1 parent 1fb90fd commit 2a77a7d

2 files changed

Lines changed: 89 additions & 1 deletion

File tree

metaflow/runner/click_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ def _compute_flow_parameters(self):
460460
self._cached_computed_parameters = []
461461

462462
config_options = None
463-
if CLICK_API_PROCESS_CONFIG:
463+
if CLICK_API_PROCESS_CONFIG and self._config_input:
464464
with flow_context(self._flow_cls) as _:
465465
# We are going to resolve the configs first and then get the parameters.
466466
# Note that configs may update/add parameters so the order is important
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""
2+
Regression test for GitHub issue #2717.
3+
4+
When a flow with a deploy-time Config triggers another flow via DeployedFlow,
5+
the parent flow's METAFLOW_FLOW_CONFIG_VALUE environment variable was leaking
6+
into the child flow's click API initialization, causing:
7+
8+
"Options were not properly set -- this is an internal error."
9+
10+
The fix ensures that _compute_flow_parameters skips config env var processing
11+
when the flow has no config parameters (self._config_input is False).
12+
"""
13+
14+
import importlib
15+
import json
16+
import os
17+
import sys
18+
import tempfile
19+
20+
import pytest
21+
22+
23+
@pytest.fixture
24+
def fake_flow_file(tmp_path):
25+
"""Create a temporary flow file without any Config parameters."""
26+
flow_code = """\
27+
from metaflow import FlowSpec, Parameter, step
28+
29+
class FakeTargetFlow(FlowSpec):
30+
alpha = Parameter("alpha", type=str, help="A param", required=False)
31+
32+
@step
33+
def start(self):
34+
self.next(self.end)
35+
36+
@step
37+
def end(self):
38+
pass
39+
40+
if __name__ == "__main__":
41+
FakeTargetFlow()
42+
"""
43+
flow_file = tmp_path / "fake_target_flow.py"
44+
flow_file.write_text(flow_code)
45+
return str(flow_file)
46+
47+
48+
def test_no_config_flow_ignores_config_env_var(fake_flow_file, monkeypatch):
49+
"""
50+
A flow without Config parameters should not fail when
51+
METAFLOW_FLOW_CONFIG_VALUE is set in the environment (e.g., from a parent
52+
flow that does have Config).
53+
54+
This simulates the DeployedFlow trigger path: the parent flow sets
55+
METAFLOW_FLOW_CONFIG_VALUE, then DeployedFlow creates a MetaflowAPI
56+
from a fake flow file (no configs). Accessing a sub-command (like 'run')
57+
triggers _compute_flow_parameters which must not choke on the stale
58+
env var.
59+
"""
60+
# Simulate the parent flow having set config env vars
61+
monkeypatch.setenv(
62+
"METAFLOW_FLOW_CONFIG_VALUE", json.dumps({"config": {"key": "value"}})
63+
)
64+
monkeypatch.setenv("METAFLOW_FLOW_CONFIG", json.dumps({"config": "kv.config"}))
65+
66+
from metaflow.parameters import flow_context
67+
68+
# Reload CLI modules to get a clean state (same as DeployerImpl does)
69+
with flow_context(None):
70+
for module_name in [
71+
"metaflow.cli",
72+
"metaflow.cli_components.run_cmds",
73+
"metaflow.cli_components.init_cmd",
74+
]:
75+
if module_name in sys.modules:
76+
importlib.reload(sys.modules[module_name])
77+
78+
from metaflow.cli import start
79+
from metaflow.runner.click_api import MetaflowAPI
80+
81+
api_func = MetaflowAPI.from_cli(fake_flow_file, start)
82+
api = api_func()
83+
84+
# Accessing a sub-command triggers _compute_flow_parameters which
85+
# previously raised "Options were not properly set" when config env
86+
# vars from a parent flow were present but the target flow had none.
87+
run_cmd = api.run
88+
assert run_cmd is not None

0 commit comments

Comments
 (0)