Per #2299, update check for deprecated MET config env vars by using t…

…he list of deprecated env vars found in each wrapper to determine the list to check. I had to move _get_wrapper_instance to another util file so that it can be imported by config_validate and run_util without creating a circular dependency

internal/tests/pytests/util/run_util/test_run_util.py

@@ -2,6 +2,7 @@
 import pytest
 from unittest import mock
 import metplus.util.run_util as ru
+import metplus.util.wrapper_init as wi
 from metplus.wrappers.ensemble_stat_wrapper import EnsembleStatWrapper
 from metplus.wrappers.grid_stat_wrapper import GridStatWrapper
 
@@ -227,8 +228,8 @@ def test_run_metplus_errors(capfd, side_effect, return_value, check_err):
 
 
 @pytest.mark.util
-def test__get_wrapper_instance(metplus_config):
-    actual = ru._get_wrapper_instance(metplus_config, 'EnsembleStat', instance=2)
+def test_get_wrapper_instance(metplus_config):
+    actual = wi.get_wrapper_instance(metplus_config, 'EnsembleStat', instance=2)
     assert isinstance(actual, EnsembleStatWrapper)
     assert actual.instance == 2
 
@@ -241,10 +242,10 @@ def test__get_wrapper_instance(metplus_config):
     ],
 )
 @pytest.mark.util
-def test__get_wrapper_instance_raises(capfd, side_effect, check_err):
+def test_get_wrapper_instance_raises(capfd, side_effect, check_err):
     config = get_config_from_file()
-    with mock.patch.object(ru, 'import_module', side_effect=side_effect):
-        actual = ru._get_wrapper_instance(config, 'EnsembleStat')
+    with mock.patch.object(wi, 'import_module', side_effect=side_effect):
+        actual = wi.get_wrapper_instance(config, 'EnsembleStat')
     assert actual == None
     out, err = capfd.readouterr()
     assert check_err in err

metplus/util/__init__.py

@@ -6,6 +6,7 @@
 from .string_template_substitution import *
 from .config_util import *
 from .config_metplus import *
+from .wrapper_init import *
 from .config_validate import *
 from .run_util import *
 from .met_config import *

metplus/util/config_validate.py

@@ -3,10 +3,10 @@
 from .constants import DEPRECATED_DICT, DEPRECATED_MET_LIST
 from .constants import UPGRADE_INSTRUCTIONS_URL
 from .string_manip import find_indices_in_config_section, getlist
-from .string_manip import is_python_script
+from .string_manip import is_python_script, get_wrapper_name
 from .string_template_substitution import do_string_sub
 from .config_util import get_process_list, get_custom_string_list
-
+from .wrapper_init import get_wrapper_instance
 
 def validate_config_variables(config):
 
@@ -154,23 +154,23 @@ def check_for_deprecated_met_config(config):
                 continue
 
             met_config_file = do_string_sub(met_config, custom=custom_string)
-
             if not check_for_deprecated_met_config_file(config,
-                                                        met_config_file):
+                                                        met_config_file,
+                                                        met_tool):
                 all_good = False
 
     return all_good, sed_cmds
 
 
-def check_for_deprecated_met_config_file(config, met_config):
+def check_for_deprecated_met_config_file(config, met_config, met_tool):
 
     all_good = True
     if not os.path.exists(met_config):
         config.logger.error(f"Config file does not exist: {met_config}")
         return False
 
