fix: Fixed doc, and went back on the client selector

Author: Robin-Van-de-Merghel

docs/source/DeveloperGuide/CodeTesting/index.rst

@@ -352,17 +352,13 @@ And you can also restart all services (it can take some time):
 
 You can also test DiracX in integration tests. To do that, you have to provide in the `prepare-environment` command the following flag: `TEST_DIRACX=Yes`. It will run DiracX alongside DIRAC, and use the available and activated legacy adapted services.
 
-To deactivate a service from being used with DiracX, you can set the `diracxClient` field to `None`:
+To deactivate a service from being used with DiracX, you can add it in `integration_tests.py` in the `DIRACX_DISABLED_SERVICES` list:
 
 .. code-block:: python
 
-    class MyOldClient(Client):
-      def __init__(self, **kwargs):
-          super().__init__(**kwargs)
-          self.setServer("...")
-
-      # Set to NULL to avoid using it in ClientSelector
-      diracxClient = None
+    DIRACX_DISABLED_SERVICES = [
+      "WorkloadManagement/JobMonitoring",
+    ]
 
 By setting `TEST_DIRACX=Yes` only, it will take the last version of DiracX by default. If you want to provide your own, you have to build your DiracX project, and provide the `dist` folder path when calling `prepare-client`. This path has to be absolute.
 

integration_tests.py

@@ -39,9 +39,12 @@
     "INSTALLATION_BRANCH": "",
     "DEBUG": "Yes",
 }
-# If TEST_DIRACX is set to yes, all variables in DIRACX_OPTIONS will also be set to "Yes"
-DIRACX_OPTIONS = ()
 DEFAULT_MODULES = {"DIRAC": Path(__file__).parent.absolute()}
+# All services that have a FutureClient, but we *explicitly* deactivate
+# (for example if we did not finish to develop it)
+DIRACX_DISABLED_SERVICES = [
+    # "WorkloadManagement/JobMonitoring",
+]
 
 # Static configuration
 DB_USER = "Dirac"
@@ -155,8 +158,9 @@ def list_commands(self, ctx):
 It will search for legacy adapted services (services with a future client activated)
 and do the necessary to make DIRAC work alongside DiracX.
 
-To deactivate a legacy adapted service (to pass CI for example), you have to set in the old
-client the `diracxClient` field to "None". Else, set it to the right future client instance.
+To deactivate a legacy adapted service (to pass CI for example), you have to add it in
+the `DIRACX_DISABLED_SERVICES` list. If you don't, the program will set this service to be used
+with DiracX, and if it is badly adapted, errors will be raised.
 
 > Note that you can provide a DiracX project (repository, branch) by building it and providing
 the dist folder to the prepare-environment command.
@@ -1184,10 +1188,16 @@ def _make_config(modules, flags, release_var, editable):
             typer.secho(f"Required feature variable {key!r} is missing", err=True, fg=c.RED)
             raise typer.Exit(code=1)
 
-    # If we test DiracX, enable all the options
+    # If we test DiracX, add specific config
     if config["TEST_DIRACX"].lower() in ("yes", "true"):
-        for key in DIRACX_OPTIONS:
-            config[key] = "Yes"
+        if DIRACX_DISABLED_SERVICES:
+            # We link all disabled services
+            # config["DIRACX_DISABLED_SERVICES"] = "Service1 Service2 Service3 ..."
+            diracx_disabled_services = " ".join(DIRACX_DISABLED_SERVICES)
+
+            typer.secho(f"The following services won't be legacy adapted: {diracx_disabled_services}", fg="yellow")
+
+            config["DIRACX_DISABLED_SERVICES"] = diracx_disabled_services
 
     config["TESTREPO"] = [f"/home/dirac/LocalRepo/TestCode/{name}" for name in modules]
     config["ALTERNATIVE_MODULES"] = [f"/home/dirac/LocalRepo/ALTERNATIVE_MODULES/{name}" for name in modules]

src/DIRAC/Core/Tornado/Client/ClientSelector.py

@@ -65,30 +65,28 @@ def ClientSelector(disetClient, *args, **kwargs):  # We use same interface as RP
         # If we are not already given a URL, resolve it
         if serviceName.startswith(("http", "dip")):
             completeUrl = serviceName
+        elif useLegacyAdapter(serviceName):
+            sLog.debug(f"Using legacy adapter for service {serviceName}")
+            if diracxClient is None:
+                raise NotImplementedError(
+                    "DiracX is enabled but no diracxClient is provided, do you need to update your client?"
+                )
+            return diracxClient()
         else:
