DM-53944: Improve handling of job environment settings.

doc/changes/DM-53944.feature.rst

@@ -0,0 +1,2 @@
+Made improvements to how the environment specifications in submit yaml are handled.  See ``Job Environment`` section in docs for details and examples.
+Environment variables set in several ``includeConfigs`` files can now chain onto each other with ``${VAR}``, whatever the variable is called.

doc/lsst.ctrl.bps/quickstart.rst

@@ -1541,6 +1541,94 @@ invisible to the user.  ``bps report`` will still show same labels and
 total counts as without ordering.  ``cancel`` and ``restart`` will still
 work the same.
 
+
+.. _job-environment:
+
+Job Environment
+---------------
+
+One can specify environment values for jobs in the submit yaml.  When creating
+the job description for the WMS-plugins, BPS will search for environment
+settings following normal scoping ordering, but performs a special merge
+instead of replacing the entire section.  Also if an environment value itself
+contains another yaml environment variable, BPS will replace that yaml environment
+variable with its yaml value.  This will allow normal shell path chaining behavior
+across sections.
+
+.. note::
+
+   To not include an environment variable from another section, use the special
+   string ``BPS_NONE`` as the environment variable value (case-insensitive).
+
+.. note::
+
+   The same chaining applies when several files pulled in via ``includeConfigs`` set the same environment variable in the same section.
+   Each later file chains onto the earlier ones and the including file chains onto all of them.
+   For example, two included files that each prepend a directory to ``DAF_BUTLER_CONFIG_PATH`` produce a single value listing both directories.
+
+Here are some examples of the environment variables that BPS instructs
+the WMS-plugin to set inside the job, based on the following BPS YAML
+configuration.  Any environmental variable left in the final value (here
+``${PACKAGE_DIR}`` and ``${VAR_PATH}``) will be expanded from the job's
+shell environment as usual.
+
+.. code-block:: YAML
+
+   var1: "root_val1"
+   environment:
+     VAR2: "root_val2"
+     VAR3: "root_val3"
+     VAR_PATH: "${PACKAGE_DIR}/root_dir:${VAR_PATH}"
+     TEST_VAR: "one {var1} three"
+   site:
+     site1:
+       var1: "site_val1"
+       environment:
+         VAR4: "site_val4"
+         VAR_PATH: "${PACKAGE_DIR}/site_dir:${VAR_PATH}"
+   cluster:
+     cl1:
+       var1: "cl1_val1"
+       environment:
+         VAR2: "BPS_NONE"
+         VAR3: "cl1_val3"
+         VAR4: "cl1_val4"
+
+#. BPS environment for a cl1 job for site1:
+
+   .. code-block::
+
+      VAR3="cl1_val3"
+      VAR4="cl1_val4"
+      VAR_PATH="${PACKAGE_DIR}/site_dir:${PACKAGE_DIR}/root_dir:${VAR_PATH}"
+      TEST_VAR="one cl1_val1 three"
+
+#. BPS environment for a non-cl1 job for site1:
+
+   .. code-block::
+
+      VAR2="root_val2"
+      VAR3="root_val3"
+      VAR4="site_val4"
+      VAR_PATH="${PACKAGE_DIR}/site_dir:${PACKAGE_DIR}/root_dir:${VAR_PATH}"
+      TEST_VAR="one site_val1 three"
+
+#. BPS environment for a non-cli job for some site other than site1:
+
+   .. code-block::
+
+      VAR2="root_val2"
+      VAR3="root_val3"
+      VAR_PATH="${PACKAGE_DIR}/root_dir:${VAR_PATH}"
+      TEST_VAR="one root_val1 three"
+
+Different WMS-plugins create the job environment via different mechanisms
+(e.g., the HTCondor plugin copies the submit environment to the job) or may
+not support setting the environment via submit yaml.  If the WMS-plugin
+supports setting the environment via submit yaml, the job environment is
+require to at least include what is in the submit yaml, but it may include
+more environment settings.
+
 .. _bps-config-generation:
 
 Config Generation

python/lsst/ctrl/bps/bps_config.py

@@ -37,6 +37,7 @@
 import os
 import re
 import string
+from collections.abc import Mapping
 from os.path import expandvars, normpath
 from typing import Any
 
@@ -71,6 +72,36 @@ def get_value(self, key, args, kwargs):
         return val
 
 
+class _ChainedEnvConfig(Config):
+    """Config whose merges chain ``${VAR}`` self-references inside
+    ``environment`` sections.
+
+    When the value being merged for environment variable ``VAR`` contains
+    ``${VAR}`` and this config already holds a value for that variable, the
+    placeholder is replaced by the existing value instead of the whole value
+    being overwritten.  This mirrors shell behavior such as
+    ``PATH="/new/dir:${PATH}"`` and lets several config files pulled in via
+    ``includeConfigs`` each prepend to the same variable.  Any ``${VAR}``
+    left in the final value is expanded from the job's shell environment as
+    usual.
+    """
+
+    def update(self, other: Mapping[str, Any]) -> None:  # type: ignore[override]
+        # Work on a copy so the caller's mapping is left untouched.
+        updates = Config(other)
+        for path in updates.nameTuples():
+            if len(path) < 2 or path[-2] != "environment":
+                continue
+            name = path[-1]
+            value = updates[path]
+            if not isinstance(value, str) or f"${{{name}}}" not in value:
+                continue
+            previous = self.get(path)
+            if isinstance(previous, str):
+                updates[path] = value.replace(f"${{{name}}}", previous)
+        super().update(updates)
+
+
 class BpsConfig(Config):
     """Contains the configuration for a BPS submission.
 
@@ -122,12 +153,15 @@ def __init__(self, other, search_order=None, defaults=None, wms_service_class_fq
         # class __getitem__ method.
         super().__init__()
 
+        # Read the config with environment chaining so that files pulled in
+        # via includeConfigs can each prepend to the same environment
+        # variable rather than the last file replacing the earlier values.
         try:
-            other_config = Config(other)
+            other_config = _ChainedEnvConfig(other)
         except Exception as exc:
             raise ValueError(f"A BpsConfig could not be loaded from other: {other}") from exc
 
-        config = Config()
+        config = _ChainedEnvConfig()
 
         # Pre-populate the config with default settings if any were provided
         # by the caller. Include WMS plugin specific defaults and/or

python/lsst/ctrl/bps/constants.py

@@ -29,11 +29,7 @@
 
 from astropy import units as u
 
-__all__ = [
-    "DEFAULT_MEM_FMT",
-    "DEFAULT_MEM_RETRIES",
-    "DEFAULT_MEM_UNIT",
-]
+__all__ = ["BPS_NONE", "DEFAULT_MEM_FMT", "DEFAULT_MEM_RETRIES", "DEFAULT_MEM_UNIT"]
 
 
 DEFAULT_MEM_RETRIES = 5
@@ -47,3 +43,7 @@
 DEFAULT_MEM_FMT = ".3f"
 """Default format specifier to use when reporting memory consumption.
 """
+
+BPS_NONE = "BPS_NONE"
+"""Special yaml value for a None value that can be read and written.
+"""