Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a withConnection context function for ensuring connections close in scripts #269

Open
azimov opened this issue Apr 3, 2024 · 0 comments
Labels
enhancement New functionality that could be added good first issue An issue that could be solved by someone new to the package

Comments

@azimov
Copy link
Contributor

azimov commented Apr 3, 2024

This was discussed in the last HADES meeting and I think it would be good to implement something that we can say is our best practice principles for working with db connections to ensure they aren't left hanging. Something I'm sure everyone is guilty of in scripts at some point.

Toy example

The following is along the lines of how I think the function should look. This heavily borrows from the withr package style.

library(DatabaseConnector)

withConnection <- function (connection, code)
{
  stopifnot(dbIsValid(connection))
  on.exit({
    if (dbIsValid(connection)) disconnect(connection)
  })
  force(code)
}

# Example
connectionDetails <- createConnectionDetails(server = ":memory:", dbms = "sqlite")
connection <- connect(connectionDetails)
withConnection(connection, {
  insertTable(connection, tableName = "cars", data = mtcars)
  result <- renderTranslateQuerySql(connection, "SELECT * FROM cars")
})

# Should print 50
print(nrow(cars))
# Should print FALSE
print(dbIsValid(connection))

Most OHDSI functions already use the on.exit to be clean but this approach is more general and could be applied everywhere.

Some semantic sugar could be added around the connection details instance so you auto assign a connection with this.

A part of any PR for this should be to update any guides and examples in this package to use the agreed standard practice for ensuring the connection is closed after usage.

@azimov azimov added enhancement New functionality that could be added good first issue An issue that could be solved by someone new to the package labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality that could be added good first issue An issue that could be solved by someone new to the package
Projects
None yet
Development

No branches or pull requests

1 participant