-            isServiceLegacyAdapted = useLegacyAdapter(serviceName)
-            if isServiceLegacyAdapted and diracxClient is not None:
-                sLog.debug(f"Using legacy adapter for service {serviceName}")
-                return diracxClient()
-            else:
-                if isServiceLegacyAdapted:
-                    # Only warn to deactivate it without making the code crash.
-                    sLog.warn(
-                        f"FutureClient for {serviceName} is provided but not activated, do you need to update your client?"
-                    )
-
             completeUrl = getServiceURL(serviceName)
             sLog.debug(f"URL resolved: {completeUrl}")
 
         if completeUrl.startswith("http"):
             sLog.debug(f"Using HTTPS for service {serviceName}")
-            return tornadoClient(*args, **kwargs)
+            rpc = tornadoClient(*args, **kwargs)
+        else:
+            rpc = disetClient(*args, **kwargs)
     except Exception as e:  # pylint: disable=broad-except
         # If anything went wrong in the resolution, we return default RPCClient
         # So the behaviour is exactly the same as before implementation of Tornado
         sLog.warn("Could not select DISET or Tornado client", f"{repr(e)}")
-        sLog.exception()
-    return disetClient(*args, **kwargs)
+        rpc = disetClient(*args, **kwargs)
+    return rpc
 
 
 # Client to use for RPC selection

src/DIRAC/WorkloadManagementSystem/Client/JobMonitoringClient.py

@@ -14,13 +14,12 @@
 
 @createClient("WorkloadManagement/JobMonitoring")
 class JobMonitoringClient(Client):
+    diracxClient = None
+
     def __init__(self, **kwargs):
         super().__init__(**kwargs)
         self.setServer("WorkloadManagement/JobMonitoring")
 
-    # Set to NULL to avoid using it in ClientSelector
-    diracxClient = None
-
     @ignoreEncodeWarning
     def getJobsStatus(self, jobIDs):
         res = self._getRPC().getJobsStatus(jobIDs)

tests/CI/docker-compose.yml

@@ -278,7 +278,6 @@ services:
       - DIRACX_SERVICE_AUTH_STATE_KEY=uSNPPtZ1EbC5np13zOwmWJ84Duix753Hejzk/u/MQE4=
       # Obtained with echo 'InsecureChangeMe' | base64 -d | openssl sha256
       - DIRACX_LEGACY_EXCHANGE_HASHED_API_KEY=07cddf6948d316ac9d186544dc3120c4c6697d8f994619665985c0a5bf76265a
-      - DIRACX_SERVICE_JOBS_ENABLED=true
       - 'DIRACX_OS_DB_JOBPARAMETERSDB={"hosts": "elastic:changeme@opensearch:9200", "use_ssl": false, "verify_certs": false}'
       - DIRACX_SANDBOX_STORE_BUCKET_NAME=sandboxes
       - DIRACX_SANDBOX_STORE_AUTO_CREATE_BUCKET=true

tests/Jenkins/dirac_ci.sh

@@ -168,9 +168,23 @@ installSite() {
     # Only if we have TEST_DIRACX we can have a legacy_adapted service, or it will crash
     # "Missing mandatory /DiracX/URL configuration"
     # Call findFutureServices and read services into an array
-    mapfile -t futureServices < <(python -m DIRAC.Core.Utilities.Extensions findFutureServices | sed 's/System / /g' | sed 's/Handler//g' | sed 's/Client//g' | sed 's/ /\//g')
 
-    # If there are any services, add them to the args
+    # Prepare grep pattern to exclude disabled services
+    disabledPattern=$(printf "%s\n" $DIRACX_DISABLED_SERVICES | sed 's/^/^/; s/$/$/' | paste -sd'|' -)
+
+    # Load and filter future services using grep -vF
+    mapfile -t futureServices < <(
+      python -m DIRAC.Core.Utilities.Extensions findFutureServices |
+        sed 's/System / /g' |
+        sed 's/Handler//g' |
+        sed 's/Client//g' |
+        sed 's/ /\//g' |
+        grep -Ev "${disabledPattern}"
+    )
+
+    echo "Legacy adapted services after filtering: $(IFS=' '; echo "${futureServices[*]}")"
+
+    # If there are any remaining services, add them to args
     if [[ ${#futureServices[@]} -gt 0 ]]; then
       diracxSetupArgs+=(--legacy-adapted-services "${futureServices[@]}")
     fi