From bcb57e2285787b4a48225ed6e493f6d5b9f08cfd Mon Sep 17 00:00:00 2001 From: Madhur Tandon Date: Tue, 26 Aug 2025 20:48:21 +0530 Subject: [PATCH 1/2] remove click api process config --- metaflow/metaflow_config.py | 5 - metaflow/runner/click_api.py | 120 ++++++++++----------- metaflow/runner/deployer_impl.py | 16 +-- metaflow/runner/metaflow_runner.py | 30 +----- metaflow/user_configs/config_parameters.py | 8 +- 5 files changed, 70 insertions(+), 109 deletions(-) diff --git a/metaflow/metaflow_config.py b/metaflow/metaflow_config.py index fd9e0e817d9..9578a59e789 100644 --- a/metaflow/metaflow_config.py +++ b/metaflow/metaflow_config.py @@ -533,11 +533,6 @@ # lexicographic ordering of attempts. This won't work if MAX_ATTEMPTS > 99. MAX_ATTEMPTS = 6 -# Feature flag (experimental features that are *explicitly* unsupported) - -# Process configs even when using the click_api for Runner/Deployer -CLICK_API_PROCESS_CONFIG = from_conf("CLICK_API_PROCESS_CONFIG", False) - # PINNED_CONDA_LIBS are the libraries that metaflow depends on for execution # and are needed within a conda environment diff --git a/metaflow/runner/click_api.py b/metaflow/runner/click_api.py index 3c03bda67f2..e5d987112bc 100644 --- a/metaflow/runner/click_api.py +++ b/metaflow/runner/click_api.py @@ -45,7 +45,6 @@ from metaflow.exception import MetaflowException from metaflow.flowspec import _FlowState from metaflow.includefile import FilePathClass -from metaflow.metaflow_config import CLICK_API_PROCESS_CONFIG from metaflow.parameters import JSONTypeClass, flow_context from metaflow.user_configs.config_options import ( ConfigValue, @@ -446,75 +445,72 @@ def _compute_flow_parameters(self): self._cached_computed_parameters = [] config_options = None - if CLICK_API_PROCESS_CONFIG: - with flow_context(self._flow_cls) as _: - # We are going to resolve the configs first and then get the parameters. - # Note that configs may update/add parameters so the order is important - # Since part of the processing of configs happens by click, we need to - # "fake" it. - - # Extract any config options as well as datastore and quiet options - method_params = self._chain[0][self._API_NAME] - opts = method_params["options"] - defaults = method_params["defaults"] - - ds = opts.get("datastore", defaults["datastore"]) - quiet = opts.get("quiet", defaults["quiet"]) - is_default = False - config_file = opts.get("config") - if config_file is None: - is_default = True - config_file = defaults.get("config") - - if config_file: - config_file = dict( - map( - lambda x: ( - x[0], - ConvertPath.convert_value(x[1], is_default), - ), - config_file, - ) + with flow_context(self._flow_cls) as _: + # We are going to resolve the configs first and then get the parameters. + # Note that configs may update/add parameters so the order is important + # Since part of the processing of configs happens by click, we need to + # "fake" it. + + # Extract any config options as well as datastore and quiet options + method_params = self._chain[0][self._API_NAME] + opts = method_params["options"] + defaults = method_params["defaults"] + + ds = opts.get("datastore", defaults["datastore"]) + quiet = opts.get("quiet", defaults["quiet"]) + is_default = False + config_file = opts.get("config") + if config_file is None: + is_default = True + config_file = defaults.get("config") + + if config_file: + config_file = dict( + map( + lambda x: ( + x[0], + ConvertPath.convert_value(x[1], is_default), + ), + config_file, ) + ) - is_default = False - config_value = opts.get("config-value") - if config_value is None: - is_default = True - config_value = defaults.get("config_value") - - if config_value: - config_value = dict( - map( - lambda x: ( - x[0], - ConvertDictOrStr.convert_value(x[1], is_default), - ), - config_value, - ) + is_default = False + config_value = opts.get("config-value") + if config_value is None: + is_default = True + config_value = defaults.get("config_value") + + if config_value: + config_value = dict( + map( + lambda x: ( + x[0], + ConvertDictOrStr.convert_value(x[1], is_default), + ), + config_value, ) + ) - if (config_file is None) ^ (config_value is None): - # If we have one, we should have the other - raise MetaflowException( - "Options were not properly set -- this is an internal error." - ) + if (config_file is None) ^ (config_value is None): + # If we have one, we should have the other + raise MetaflowException( + "Options were not properly set -- this is an internal error." + ) - if config_file: - # Process both configurations; the second one will return all the merged - # configuration options properly processed. - self._config_input.process_configs( - self._flow_cls.__name__, "config", config_file, quiet, ds - ) - config_options = self._config_input.process_configs( - self._flow_cls.__name__, "config_value", config_value, quiet, ds - ) + if config_file: + # Process both configurations; the second one will return all the merged + # configuration options properly processed. + self._config_input.process_configs( + self._flow_cls.__name__, "config", config_file, quiet, ds + ) + config_options = self._config_input.process_configs( + self._flow_cls.__name__, "config_value", config_value, quiet, ds + ) # At this point, we are like in start() in cli.py -- we obtained the # properly processed config_options which we can now use to process # the config decorators (including StepMutator/FlowMutator) - # Note that if CLICK_API_PROCESS_CONFIG is False, we still do this because - # it will init all parameters (config_options will be None) # We ignore any errors if we don't check the configs in the click API. # Init all values in the flow mutators and then process them @@ -522,7 +518,7 @@ def _compute_flow_parameters(self): decorator.external_init() new_cls = self._flow_cls._process_config_decorators( - config_options, process_configs=CLICK_API_PROCESS_CONFIG + config_options, process_configs=True ) if new_cls: self._flow_cls = new_cls diff --git a/metaflow/runner/deployer_impl.py b/metaflow/runner/deployer_impl.py index f0f8143f25b..04121373417 100644 --- a/metaflow/runner/deployer_impl.py +++ b/metaflow/runner/deployer_impl.py @@ -5,8 +5,6 @@ from typing import Any, ClassVar, Dict, Optional, TYPE_CHECKING, Type, List -from metaflow.metaflow_config import CLICK_API_PROCESS_CONFIG - from .subprocess_manager import SubprocessManager from .utils import get_lower_level_group, handle_timeout, temporary_fifo, with_dir @@ -150,16 +148,10 @@ def _create( ) -> "metaflow.runner.deployer.DeployedFlow": with temporary_fifo() as (attribute_file_path, attribute_file_fd): # every subclass needs to have `self.deployer_kwargs` - # TODO: Get rid of CLICK_API_PROCESS_CONFIG in the near future - if CLICK_API_PROCESS_CONFIG: - # We need to run this in the cwd because configs depend on files - # that may be located in paths relative to the directory the user - # wants to run in - with with_dir(self.cwd): - command = get_lower_level_group( - self.api, self.top_level_kwargs, self.TYPE, self.deployer_kwargs - ).create(deployer_attribute_file=attribute_file_path, **kwargs) - else: + # We need to run this in the cwd because configs depend on files + # that may be located in paths relative to the directory the user + # wants to run in + with with_dir(self.cwd): command = get_lower_level_group( self.api, self.top_level_kwargs, self.TYPE, self.deployer_kwargs ).create(deployer_attribute_file=attribute_file_path, **kwargs) diff --git a/metaflow/runner/metaflow_runner.py b/metaflow/runner/metaflow_runner.py index 84eb8ac4284..da9313dd4ba 100644 --- a/metaflow/runner/metaflow_runner.py +++ b/metaflow/runner/metaflow_runner.py @@ -8,8 +8,6 @@ from metaflow import Run -from metaflow.metaflow_config import CLICK_API_PROCESS_CONFIG - from metaflow.plugins import get_runner_cli from .utils import ( @@ -377,12 +375,7 @@ def run(self, **kwargs) -> ExecutingRun: ExecutingRun containing the results of the run. """ with temporary_fifo() as (attribute_file_path, attribute_file_fd): - if CLICK_API_PROCESS_CONFIG: - with with_dir(self.cwd): - command = self.api(**self.top_level_kwargs).run( - runner_attribute_file=attribute_file_path, **kwargs - ) - else: + with with_dir(self.cwd): command = self.api(**self.top_level_kwargs).run( runner_attribute_file=attribute_file_path, **kwargs ) @@ -414,12 +407,7 @@ def resume(self, **kwargs) -> ExecutingRun: ExecutingRun containing the results of the resumed run. """ with temporary_fifo() as (attribute_file_path, attribute_file_fd): - if CLICK_API_PROCESS_CONFIG: - with with_dir(self.cwd): - command = self.api(**self.top_level_kwargs).resume( - runner_attribute_file=attribute_file_path, **kwargs - ) - else: + with with_dir(self.cwd): command = self.api(**self.top_level_kwargs).resume( runner_attribute_file=attribute_file_path, **kwargs ) @@ -453,12 +441,7 @@ async def async_run(self, **kwargs) -> ExecutingRun: ExecutingRun representing the run that was started. """ with temporary_fifo() as (attribute_file_path, attribute_file_fd): - if CLICK_API_PROCESS_CONFIG: - with with_dir(self.cwd): - command = self.api(**self.top_level_kwargs).run( - runner_attribute_file=attribute_file_path, **kwargs - ) - else: + with with_dir(self.cwd): command = self.api(**self.top_level_kwargs).run( runner_attribute_file=attribute_file_path, **kwargs ) @@ -491,12 +474,7 @@ async def async_resume(self, **kwargs) -> ExecutingRun: ExecutingRun representing the resumed run that was started. """ with temporary_fifo() as (attribute_file_path, attribute_file_fd): - if CLICK_API_PROCESS_CONFIG: - with with_dir(self.cwd): - command = self.api(**self.top_level_kwargs).resume( - runner_attribute_file=attribute_file_path, **kwargs - ) - else: + with with_dir(self.cwd): command = self.api(**self.top_level_kwargs).resume( runner_attribute_file=attribute_file_path, **kwargs ) diff --git a/metaflow/user_configs/config_parameters.py b/metaflow/user_configs/config_parameters.py index c9982316d20..28507e041d8 100644 --- a/metaflow/user_configs/config_parameters.py +++ b/metaflow/user_configs/config_parameters.py @@ -569,10 +569,10 @@ def resolve_delayed_evaluator( if ignore_errors: # Assumption is that default value of None is always allowed. # This code path is *only* used when evaluating Parameters AND they - # use configs in their attributes AND the runner/deployer is being used - # AND CLICK_API_PROCESS_CONFIG is False. In those cases, all attributes in - # Parameter can be set to None except for required and show_default - # and even in those cases, a wrong value will have very limited consequence. + # use configs in their attributes AND the runner/deployer is being used. + # In those cases, all attributes in Parameter can be set to None except + # for required and show_default and even in those cases, a wrong value + # will have very limited consequence. return None raise e From 49e696bbbb8ecb29e69d4f45849681ea6325629e Mon Sep 17 00:00:00 2001 From: Madhur Tandon Date: Tue, 26 Aug 2025 21:22:40 +0530 Subject: [PATCH 2/2] fix filename --- test/core/tests/basic_config_parameters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/tests/basic_config_parameters.py b/test/core/tests/basic_config_parameters.py index 5680e504cc6..f3aa1ed4427 100644 --- a/test/core/tests/basic_config_parameters.py +++ b/test/core/tests/basic_config_parameters.py @@ -28,7 +28,7 @@ class BasicConfigTest(MetaflowTest): "silly_config": { "required": True, "parser": "silly_parser", - "default": "'silly.txt'", + "default": "'basic_config_silly.txt'", }, "config2": {}, # Test using a function to get the value