[r] Use cached SOMA context rather than re-creating #2988
Conversation
Cache a SOMA context during `$new()` as a private field when instantiating new objects. This cached context can then be used in other methods. Note, this PR is not yet changing the UX of config/context options. As the context from `soma_context()` is cached globally, we can modify the global one before instantiating to test customized cached contexts Modified SOMA methods: - `TileDBObject$new()`: creates context and caches it within the object - `TileDBGroup$create()`: no longer caches context, instead uses cached context from `$new()` - `TileDBGroup$open()`: uses cached context rather than calling `soma_context()` [SC-55084](https://app.shortcut.com/tiledb-inc/story/55084/)
| @@ -38,6 +38,13 @@ TileDBObject <- R6::R6Class( | |||
| private$.tiledbsoma_ctx <- tiledbsoma_ctx | |||
| private$.tiledb_ctx <- self$tiledbsoma_ctx$context() | |||
|
|
|||
| # TODO: re-enable once new UX is worked out | |||
| # soma_context <- soma_context %||% soma_context() | |||
There was a problem hiding this comment.
I think we want this active to be able to test as we just did with different regions. So let's expand the signature, and label the accessor as temporary. It's not in the 'official' user facing factory method.
There was a problem hiding this comment.
We may also want to add the 'existence proof test' in a test file, possibly commented out (and we don't call aws, usually) -- but for illustration?
There was a problem hiding this comment.
Because soma_context() caches globally, my thought process was that we could use the global context for modifications without changing the signature
soma_context(config = c(vfs.s3.region = "us-west-2"))
grp <- TileDBGroup$new(uri, ...)If you feel strongly otherwise, I can change this
There was a problem hiding this comment.
Ah, fair, but that is kinda a hidden feature I will have to take out at some point?
But agreed, for now it can stay.
Bump develop version Add commented existence proof via cached global context [ci skip]
mojaveazure
deleted the
paulhoffman/sc-55084/use-cached-soma-context-in-tiledbsoma-r-objects
branch
|
Good additions in the last comit. |
Cache a SOMA context during
$new()as a private field when instantiating new objects. This cached context can then be used in other methods. Note, this PR is not yet changing the UX of config/context options. As the context fromsoma_context()is cached globally, we can modify the global one before instantiating to test customized cached contextsModified SOMA methods:
TileDBObject$new(): creates context and caches it within the objectTileDBGroup$create(): no longer caches context, instead uses cached context from$new()TileDBGroup$open(): uses cached context rather than callingsoma_context()SC-55084