ctsit / redcapcustodian

Simplified, automated data management on REDCap systems
Other
12 stars 6 forks source link

Store redcap connection object on environment in connect_to_redcap_db #14

Closed pbchase closed 2 years ago

ChemiKyle commented 2 years ago

With the changes proposed to get the function working, I noticed that the returned conn object and the package-scoped conn object are distinct entities. Notably, closing the returned object does not close the package-scoped object.


Additional functions provided to redcap.R

#' Get info on the primary REDCap database
#'
#' @return The results of DBI::dbGetInfo
#'
#' @export
#' @examples
#' \dontrun{
#' get_db_info()
#' }
get_db_info <- function() {
  if (!exists("redcapcustodian.env$conn")) {
    warning("You have no connection!")
  }

  return(DBI::dbGetInfo(redcapcustodian.env$conn))
}

#' Returns the result of a user provided query
#'
#' @param query SQL string to be sent to the REDCap database
#' @return A dataframe resulting from the provided query
#'
#' @export
#' @examples
#' \dontrun{
#' get_db_query("SELECT * FROM redcap_user_information LIMIT 1")
#' }
get_db_query <- function(query) {
  if (!DBI::dbIsValid(redcapcustodian.env$conn)) {
    warning("You do not appear to have a valid connection to a REDCap database, attempting to connect...")
    connect_to_redcap_db()
  }

  df <- DBI::dbGetQuery(redcapcustodian.env$conn,
                        query
                        )

  return(df)
}

Script used to discover this issue:

# remove current redcapcustodian from scope in case it's just been rebuilt
detach("package:redcapcustodian", unload = T)

library(redcapcustodian)
library(dotenv)

load_dot_env("hosts/example/env/.env")

Sys.getenv("REDCAP_DB_NAME")

my_conn <- connect_to_redcap_db()

# spits out info, but also the warning
redcapcustodian::get_db_info()

# successfully returns 1 row of data
redcapcustodian::get_db_query("SELECT * FROM redcap_user_information LIMIT 1")

DBI::dbGetInfo(my_conn)
DBI::dbIsValid(my_conn)

DBI::dbDisconnect(my_conn)

# check we really closed this
DBI::dbIsValid(my_conn)

# invalid connection, as desired
DBI::dbGetInfo(my_conn)

# Package scoped connection was not closed by previous commend
# Warning still shows
redcapcustodian::get_db_info()

# returns 1 row of data becuase the package-scoped connection is indeed still open
redcapcustodian::get_db_query("SELECT * FROM redcap_user_information LIMIT 1")
pbchase commented 2 years ago

My new commit addresses the flaws in my initial PR.

My work does not include the new functions you propose. I'm not excited about making new versions of every DBI function just so I don't have to store and pass conn. My thought was to use the stored object to allow our novel functions that need a connection to the REDCap DB to not have to rely on the passed value for the connection.

ChemiKyle commented 2 years ago

For clarity's sake, I wasn't proposing those, just using them as proof of concept.
This schism actually works in my favor in the case of multi-instance juggling.