From d64a2732691a82cd5a5ded3ea434cd0c37a2384b Mon Sep 17 00:00:00 2001
From: Marco Mambelli <marcom@fnal.gov>
Date: Mon, 16 Dec 2024 00:26:37 -0600
Subject: [PATCH] Fixed and disabled DictFile.append(), replaced by
 add_environment()

append was used only for environment variables in the condor submit file and was causing a malformed submit file

add_environment was already used at times and is a better method. append was fixed and is still there commented in case we'd like to use it for other attributes in the future
---
 creation/lib/cgWCreate.py   |  3 ++-
 creation/lib/cgWDictFile.py | 40 ++++++++++++++++++++++++-------------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/creation/lib/cgWCreate.py b/creation/lib/cgWCreate.py
index d9727aabd..d106b285e 100644
--- a/creation/lib/cgWCreate.py
+++ b/creation/lib/cgWCreate.py
@@ -193,7 +193,8 @@ def populate(self, exe_fname, entry_name, conf, entry):
         frontend_recipients = []  # TODO: change when adding support for LOG_RECIPIENTS_CLIENT
         log_recipients = list(set(factory_recipients + frontend_recipients))
         if len(log_recipients) > 0:
-            self.append("environment", '"LOG_RECIPIENTS=' + "'" + " ".join(log_recipients) + "'" + '"')
+            # self.append("environment", '"LOG_RECIPIENTS=' + "'" + " ".join(log_recipients) + "'" + '"')
+            self.add_environment("LOG_RECIPIENTS='" + " ".join(log_recipients) + "'")
         # print("Token dir: " + token_basedir)  # DEBUG
 
         # Add in some common elements before setting up grid type specific attributes
diff --git a/creation/lib/cgWDictFile.py b/creation/lib/cgWDictFile.py
index 4b8e22770..c4245bf6e 100644
--- a/creation/lib/cgWDictFile.py
+++ b/creation/lib/cgWDictFile.py
@@ -159,22 +159,34 @@ def __init__(
         cWDictFile.DictFile.__init__(self, dir, fname, sort_keys, order_matters, fname_idx)
         self.jobs_in_cluster = jobs_in_cluster
 
-    def append(self, key, val):
-        # TODO add_environment would allow an easier handling of the environment attribute:
-        #  - ordered dict as input
-        #  - handling quoting and escaping
-        #  - formatting and overwriting the environment attribute in the submit file dict
-        if key == "environment":
-            # assumed to be in the new format (quoted)
-            if key not in self.keys:
-                self.add(key, val)
-            else:
-                # should add some protection about correct quoting
-                self.add(f"{val[:-1]} {self[key][1:]}", True)
-        else:
-            raise RuntimeError(f"CondorJDLDictFile append unsupported for key {key} (val: {val})!")
+    # append has been replaced by add_environment to ease the quote handling
+    # If a generic version is needed, the code can be modified and restored
+    # def append(self, key, val):
+    #     if key == "environment":
+    #         # assumed to be in the new format (quoted)
+    #         if key not in self.keys:
+    #             self.add(key, val)
+    #         else:
+    #             # should add some protection about correct quoting
+    #             self.add(key, f"{val[:-1]} {self[key][1:]}", True)
+    #     else:
+    #         raise RuntimeError(f"CondorJDLDictFile append unsupported for key {key} (val: {val})!")
 
     def add_environment(self, val):
+        """Add a variable definition to the "environment" key in the CondorJDLDictFile.
+        Defines a new "environment" entry if not there,
+        appends to the existing environment if there.
+        The whole environment value is enclosed in double quotes (")
+
+        Args:
+            val (str): variable definition to add to the environment. Cannot contain double quotes (")
+                Single quotes (') are OK and can be used to quote string values
+        """
+        # TODO: improve environment handling:
+        #  - allow ordered dict as input
+        #  - handling quoting and escaping, e.g. validate val for double quotes
+        #  - formatting and overwriting the environment attribute in the submit file dict
+        #    e.g. add a variable already in the environment
         curenv = self.get("environment")
         if curenv:
             if curenv[0] == '"':