Fixed some non Python3 compliant code and addressed review comments.

Author: mambelli

build/ReleaseManager/tags_all.yaml

@@ -154,7 +154,7 @@ v3_4:
     - Increased testing (more than doubled unit tests coverage and testing more files
       w/ pylint)
   NOTE:
-    - The type of the GLIDEIN_CPU attr is String (to accommodate the keywords auto, slot,
+    - The type of the GLIDEIN_CPUS attr is String (to accommodate the keywords auto, slot,
       node). Documentation was reporting Int incorrectly. Make sure your configuration
       uses the correct type or you may get a reconfig/upgrade error.
     - If you use HTCondor 8.7.2 or bigger with the GlideinWMS Factory, you must install
@@ -452,7 +452,7 @@ v3_2_19:
       to multiple schedds on the Factory"
     - Clarified attribute types and fixed globbing behavior
     - RPM verification fails when config files are changed
-    - 'GLIDEIN_CPU settings "node" and "slot" supported also in Frontend,
+    - 'GLIDEIN_CPUS settings "node" and "slot" supported also in Frontend,
 
       better documented and improved to compensate for PBS misconfiguration'
     - Fixed submission to AWS which uses key_pair as auth_method

factory/glideFactory.py

@@ -507,7 +507,8 @@ def _set_rlimit(soft_l=None, hard_l=None):
         the inherited limits from the parent process are used.
 
         Note:
-            It is possible to raise limits up to [hard_l, hard_l] but once lowered they cannot be raised.
+            It is possible to raise limits up to [hard_l, hard_l], i.e. soft limit can be raised by a non-root user up
+            to the current process hard limit, but once lowered they cannot be raised.
             It may be better to omit calling this function from subprocesses so that they inherit limits from the parent.
 
         Args:

factory/glideFactoryConfig.py

@@ -71,7 +71,7 @@ class ConfigFile:
     Loads a file composed of
         NAME VAL
     lines and creates a dictionary where each key is assigned the value produced
-    by applying the conversion function to the corresponding string. The default
+    by applying the conversion function to the corresponding value string. The default
     conversion is repr, but a custom conversion function can be provided:
         self.data[NAME]=convert_function(VAL)
     It also defines:
@@ -90,7 +90,7 @@ def __init__(self, config_file, convert_function=repr):
 
         Args:
             config_file (str): The path to the configuration file.
-            convert_function (Callable, optional): Function to convert string values.
+            convert_function (Callable, optional): Function to convert value strings.
                 Defaults to repr.
         """
         self.config_file = config_file
@@ -163,7 +163,7 @@ class EntryConfigFile(ConfigFile):
     Attributes:
         config_file (str): Name of file with entry directory (from parent ConfigFile)
         entry_name (str): Entry name
-        config_file_short(str): Name of file (just the file name since the other had the directory)
+        config_file_short (str): Name of file (just the file name since the other had the directory)
     """
 
     def __init__(self, entry_name, config_file, convert_function=repr):
@@ -188,7 +188,7 @@ class JoinConfigFile(ConfigFile):
     This class does not support saving changes to disk.
 
     Attributes:
-        config_file(str): Name of both files, with and without entry directory, with " AND " in the middle.
+        config_file (str): Name of both files, with and without entry directory, with " AND " in the middle.
             It is not an actual file. (from parent ConfigFile, different value)
         data (dict): Will contain the joint items (initially the common one, then is updated using
             the content of `entry_obj.data`) (from parent ConfigFile, different value)
@@ -303,7 +303,7 @@ def get_pub_key_id(self):
         return self.pub_key_id[0:]
 
     def extract_sym_key(self, enc_sym_key):
-        """Extract the symmetric key from an encrypted value (Fronted attribute).
+        """Extract the symmetric key from an encrypted value (Frontend attribute).
 
         Args:
             enc_sym_key (str): Encrypted symmetric key as an ASCII string (AnyStrASCII).

factory/glideFactoryCredentials.py

@@ -48,7 +48,7 @@ class SubmitCredentials:
 
     Attributes:
         username (str): The username for submission.
-        security_class: The security class for submission. (Seems redundant info)
+        security_class: The security class for submission.
         id: The identifier used for tracking the submit credentials.
         cred_dir (str): The directory location of credentials.
         security_credentials (dict): Dictionary mapping credential types to file paths.
@@ -212,8 +212,8 @@ def process_global(classad, glidein_descript, frontend_descript):
 
     Args:
         classad (dict): A dictionary representation of the classad.
-        glidein_descript (glideFactoryConfig.GlideinDescript): The glidein description object.
-        frontend_descript (glideFactoryConfig.FrontendDescript): The frontend description object.
+        glidein_descript (glideFactoryConfig.GlideinDescript): Factory configuration's Glidein description object.
+        frontend_descript (glideFactoryConfig.FrontendDescript): Factory configuration's Frontend description object.
 
     Raises:
         CredentialError: If the factory has no public key or if any decryption error occurs.
@@ -295,7 +295,7 @@ def validate_frontend(classad, frontend_descript, pub_key_obj):
 
     Args:
         classad (dict): A dictionary representation of the classad.
-        frontend_descript (glideFactoryConfig.FrontendDescript): Object containing Frontend information.
+        frontend_descript (glideFactoryConfig.FrontendDescript): Factory configuration's Frontend description object.
         pub_key_obj (object): The Factory public key object with encryption/decryption methods.
 
     Returns:

factory/glideFactoryDowntimeLib.py

@@ -1,7 +1,7 @@
 # SPDX-FileCopyrightText: 2009 Fermi Research Alliance, LLC
 # SPDX-License-Identifier: Apache-2.0
 
-"""This module implements the functions needed to handle the downtimes
+"""This module implements the functions needed to handle the downtimes.
 """
 
 import fcntl
@@ -23,8 +23,8 @@ class DowntimeFile:
     Each non-comment line in the file must have at least two space-separated entries (start_time and end_time),
     expressed in Unix time (seconds since epoch). If end_time is "None", the downtime does not have
     a set expiration (i.e. it runs forever). Additional entries may be used to limit the scope
-    (Entry, Frontend, Sec_Class names) and to add a comment. Missing values or, respectively "factory",
-    "All", "All" mean that there are no scope restrictions.
+    (Entry, Frontend, Sec_Class names) and to add a comment. Missing scope values or, respectively, the keywords
+    "factory", "All", "All" mean that there are no scope restrictions.
     """
 
     def __init__(self, fname):
@@ -124,8 +124,8 @@ def start_downtime(
     ):
         """Start a downtime period with an indefinite end time.
 
-        If start_time is None, the current time is used. The default values for entry, frontend,
-        and security_class are "All", meaning that there are no scope restrictions.
+        If `start_time` is None, the current time is used. The default values for `entry`, `frontend`,
+        and `security_class` are "All", meaning that there are no scope restrictions.
 
         Args:
             start_time (int or None): The start time in seconds since the epoch.
@@ -140,7 +140,7 @@ def start_downtime(
                 Defaults to True.
 
         Returns:
-            int: The result of addPeriod (typically 0).
+            int: The result of `add_period` (typically 0).
         """
         if start_time is None:
             start_time = int(time.time())
@@ -149,8 +149,8 @@ def start_downtime(
     def end_downtime(self, end_time=None, entry="All", frontend="All", security_class="All", comment=""):
         """End an active downtime period.
 
-        If end_time is None, the current time is used. "All" (default) is a wildcard for entry,
-        frontend, and security_class.
+        If `end_time` is None, the current time is used. "All" (default) is a wildcard for `entry`,
+        `frontend`, and `security_class`, meaning that there are no scope restrictions.
 
         Args:
             end_time (int or None): The end time in seconds since the epoch.
@@ -329,9 +329,10 @@ def _print_downtime(fname, entry="Any", check_time=None):
 def _check_downtime(fname, entry="Any", frontend="Any", security_class="Any", check_time=None):
     """Check if a downtime period is active at the specified time.
 
-    If check_time is None, the current time is used.
-    entry, frontend, and security_class can be used to restrict the scope.
-    "All" is used as a wildcard for entry, frontend, and security_class.
+    If `check_time` is None, the current time is used.
+    `entry`, `frontend`, and `security_class` can be used to restrict the scope.
+    "All" is used as a wildcard for `entry`, `frontend`, and `security_class`,
+    to avoid scope restrictions.
 
     Args:
         fname (str or Path): The downtime file.
@@ -396,7 +397,7 @@ def _add_period(
         int: 0 upon successful addition.
 
     Raises:
-        FileNotFoundError: if there is no downtime file and create_if_empty is False
+        FileNotFoundError: if there is no downtime file and create_if_empty is False.
     """
     exists = os.path.isfile(fname)
     if (not exists) and (not create_if_empty):
@@ -523,8 +524,9 @@ def _purge_old_periods(fname, cut_time=None, raise_on_error=False):
 def _end_downtime(fname, end_time=None, entry="All", frontend="All", security_class="All", comment=""):
     """End an active downtime period.
 
-    If end_time is None, the current time is used.
-    "All" (default) is used as a wildcard for entry, frontend, and security_class.
+    If `end_time` is None, the current time is used.
+    "All" (default) is used as a wildcard for `entry`, `frontend`, and `security_class`,
+    to avoid scope restrictions.
 
     Args:
         fname (str or Path): The downtime file.

factory/glideFactoryEntry.py

@@ -175,11 +175,11 @@ def loadContext(self):
     def getGlideinExpectedCores(self):
         """Return the expected number of cores for each glidein.
 
-        This returns the value of the GLIDEIN_CPU attribute if > 0;
-        otherwise, it returns the GLIDEIN_ESTIMATED_CPUS value when GLIDEIN_CPU <= 0
+        This returns the value of the GLIDEIN_CPUS attribute if > 0;
+        otherwise, it returns the GLIDEIN_ESTIMATED_CPUS value when GLIDEIN_CPUS <= 0
         or a string (auto/node/slot) or 1 if not set.
         Note that this number is used in provisioning and set in HTCondor. The actual
-        number of cores used will depend on the RSL or HTCondor attributes and the Entry
+        number of cores used will depend on the RSL or HTCondor attributes, on the Entry,
         and could also vary over time.
 
         Returns:
@@ -235,7 +235,7 @@ def loadDowntimes(self):
         self.jobAttributes.data["GLIDEIN_Downtime_Comment"] = self.downtimes.downtime_comment
 
     def isClientBlacklisted(self, client_sec_name):
-        """Check if a client (Frontend) is blacklisted. I.e. the whitelist is enable and the client is not in the list.
+        """Check if a client (Frontend) is blacklisted. I.e. the whitelist is enabled and the client is not in the list.
 
         Args:
             client_sec_name (str): The security name of the client.
@@ -507,7 +507,7 @@ def writeClassadsToFile(self, downtime_flag, gf_filename, gfc_filename, append=T
         Advertising is done in a separate function.
 
         Args:
-            downtime_flag (bool): Downtime flag.
+            downtime_flag (bool): Flag indicating whether the Factory is in downtime or not.
             gf_filename (str): Filename to write the glidefactory classads.
             gfc_filename (str): Filename to write the glidefactoryclient classads.
             append (bool, optional): If True, append to existing files (i.e Multi ClassAds file);
@@ -657,7 +657,7 @@ def advertise(self, downtime_flag):
         """Advertise the glidefactory and glidefactoryclient classads.
 
         Args:
-            downtime_flag (bool): Downtime flag.
+            downtime_flag (bool): Flag indicating whether the Factory is in downtime or not.
         """
         self.loadContext()
 
@@ -736,7 +736,7 @@ def getLogStatsCurrentStatsData(self):
         return self.getLogStatsData(self.gflFactoryConfig.log_stats.current_stats_data)
 
     def getLogStatsData(self, stats_data):
-        """Retrieve normalized log statistics data, `stats_data(stats_data[frontend][user].data)`, for pickling.
+        """Retrieve normalized log statistics data, ``stats_data[frontend][user].data``, for pickling.
 
         Args:
             stats_data (dict): Dictionary of statistics data keyed by Frontend and user.
@@ -769,7 +769,7 @@ def setLogStatsCurrentStatsData(self, new_data):
         self.setLogStatsData(self.gflFactoryConfig.log_stats.current_stats_data, new_data)
 
     def setLogStatsData(self, stats_data, new_data):
-        """Set log statistics data, `stats_data(stats_data[frontend][user].data)`, from pickled information.
+        """Set log statistics data, ``stats_data[frontend][user].data``, from pickled information.
 
         Args:
             stats_data (dict): The existing statistics data.
@@ -812,19 +812,6 @@ def getState(self):
         }
         return state
 
-    def setState_old(self, state):
-        """Load the post work state from pickled information.
-
-        Args:
-            state (dict): The pickled state after work has been performed.
-        """
-        self.gflFactoryConfig.client_stats = state.get("client_stats")
-        self.gflFactoryConfig.qc_stats = state.get("qc_stats")
-        self.gflFactoryConfig.rrd_stats = state.get("rrd_stats")
-        self.gflFactoryConfig.client_internals = state.get("client_internals")
-        self.glideinTotals = state.get("glidein_totals")
-        self.gflFactoryConfig.log_stats = state["log_stats"]
-
     def setState(self, state):
         """Load the post work state from pickled information.
 
@@ -867,7 +854,7 @@ def setState(self, state):
     # Debugging functions
     #####################
     def logLogStats(self, marker=""):
-        """Log detailed log statistics for debugging.
+        """Log detailed statistics for debugging.
 
         Args:
             marker (str, optional): A marker string to prefix the debug logs. Defaults to an empty string.
@@ -893,7 +880,7 @@ def check_and_perform_work(factory_in_downtime, entry, work):
 
     This function checks if work needs to be done for the entry and processes each work request
     using the v3 protocol. It updates credentials, checks downtimes and whitelisting, and submits work.
-    It is called by a child process per entry
+    It is called by a child process per entry.
 
     Args:
         factory_in_downtime (bool): True if the factory is in downtime.
@@ -1771,7 +1758,8 @@ def update_entries_stats(factory_in_downtime, entry_list):
     TODO: #22163, skip update when in downtime?
 
     Note:
-        qc_stats cannot be updated because the frontend certificate information are missing
+        `qc_stats` (the client queue information) cannot be updated because the
+        Frontend certificate information are missing.
 
     Args:
         factory_in_downtime (bool): True if the factory is in downtime.

factory/glideFactoryEntryGroup.py

@@ -243,7 +243,7 @@ def find_and_perform_work(do_advertize, factory_in_downtime, glideinDescript, fr
         glideinDescript (dict): Factory glidein configuration values.
         frontendDescript (dict): Security mappings for Frontend identities, security classes, and usernames.
         group_name (str): Name of the group.
-        my_entries (dict): Dictionary of entry objects (Entry) keyed by entry name.
+        my_entries (dict): Dictionary of entry objects (`glideFactoryEntry.Entry`) keyed by entry name.
 
     Returns:
         dict: Dictionary of work done, keyed by entry name.
@@ -613,7 +613,7 @@ def main(parent_pid, sleep_time, advertize_rate, startup_dir, entry_names, group
         startup_dir (str|Path): The "home" directory for the entry.
         entry_names (str): Colon-separated list of entry names to process.
         group_id (str): Group ID (normally a number, with the "group_" prefix it forms the group name).
-            It can change between Factory reconfigurations
+            It can change between Factory reconfigurations.
     """
     # Assume name to be group_[0,1,2] etc. Only required to create log_dir
     # where tasks common to the group will be stored. There is no other