Invalid connection when doing initial DB build with custom db_connection

[awgymer]

When using building the DB for the first time or using force_rebuild an Error: Invalid connection is raised (using RPostgres - suspect error message may differ between drivers) which I believe is caused by build_db calling DBI::dbDisconnect(connection). This is not a problem when using the default RSQLite connection because the following code is run after the initial build/rebuild code which re-establishes an SQLite connection:

if (is.null(db_connection)) {
    connection <- DBI::dbConnect(RSQLite::SQLite(), db)
  } else {
    connection <- db_connection
  }

I'm not overly familiar with DBI::DBIConnection objects but as far as I know there isn't a way to reconnect? Perhaps the best solution to this would be a disconnect parameter to build_db - so that it can still run independently from update_db where necessary - but when a custom connection is used in update_db and you are calling it you don't disconnect?

Something like the following:

build_db <- function(dbdir = ".", dbname = "pbp_db", tblname = "nflfastR_pbp",  db_conn, disconnect=TRUE) {
     ...
     if(disconnect){
          DBI::dbDisconnect(connection
     }
}

update_db <- function(dbdir = ".",
                      dbname = "pbp_db",
                      tblname = "nflfastR_pbp",
                      force_rebuild = FALSE,
                      db_connection = NULL) {

    ...
    if (!file.exists(db) & is.null(db_connection)) {
    message(glue::glue("Can't find database {db}. Will try to create it and load the play by play data into the data table \"{tblname}\"."))
    build_db(dbdir, dbname, tblname, db_connection)
  } else if (file.exists(db) & force_rebuild & is.null(db_connection)) {
    message(glue::glue("Start rebuilding the data table \"{tblname}\" in your database {db}."))
    build_db(dbdir, dbname, tblname, db_connection)
  } else if (force_rebuild & !is.null(db_connection)) {
    message(glue::glue("Start rebuilding the data table in your connected database."))
    build_db(dbdir, dbname, tblname, db_connection, disconnect=FALSE)
  }

[awgymer]

I think this will also be problematic when there are lots of missing games actually:

if(length(missing) >= 50) {
    DBI::dbDisconnect(connection)
    message("The number of missing games is so large that rebuilding the database is more efficient.")
    build_db(dbdir, dbname, tblname, db_connection)
    if (is.null(db_connection)) {
      connection <- DBI::dbConnect(RSQLite::SQLite(), db)
    } else {
      connection <- db_connection
    }
    missing <- get_missing_games(completed_games, connection, tblname)
  }

would probably need to become:

if(length(missing) >= 50) {
   if(is.null(db_connection)){DBI::dbDisconnect(connection)}
    message("The number of missing games is so large that rebuilding the database is more efficient.")
    if(is.null(db_connection)){build_db(dbdir, dbname, tblname, db_connection, FALSE)}
    else{build_db(dbdir, dbname, tblname, db_connection)}
    if (is.null(db_connection)) {
      connection <- DBI::dbConnect(RSQLite::SQLite(), db)
    } else {
      connection <- db_connection
    }
    missing <- get_missing_games(completed_games, connection, tblname)
  }

[mrcaseb]

Can you please provide a code example to reproduce the Error message? I am wondering if you are doing the connection correct as nobody else had those problems.

[mrcaseb]

Looked at it and think you are right. Should not disconnect if a custom connection is provided. Good catch, thanks!

[awgymer]

Just in case it does make a difference this is the code I use to connect. It's a local Postgres instance on my Mac.

conn <- DBI::dbConnect(RPostgres::Postgres(), dbname = dbname)
nflfastR::update_db(db_connection = conn, tblname = 'pbp', dbname = dbname, force_rebuild = force_rebuild)
DBI::dbDisconnect(conn)

[mrcaseb]

The issue got closed automatically as I merged in a potential fix now. If you are still having problems @awgymer please feel free to reopen the issue.