-    # skip check if no deprecated variables are set
-    if not DEPRECATED_MET_LIST:
+    deprecated_met_list = _get_deprecated_met_list(config, met_tool)
+    if not deprecated_met_list:
         return all_good
 
     config.logger.debug("Checking for deprecated environment "
@@ -180,17 +180,35 @@ def check_for_deprecated_met_config_file(config, met_config):
         lines = file_handle.read().splitlines()
 
     for line in lines:
-        for deprecated_item in DEPRECATED_MET_LIST:
+        for deprecated_item in deprecated_met_list:
             if '${' + deprecated_item + '}' not in line:
                 continue
             all_good = False
-            config.logger.error("Please remove deprecated environment variable"
+            config.logger.error("Deprecated environment variable"
                                 f" ${{{deprecated_item}}} found in MET config "
-                                f"file: {met_config}")
+                                f"file: {met_config}. Please unset "
+                                f"{met_tool}_CONFIG_FILE to use the wrapped "
+                                "MET config that is provided with the "
+                                "METplus wrappers.")
 
     return all_good
 
 
+def _get_deprecated_met_list(config, met_tool):
+    wrapper_name = get_wrapper_name(met_tool)
+    if not wrapper_name:
+        return None
+
+    wrapper = get_wrapper_instance(config, wrapper_name)
+    if not wrapper:
+        return None
+
+    if not hasattr(wrapper, 'DEPRECATED_WRAPPER_ENV_VAR_KEYS'):
+        return None
+
+    return wrapper.DEPRECATED_WRAPPER_ENV_VAR_KEYS
+
+
 def validate_field_info_configs(config):
     """!Verify that config variables with _VAR<n>_ in them are valid.
 

metplus/util/run_util.py

@@ -13,7 +13,7 @@
 from .config_validate import validate_config_variables
 from .. import get_metplus_version
 from .config_metplus import setup
-from . import camel_to_underscore
+from . import camel_to_underscore, get_wrapper_instance
 
 
 def pre_run_setup(config_inputs):
@@ -105,7 +105,7 @@ def run_metplus(config):
     try:
         # if Usage is in process list, run it and exit
         if 'Usage' in [p[0] for p in process_list]:
-            wrapper = _get_wrapper_instance(config, 'Usage')
+            wrapper = get_wrapper_instance(config, 'Usage')
             wrapper.run_all_times()
             return 0
 
@@ -137,33 +137,6 @@ def run_metplus(config):
         return 1
 
 
-def _get_wrapper_instance(config, process, instance=None):
-    """!Initialize METplus wrapper instance.
-
-    @param config METplusConfig object to pass to wrapper constructor
-    @param process name of wrapper in camel case, e.g. GridStat
-    @param instance (optional) instance identifier for creating multiple
-    instances of a wrapper. Set to None (default) if no instance is specified
-    @returns CommandBuilder sub-class object or None if something went wrong
-    """
-    try:
-        package_name = ('metplus.wrappers.'
-                        f'{camel_to_underscore(process)}_wrapper')
-        module = import_module(package_name)
-        metplus_wrapper = (
-            getattr(module, f"{process}Wrapper")(config, instance=instance)
-        )
-    except AttributeError as err:
-        config.logger.error(f"There was a problem loading {process} wrapper: {err}")
-        return None
-    except ModuleNotFoundError:
-        config.logger.error(f"Could not load {process} wrapper. "
-                            "Wrapper may have been disabled.")
-        return None
-
-    return metplus_wrapper
-
-
 def _load_all_wrappers(config, process_list):
     """!Initialize all METplus wrapper instances in process list.
 
@@ -176,7 +149,7 @@ def _load_all_wrappers(config, process_list):
     processes = []
     is_ok = True
     for process, instance in process_list:
-        wrapper = _get_wrapper_instance(config, process, instance)
+        wrapper = get_wrapper_instance(config, process, instance)
         if not wrapper:
             is_ok = False
             continue

metplus/util/wrapper_init.py

@@ -0,0 +1,30 @@
+from importlib import import_module
+
+from . import camel_to_underscore
+
+
+def get_wrapper_instance(config, process, instance=None):
+    """!Initialize METplus wrapper instance.
+
+    @param config METplusConfig object to pass to wrapper constructor
+    @param process name of wrapper in camel case, e.g. GridStat
+    @param instance (optional) instance identifier for creating multiple
+    instances of a wrapper. Set to None (default) if no instance is specified
+    @returns CommandBuilder sub-class object or None if something went wrong
+    """
+    try:
+        package_name = ('metplus.wrappers.'
+                        f'{camel_to_underscore(process)}_wrapper')
+        module = import_module(package_name)
+        metplus_wrapper = (
+            getattr(module, f"{process}Wrapper")(config, instance=instance)
+        )
+    except AttributeError as err:
+        config.logger.error(f"There was a problem loading {process} wrapper: {err}")
+        return None
+    except ModuleNotFoundError:
+        config.logger.error(f"Could not load {process} wrapper. "
+                            "Wrapper may have been disabled.")
+        return None
+
+    return metplus_wrapper