From 0d625e1bce8e17465292914319215ec04fffa023 Mon Sep 17 00:00:00 2001 From: Andrei Tsaregorodtsev Date: Tue, 5 Sep 2023 14:59:47 +0200 Subject: [PATCH 1/7] feat: changes to drop the system instance level in the CS --- src/DIRAC/ConfigurationSystem/Client/PathFinder.py | 13 ++++++++----- .../Agent/StalledJobAgent.py | 7 ++----- .../WorkloadManagementSystem/JobWrapper/Watchdog.py | 7 ++----- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py index 941cf8dec41..156593d16ee 100755 --- a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py +++ b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py @@ -40,11 +40,14 @@ def getSystemInstance(system, setup=False): :return: str """ - optionPath = Path.cfgPath("/DIRAC/Setups", setup or getDIRACSetup(), system) - instance = gConfigurationData.extractOptionFromCFG(optionPath) - if not instance: - raise RuntimeError(f"Option {optionPath} is not defined") - return instance + if "Setups" in gConfigurationData.getSectionsFromCFG("/DIRAC"): + optionPath = Path.cfgPath("/DIRAC/Setups", setup or getDIRACSetup(), system) + instance = gConfigurationData.extractOptionFromCFG(optionPath) + if not instance: + raise RuntimeError(f"Option {optionPath} is not defined") + return instance + # If /DIRAC/Setups is not present in the CS return empty instance string + return "" def getSystemSection(system, instance=False, setup=False): diff --git a/src/DIRAC/WorkloadManagementSystem/Agent/StalledJobAgent.py b/src/DIRAC/WorkloadManagementSystem/Agent/StalledJobAgent.py index f95b74021b8..5d745e0e409 100755 --- a/src/DIRAC/WorkloadManagementSystem/Agent/StalledJobAgent.py +++ b/src/DIRAC/WorkloadManagementSystem/Agent/StalledJobAgent.py @@ -16,7 +16,7 @@ from DIRAC import S_ERROR, S_OK, gConfig from DIRAC.AccountingSystem.Client.Types.Job import Job from DIRAC.ConfigurationSystem.Client.Helpers import cfgPath -from DIRAC.ConfigurationSystem.Client.PathFinder import getSystemInstance +from DIRAC.ConfigurationSystem.Client.PathFinder import getSystemSection from DIRAC.Core.Base.AgentModule import AgentModule from DIRAC.Core.Utilities import DErrno from DIRAC.Core.Utilities.ClassAd.ClassAdLight import ClassAd @@ -55,9 +55,6 @@ def initialize(self): if not self.am_getOption("Enable", True): self.log.info("Stalled Job Agent running in disabled mode") - wms_instance = getSystemInstance("WorkloadManagement") - if not wms_instance: - return S_ERROR("Can not get the WorkloadManagement system instance") self.stalledJobsTolerantSites = self.am_getOption("StalledJobsTolerantSites", []) self.stalledJobsToleranceTime = self.am_getOption("StalledJobsToleranceTime", 0) @@ -67,7 +64,7 @@ def initialize(self): self.matchedTime = self.am_getOption("MatchedTime", self.matchedTime) self.rescheduledTime = self.am_getOption("RescheduledTime", self.rescheduledTime) - wrapperSection = cfgPath("Systems", "WorkloadManagement", wms_instance, "JobWrapper") + wrapperSection = f"{getSystemSection('WorkloadManagement')}/JobWrapper" failedTime = self.am_getOption("FailedTimeHours", 6) watchdogCycle = gConfig.getValue(cfgPath(wrapperSection, "CheckingTime"), 30 * 60) diff --git a/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py b/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py index c2a0ef50ad6..e8eeb7be128 100755 --- a/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py +++ b/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py @@ -25,7 +25,7 @@ from DIRAC import S_ERROR, S_OK, gLogger from DIRAC.ConfigurationSystem.Client.Config import gConfig -from DIRAC.ConfigurationSystem.Client.PathFinder import getSystemInstance +from DIRAC.ConfigurationSystem.Client.PathFinder import getSystemSection from DIRAC.Core.Utilities import MJF from DIRAC.Core.Utilities.Os import getDiskSpace from DIRAC.Core.Utilities.Profiler import Profiler @@ -107,10 +107,7 @@ def initialize(self): setup = gConfig.getValue("/DIRAC/Setup", "") if not setup: return S_ERROR("Can not get the DIRAC Setup value") - wms_instance = getSystemInstance("WorkloadManagement") - if not wms_instance: - return S_ERROR("Can not get the WorkloadManagement system instance") - self.section = f"/Systems/WorkloadManagement/{wms_instance}/JobWrapper" + self.section = f"{getSystemSection('WorkloadManagement')}/JobWrapper" self.log.verbose("Watchdog initialization") # Test control flags From a54f6b608e589a6429da1d5e1310157234bd1824 Mon Sep 17 00:00:00 2001 From: Andrei Tsaregorodtsev Date: Tue, 5 Sep 2023 15:54:52 +0200 Subject: [PATCH 2/7] fix: fix the pytest getSystemInstance mock objects --- .../Agent/test/Test_Agent_StalledJobAgent.py | 4 ++-- src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py | 3 --- .../JobWrapper/test/Test_JobWrapper.py | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_StalledJobAgent.py b/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_StalledJobAgent.py index 615c55065a7..dbf4ce2fbf7 100644 --- a/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_StalledJobAgent.py +++ b/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_StalledJobAgent.py @@ -25,11 +25,11 @@ def sja(mocker): ) mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.JobDB") mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.JobLoggingDB") - mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.getSystemInstance", side_effect=mockNone) + mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.getSystemSection", side_effect=mockNone) mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.JobMonitoringClient", return_value=MagicMock()) mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.PilotManagerClient", return_value=MagicMock()) mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.WMSClient", return_value=MagicMock()) - mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.getSystemInstance", return_value="/bof/bih") + mocker.patch("DIRAC.WorkloadManagementSystem.Agent.StalledJobAgent.getSystemSection", return_value="/bof/bih") stalledJobAgent = StalledJobAgent() stalledJobAgent._AgentModule__configDefaults = mockAM diff --git a/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py b/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py index e8eeb7be128..7ff8a46d3b8 100755 --- a/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py +++ b/src/DIRAC/WorkloadManagementSystem/JobWrapper/Watchdog.py @@ -104,9 +104,6 @@ def initialize(self): else: self.initialized = True - setup = gConfig.getValue("/DIRAC/Setup", "") - if not setup: - return S_ERROR("Can not get the DIRAC Setup value") self.section = f"{getSystemSection('WorkloadManagement')}/JobWrapper" self.log.verbose("Watchdog initialization") diff --git a/src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py b/src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py index b13d7c6ce2f..4fb1521f718 100644 --- a/src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py +++ b/src/DIRAC/WorkloadManagementSystem/JobWrapper/test/Test_JobWrapper.py @@ -92,7 +92,7 @@ def test_execute(mocker, executable, args, src, expectedResult): "DIRAC.WorkloadManagementSystem.JobWrapper.JobWrapper.getSystemSection", side_effect=getSystemSectionMock ) mocker.patch( - "DIRAC.WorkloadManagementSystem.JobWrapper.Watchdog.getSystemInstance", side_effect=getSystemSectionMock + "DIRAC.WorkloadManagementSystem.JobWrapper.Watchdog.getSystemSection", side_effect=getSystemSectionMock ) if src: From e81da3ba436b53ce4a20fb57a1c51847cfe21c9f Mon Sep 17 00:00:00 2001 From: Andrei Tsaregorodtsev Date: Wed, 1 Nov 2023 22:36:22 +0100 Subject: [PATCH 3/7] fix: doc string and minor logic --- .../ConfigurationSystem/Client/PathFinder.py | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py index 156593d16ee..6dd2920f377 100755 --- a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py +++ b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py @@ -33,21 +33,22 @@ def divideFullName(entityName, componentName=None): def getSystemInstance(system, setup=False): - """Find system instance name + """Find system instance name. :param str system: system name :param str setup: setup name - :return: str + :return: str: instance name. If the system option is present + in the /DIRAC/Setups/ section but is not given a value, + an empty string is returned """ - if "Setups" in gConfigurationData.getSectionsFromCFG("/DIRAC"): - optionPath = Path.cfgPath("/DIRAC/Setups", setup or getDIRACSetup(), system) - instance = gConfigurationData.extractOptionFromCFG(optionPath) - if not instance: - raise RuntimeError(f"Option {optionPath} is not defined") - return instance - # If /DIRAC/Setups is not present in the CS return empty instance string - return "" + + setupPath = Path.cfgPath("/DIRAC/Setups", setup or getDIRACSetup()) + optionPath = Path.cfgPath(setupPath, system) + instance = gConfigurationData.extractOptionFromCFG(optionPath) + if not instance and system not in gConfigurationData.getOptionsFromCFG(setupPath): + raise RuntimeError(f"Option {optionPath} is not defined") + return instance def getSystemSection(system, instance=False, setup=False): From ad15178f9a68e0d64e33718b9a5eacd898e27dcd Mon Sep 17 00:00:00 2001 From: Andrei Tsaregorodtsev Date: Wed, 17 Jan 2024 14:36:51 +0100 Subject: [PATCH 4/7] fix: use None as a value for no setup --- src/DIRAC/ConfigurationSystem/Client/PathFinder.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py index 6dd2920f377..c6022367930 100755 --- a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py +++ b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py @@ -43,10 +43,14 @@ def getSystemInstance(system, setup=False): an empty string is returned """ - setupPath = Path.cfgPath("/DIRAC/Setups", setup or getDIRACSetup()) - optionPath = Path.cfgPath(setupPath, system) + # If Setup is None, it means that we have the case of configuration without Setup + setupToUse = setup or getDIRACSetup() + if setupToUse == "None": + return "" + + optionPath = Path.cfgPath("/DIRAC/Setups", setupToUse, system) instance = gConfigurationData.extractOptionFromCFG(optionPath) - if not instance and system not in gConfigurationData.getOptionsFromCFG(setupPath): + if not instance: raise RuntimeError(f"Option {optionPath} is not defined") return instance From e6013cadf5a29f541413a183bdb99fdc41daeceb Mon Sep 17 00:00:00 2001 From: Andrei Tsaregorodtsev Date: Fri, 13 Sep 2024 15:32:23 +0200 Subject: [PATCH 5/7] feat: rely on remote config for the Setup definition --- src/DIRAC/ConfigurationSystem/Client/PathFinder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py index c6022367930..dcc4805cb4c 100755 --- a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py +++ b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py @@ -43,8 +43,8 @@ def getSystemInstance(system, setup=False): an empty string is returned """ - # If Setup is None, it means that we have the case of configuration without Setup - setupToUse = setup or getDIRACSetup() + # If Setup is "None", it means that we have the case of configuration without Setup + setupToUse = setup or gConfigurationData.remoteCFG.getOption("/DIRAC/Setup") if setupToUse == "None": return "" From 160b1ba6f35b89f5077260f05154289f6a5d896b Mon Sep 17 00:00:00 2001 From: Andrei Tsaregorodtsev Date: Fri, 13 Sep 2024 18:06:14 +0200 Subject: [PATCH 6/7] feat: use /DIRAC/NoSetup flag to denote no setups used --- src/DIRAC/ConfigurationSystem/Client/PathFinder.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py index dcc4805cb4c..8009c412b14 100755 --- a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py +++ b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py @@ -43,11 +43,17 @@ def getSystemInstance(system, setup=False): an empty string is returned """ - # If Setup is "None", it means that we have the case of configuration without Setup - setupToUse = setup or gConfigurationData.remoteCFG.getOption("/DIRAC/Setup") - if setupToUse == "None": + # If noSetupFlag is set to True, we do not have system instances + try: + noSetupFlag = gConfigurationData.extractOptionFromCFG("/DIRAC/NoSetup") + if noSetupFlag == "True": + return "" + except: return "" + # If Setup is "None", it means that we have the case of configuration without Setup + setupToUse = setup or getDIRACSetup() + optionPath = Path.cfgPath("/DIRAC/Setups", setupToUse, system) instance = gConfigurationData.extractOptionFromCFG(optionPath) if not instance: From c441cb344a9e00517b878a6d87296593d78c8672 Mon Sep 17 00:00:00 2001 From: Andrei Tsaregorodtsev Date: Fri, 13 Sep 2024 23:01:51 +0200 Subject: [PATCH 7/7] fix: simplify, do not check for exceptions --- src/DIRAC/ConfigurationSystem/Client/PathFinder.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py index 8009c412b14..95737405d09 100755 --- a/src/DIRAC/ConfigurationSystem/Client/PathFinder.py +++ b/src/DIRAC/ConfigurationSystem/Client/PathFinder.py @@ -43,17 +43,12 @@ def getSystemInstance(system, setup=False): an empty string is returned """ - # If noSetupFlag is set to True, we do not have system instances - try: - noSetupFlag = gConfigurationData.extractOptionFromCFG("/DIRAC/NoSetup") - if noSetupFlag == "True": - return "" - except: + # If noSetupFlag is set to True, we do not have system instances in the configuration + noSetupFlag = gConfigurationData.extractOptionFromCFG("/DIRAC/NoSetup") + if noSetupFlag == "True": return "" - # If Setup is "None", it means that we have the case of configuration without Setup setupToUse = setup or getDIRACSetup() - optionPath = Path.cfgPath("/DIRAC/Setups", setupToUse, system) instance = gConfigurationData.extractOptionFromCFG(optionPath) if not instance: