ctsit / redcapcustodian

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

init_etl() fails #58

Closed pbchase closed 2 years ago

pbchase commented 2 years ago

init_etl() fails if the database driver is not specified

> library(REDCapR)
> library(dotenv)
> library(redcapcustodian)
> library(DBI)
> library(RMariaDB)
> init_etl("cleanup_bad_email_addresses")
Warning message:
In value[[3L]](cond) : Failed to connect to log DB: ctsi_rcc 
The reason given was:
 Error in (function (classes, fdef, mtable) : unable to find an inherited method for function ‘dbConnect’ for signature ‘"NULL"’

init_etl has a DB driver parameter with a NULL default at https://github.com/ctsit/redcapcustodian/blob/7548508958a025de5158ae202768fe89daa8aa63/R/utils.R#L56

This null gets passed all the way down the chain to DBI::dbConnect where it fails with the above error. You can prevent that error by setting the option e.g., init_etl("cleanup_bad_email_addresses", log_db_drv = RMariaDB::MariaDB()), yet this seems tedious. I feel like the better solution is to set a non-default value for log_db_drv in init_etl. e.g.

init_etl <- function(script_name = "", fake_runtime = NULL, log_db_drv = RMariaDB::MariaDB()) {
mbentz-uf commented 2 years ago

Do you have these set in your .env:

LOG_DB_NAME
LOG_DB_HOST
LOG_DB_USER
LOG_DB_PASSWORD
LOG_DB_SCHEME
LOG_DB_PORT
pbchase commented 2 years ago

Do you have these set in your .env:

LOG_DB_NAME
LOG_DB_HOST
LOG_DB_USER
LOG_DB_PASSWORD
LOG_DB_SCHEME
LOG_DB_PORT

Yes. Without them, my fix would not work

mbentz-uf commented 2 years ago

I think I misunderstood, do you just want me to review the PR? I got confused by "I figure you are bested qualified to tell me I;m doing it wrong"

pbchase commented 2 years ago

I can see your code has the intent to set a default DV driver of RMariaDB::MariaDB(), but it didn't work for me. I am proposing a different way to set a default DB driver of RMariaDB::MariaDB().

I made it work, but maybe my design is garbage. Maybe I compromised some other functionality in the process. It's these latter issues that make me think you'd be the best person to talk to about my solution.

mbentz-uf commented 2 years ago

Addressed by #59