Improve validation #153
Improve validation #153
Conversation
* Correctly access pool metadata, respecting `validateQuery` and ensuring cache actually works. * Validate on creation to ensure problems are revealed as soon as possible. Fixes #133
|
To do:
|
|
When I reinstall the package and attempt to run Could that be because the Forgive me if I'm wrong, I don't fully understand how testthat works within package development. |
|
@michaelbonilla yeah that was just a dumb mistake on my behalf. |
|
Everything works as expected for me again. |
|
Is there any update on this? We are still waiting on this PR and the next release to CRAN before being able to update it in our Package Manager and use Pool in production. |
|
@michaelbonilla just waiting for @jcheng5 to have some free time to review. I will see him in person next week so maybe I'll be able to force his eyeballs to look at this 😄 |
R/DBI-pool.R
Outdated
| @@ -24,7 +24,7 @@ setMethod("onDestroy", "DBIConnection", function(object) { | |||
| #' @export | |||
| #' @rdname object | |||
| setMethod("onValidate", "DBIConnection", function(object) { | |||
| pool <- attr(object, "..metadata", exact = TRUE)$pool | |||
| pool <- pool_metadata(object)$pool | |||
There was a problem hiding this comment.
This is the change that fixed the underlying problem: now we have a helper that ensures we're always accessing the correct attribute.
R/DBI-pool.R
Outdated
| "select count(*) from SYS_TABLES", | ||
| "select count(*) from SYS.TABLES" | ||
| # informix | ||
| "SELECT 1 FROM systables WHERE 0=1", |
There was a problem hiding this comment.
Introduced 0=1 to ensure query always runs quickly.
#Conflicts: # DESCRIPTION # NEWS.md # R/DBI-pool.R # R/pool.R # tests/testthat/_snaps/release.md # tests/testthat/test-release.R
validateQueryand ensuring cache actually works.Fixes #133