Remove local library attachment in module run script (#107)

* Remove local library attachment in module run script * Embed unlockKeyring function in module scripts and remove custom assertion to support this * Disable Redshift tests for now
OHDSI · Dec 18, 2023 · a185331 · a185331
1 parent 00d0c00
commit a185331
Show file tree

Hide file tree

Showing 10 changed files with 710 additions and 1,188 deletions.
diff --git a/R/ModuleEnv.R b/R/ModuleEnv.R
@@ -47,16 +47,13 @@
 #' @param injectVars        list of var names list(name=value) to replace (e.g. replace list(foo = "some string") will
 #'                          find the pattern foo and replace it with the string some string - be careful!
 #' @param tempScriptFile    tempFile to write script to
-#' @param useLocalStrategusLibrary Use the locally installed Strategus library? TRUE will use the Strategus
-#'                          installation from the calling R process.
 #' @param job               run as rstudio job
 #' @param processName       String name for process
 #' @returns NULL invisibly
 withModuleRenv <- function(code,
                            moduleFolder,
                            injectVars = list(),
                            tempScriptFile = tempfile(fileext = ".R"),
-                           useLocalStrategusLibrary = TRUE,
                            job = FALSE,
                            processName = paste(moduleFolder, "_renv_run")) {
   # convert human readable code to a string for writing
@@ -81,18 +78,9 @@ withModuleRenv <- function(code,
     }
   }
 
-  # Enforce attachment of Strategus from calling process - note one inside the renv
-  if (useLocalStrategusLibrary) {
-    script <- c(.getLocalLibraryScipt("Strategus"), script)
-    # Adding Strategus dependencies to the script
-    script <- c(.getLocalLibraryScipt("ParallelLogger"), script)
-    script <- c(.getLocalLibraryScipt("CohortGenerator"), script)
-    script <- c(.getLocalLibraryScipt("DatabaseConnector"), script)
-    script <- c(.getLocalLibraryScipt("keyring"), script)
-    script <- c(.getLocalLibraryScipt("openssl"), script)
-    script <- c(.getLocalLibraryScipt("dplyr"), script)
-    script <- c(.getLocalLibraryScipt("R6"), script)
-  }
+  # Import the Strategus functions we need to use in the module scripts
+  script <- c("retrieveConnectionDetails <- ", base::deparse(Strategus::retrieveConnectionDetails), script)
+  script <- c("unlockKeyring <- ", base::deparse(Strategus::unlockKeyring), script)
 
   # Write file and execute script inside an renv
   fileConn <- file(tempScriptFile)

diff --git a/R/ResultModelCreation.R b/R/ResultModelCreation.R
@@ -200,7 +200,7 @@ runSchemaCreation <- function(analysisSpecifications, keyringSettings, moduleInd
       if (is.function(createDataModelSchema)) {
         # If the keyring is locked, unlock it, set the value and then re-lock it
         keyringName <- jobContext$keyringSettings$keyringName
-        keyringLocked <- Strategus::unlockKeyring(keyringName = keyringName)
+        keyringLocked <- unlockKeyring(keyringName = keyringName)
 
         resultsConnectionDetails <- keyring::key_get(jobContext$moduleExecutionSettings$resultsConnectionDetailsReference, keyring = keyringName)
         resultsConnectionDetails <- ParallelLogger::convertJsonToSettings(resultsConnectionDetails)

diff --git a/R/RunModule.R b/R/RunModule.R
@@ -71,16 +71,8 @@ runModule <- function(analysisSpecifications, keyringSettings, moduleIndex, exec
       source("Main.R")
       jobContext <- readRDS(jobContextFileName)
 
-      unlockKeyring <- function(keyringName) {
-        # If the keyring is locked, unlock it, set the value and then re-lock it
-        keyringLocked <- keyring::keyring_is_locked(keyring = keyringName)
-        if (keyringLocked) {
-          keyring::keyring_unlock(keyring = keyringName, password = Sys.getenv("STRATEGUS_KEYRING_PASSWORD"))
-        }
-        return(keyringLocked)
-      }
-
       keyringName <- jobContext$keyringSettings$keyringName
+      # unlockKeyring will be injected automatically
       keyringLocked <- unlockKeyring(keyringName = keyringName)
 
       ParallelLogger::addDefaultFileLogger(file.path(jobContext$moduleExecutionSettings$resultsSubFolder, "log.txt"))
@@ -95,15 +87,16 @@ runModule <- function(analysisSpecifications, keyringSettings, moduleIndex, exec
         renv::use(lockfile = "renv.lock")
       }
 
-      # NOTE injected variable isResultsExecution - will look strange outside of Strategus definition
+      # NOTE: injected variable isResultsExecution - will look strange outside of Strategus definition
+      # NOTE: retrieveConnectionDetails function is injected by withModuleRenv
       if (isCdmExecution) {
-        connectionDetails <- Strategus::retrieveConnectionDetails(
+        connectionDetails <- retrieveConnectionDetails(
           connectionDetailsReference = jobContext$moduleExecutionSettings$connectionDetailsReference,
           keyringName = keyringName
         )
         jobContext$moduleExecutionSettings$connectionDetails <- connectionDetails
       } else {
-        resultsConnectionDetails <- Strategus::retrieveConnectionDetails(
+        resultsConnectionDetails <- retrieveConnectionDetails(
           connectionDetailsReference = jobContext$moduleExecutionSettings$resultsConnectionDetailsReference,
           keyringName = keyringName
         )

diff --git a/R/Settings.R b/R/Settings.R
@@ -340,21 +340,15 @@ unlockKeyring <- function(keyringName) {
   # If the keyring is locked, unlock it, set the value and then re-lock it
   keyringLocked <- keyring::keyring_is_locked(keyring = keyringName)
   if (keyringLocked) {
-    assertKeyringPassword(x = Sys.getenv("STRATEGUS_KEYRING_PASSWORD"), keyringName = keyringName)
+    x <- Sys.getenv("STRATEGUS_KEYRING_PASSWORD")
+    if (length(x) == 0 || x == "") {
+      stop(paste0("STRATEGUS_KEYRING_PASSWORD NOT FOUND. STRATEGUS_KEYRING_PASSWORD must be set using Sys.setenv(STRATEGUS_KEYRING_PASSWORD = \"<your password>\") to unlock the keyring: ", keyringName))
+    }
     keyring::keyring_unlock(keyring = keyringName, password = Sys.getenv("STRATEGUS_KEYRING_PASSWORD"))
   }
   return(keyringLocked)
 }
 
-#' @keywords internal
-.checkKeyringPasswordSet <- function(x, keyringName = NULL) {
-  if (length(x) == 0 || x == "") {
-    return(paste0("STRATEGUS_KEYRING_PASSWORD NOT FOUND. STRATEGUS_KEYRING_PASSWORD must be set using Sys.setenv(STRATEGUS_KEYRING_PASSWORD = \"<your password>\") to unlock the keyring: ", keyringName))
-  } else {
-    return(TRUE)
-  }
-}
-
 #' Used when serializing connection details to retain NULL values
 #'
 #' @keywords internal

diff --git a/R/Strategus.R b/R/Strategus.R
@@ -24,5 +24,3 @@
 #' @importFrom methods is
 NULL
 
-# Add custom asssertions
-assertKeyringPassword <- checkmate::makeAssertionFunction(.checkKeyringPasswordSet)