Turn off CRDS checks by default

src/stpipe/step.py

@@ -434,14 +434,17 @@ def run(self, *args, **kwargs):
         the running of each step.  The real work that is unique to
         each step type is done in the `process` method.
 
-        If this step was not created via a `call` or command line process,
-        default parameters from CRDS are retrieved if available.  Override
-        parameters can also be passed as keyword arguments if desired.
+        If this step was not created via a `call` or a command line process,
+        and if 'disable_crds_parameters' is set to False,
+        default parameters from CRDS are retrieved at runtime, if available.
+        Override parameters can also be passed as keyword arguments if desired.
 
         The order of parameter checking and overrides is:
+
            1. spec default value for the step
            2. keyword parameters or configuration set on step initialization
            3. CRDS parameters if available and if CRDS checks are not disabled
+              (disable_crds_parameters = False)
            4. step attributes explicitly set by the user before calling run,
               either on instantiation or by directly setting the attribute
               after instantiation.
@@ -450,6 +453,23 @@ def run(self, *args, **kwargs):
         Only 1 and 2 are checked if the step was created via `call`
         or the command line.
 
+        Parameters
+        ----------
+        disable_crds_steppars : bool, optional
+            If set to True, CRDS parameter checks are disabled.
+            If set to False, CRDS parameter checks are enabled.
+            If set to None, the environment variable STPIPE_DISABLE_CRDS_STEPPARS
+            is checked: if set to 'true', CRDS parameters checks
+            are disabled.
+
+        Notes
+        -----
+        Currently, the default value for disable_crds_steppars is True,
+        for backward compatibility with previous behavior for the `run`
+        function.  In future builds, the default value will be None,
+        so that the default behavior is to check CRDS for default
+        parameters when possible.
+
         """
         gc.collect()
 
@@ -465,7 +485,15 @@ def run(self, *args, **kwargs):
             self.log.info("Step %s running with args %s.", self.name, args)
 
             # Check for explicit disable for CRDS parameters
-            disable_crds_steppars = kwargs.pop("disable_crds_steppars", None)
+            if 'disable_crds_steppars' in kwargs:
+                disable_crds_steppars = kwargs.pop("disable_crds_steppars")
+            else:
+                disable_crds_steppars = True
+                if self.parent is None and self._validate_kwds:
+                    self.log.warning("CRDS parameter checks are currently disabled by default.")
+                    self.log.warning("In future builds, they will be enabled by default.")
+                    self.log.warning("To turn them on now, set 'disable_crds_steppars' to False "
+                                     "in the arguments to 'run'.")
 
             # Get parameters from user
             parameters = None
@@ -488,11 +516,12 @@ def run(self, *args, **kwargs):
                     parameters, _ = self.build_config(
                         filename, disable=disable_crds_steppars, **kwargs
                     )
-                except (NotImplementedError, FileNotFoundError):
+                except (NotImplementedError, FileNotFoundError, RuntimeError):
                     # Catch steps that cannot build a config
                     # (e.g. post hooks created from local functions,
                     # missing input files)
-                    self.log.warning(f"Cannot retrieve CRDS keywords for {self.name}.")
+                    raise ValueError(f"Cannot retrieve CRDS keywords for "
+                                     f"{self.name} with input {str(filename)}.")
 
             # Update parameters from the retrieved config + keywords
             if parameters:
@@ -530,7 +559,7 @@ def run(self, *args, **kwargs):
 
                 hook_args = args
                 for pre_hook in self._pre_hooks:
-                    hook_results = pre_hook.run(*hook_args)
+                    hook_results = pre_hook.run(*hook_args, disable_crds_steppars=True)
                     if hook_results is not None:
                         hook_args = hook_results
                 args = hook_args
@@ -602,7 +631,7 @@ def run(self, *args, **kwargs):
 
                 # Run the post hooks
                 for post_hook in self._post_hooks:
-                    hook_results = post_hook.run(step_result)
+                    hook_results = post_hook.run(step_result, disable_crds_steppars=True)
                     if hook_results is not None:
                         step_result = hook_results
 

tests/test_step.py

@@ -457,7 +457,7 @@ def test_step_run_crds_values(step_class):
     assert step.str1 == "default"
     assert step._initialized["str1"] is False
 
-    step.run("science.fits")
+    step.run("science.fits", disable_crds_steppars=False)
     assert step.str1 == "from config"
     assert step._initialized["str1"] is True
 
@@ -489,10 +489,42 @@ def test_step_run_disable_crds_via_environment(monkeypatch, step_class):
 
     monkeypatch.setenv("STPIPE_DISABLE_CRDS_STEPPARS", "True")
 
+    step.run("science.fits", disable_crds_steppars=None)
+    assert step.str1 == "default"
+    assert step._initialized["str1"] is False
+
+
+@pytest.mark.usefixtures("_mock_crds_reffile")
+@pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep])
+def test_step_run_disable_crds_default(caplog, step_class):
+    """Test that CRDS parameters are not checked by default."""
+    step = step_class()
+    step.process = lambda *args: None
+
+    assert step.str1 == "default"
+    assert step._initialized["str1"] is False
+
     step.run("science.fits")
     assert step.str1 == "default"
     assert step._initialized["str1"] is False
 
+    assert "CRDS parameter checks are currently disabled by default" in caplog.text
+
+
+@pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep])
+def test_step_run_crds_error(caplog, step_class):
+    """Test that exception is raised if CRDS parameters cannot be checked."""
+    step = step_class()
+    step.process = lambda *args: None
+
+    assert step.str1 == "default"
+    assert step._initialized["str1"] is False
+
+    # Call run with an incomplete step implementation, on a file that does
+    # not exist, without mocking the CRDS retrieval
+    with pytest.raises(ValueError, match='Cannot retrieve CRDS keywords'):
+        step.run("science.fits", disable_crds_steppars=False)
+
 
 @pytest.mark.usefixtures("_mock_crds_reffile")
 @pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep])
@@ -507,7 +539,7 @@ def test_step_run_initialized_values(step_class):
     step.str1 = "from user"
     assert step._initialized["str1"] is True
 
-    step.run("science.fits")
+    step.run("science.fits", disable_crds_steppars=False)
     assert step.str1 == "from user"
     assert step._initialized["str1"] is True
 
@@ -522,7 +554,7 @@ def test_step_run_initialized_values_on_instantiation(step_class):
     assert step.str1 == "on instantiation"
     assert step._initialized["str1"] is True
 
-    step.run("science.fits")
+    step.run("science.fits", disable_crds_steppars=False)
     assert step.str1 == "on instantiation"
     assert step._initialized["str1"] is True
 
@@ -537,7 +569,7 @@ def test_step_run_keyword_values(step_class):
     assert step.str1 == "default"
     assert step._initialized["str1"] is False
 
-    step.run("science.fits", str1="from keywords")
+    step.run("science.fits", str1="from keywords", disable_crds_steppars=False)
     assert step.str1 == "from keywords"
     assert step._initialized["str1"] is True
 
@@ -556,7 +588,7 @@ def test_step_run_keyword_values_after_initialize(step_class):
     assert step._initialized["str1"] is True
 
     # Keyword values still override direct attribute setting
-    step.run("science.fits", str1="from keywords")
+    step.run("science.fits", str1="from keywords", disable_crds_steppars=False)
     assert step.str1 == "from keywords"
     assert step._initialized["str1"] is True
 
@@ -569,7 +601,7 @@ def test_step_run_invalid_parameter(step_class):
     step.process = lambda *args: None
 
     with pytest.raises(cp.ValidationError, match="Extra value"):
-        step.run("science.fits", bad_param="from keywords")
+        step.run("science.fits", bad_param="from keywords", disable_crds_steppars=False)
 
 
 @pytest.mark.usefixtures("_mock_crds_reffile")
@@ -579,7 +611,7 @@ def test_step_run_format_bool_parameters(step_class):
     step = step_class()
     step.process = lambda *args: None
 
-    step.run("science.fits", save_results="False")
+    step.run("science.fits", save_results="False", disable_crds_steppars=False)
     assert step.save_results is False
 
 
@@ -595,7 +627,7 @@ def test_pipe_run_step_values():
     assert pipe.step1._initialized["str1"] is False
 
     # Parameters are set by CRDS
-    pipe.run("science.fits")
+    pipe.run("science.fits", disable_crds_steppars=False)
     assert pipe.step1.str1 == "from config"
     assert pipe.step1._initialized["str1"] is True
 
@@ -612,7 +644,8 @@ def test_pipe_run_step_values_from_keywords():
     assert pipe.step1._initialized["str1"] is False
 
     # Parameters are set by user
-    pipe.run("science.fits", steps={"step1": {"str1": "from keywords"}})
+    pipe.run("science.fits", steps={"step1": {"str1": "from keywords"}},
+             disable_crds_steppars=False)
     assert pipe.step1.str1 == "from keywords"
     assert pipe.step1._initialized["str1"] is True
 
@@ -632,7 +665,7 @@ def test_pipe_run_step_values_skip_initialized():
     assert pipe.step1._initialized["str1"] is True
 
     # Parameters are not overridden by CRDS
-    pipe.run("science.fits")
+    pipe.run("science.fits", disable_crds_steppars=False)
     assert pipe.step1.str1 == "from user"
     assert pipe.step1._initialized["str1"] is True
 
@@ -647,6 +680,6 @@ def test_pipe_run_step_values_skip_initialized_on_instantiation():
     assert pipe.step1._initialized["str1"] is True
 
     # Parameters are not overridden by CRDS
-    pipe.run("science.fits")
+    pipe.run("science.fits", disable_crds_steppars=False)
     assert pipe.step1.str1 == "on instantiation"
     assert pipe.step1._initialized["str1"] is True