Configure the rrq offload directory each time a controller is created. #145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We previously setup the rrq offload directory once when creating the queue, storing the absolute path to it in the redis database. Using a single global value breaks down when that directory is mounted on multiple machines but at differing paths. The most spectacular failure case happens when configuring the queue from a linux machine and using it to run on a Windows cluster. When this happens, the offload path looks something like `/home/<user>/path/to/project/hipercow/rrq/offload`, which when used from a Windows machine obviously does not work. This is solved by configuring each instance of an `rrq_controller` with its own path to the offload directory, relative to the hipercow root. This root is already automatically adjusted based on where we are running.
|
This requires mrc-ide/rrq#125 to be released first. It will also require a bootstrap library update with the new versions of both rrq and hipercow. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #145 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 2377 2392 +15
=========================================
+ Hits 2377 2392 +15 ☔ View full report in Codecov by Sentry. |
richfitz
approved these changes
Nov 11, 2024
| @@ -33,12 +32,30 @@ hipercow_rrq_controller <- function(..., set_as_default = TRUE, driver = NULL, | |||
| queue_id = NULL, root = NULL) { | |||
| root <- hipercow_root(root) | |||
| call <- rlang::current_env() | |||
|
|
|||
There was a problem hiding this comment.
Perhaps loadNamespace with versionCheck here to make sure we have 0.7.20 or later
There was a problem hiding this comment.
Used compareVersion instead for nicer error messages.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We previously setup the rrq offload directory once when creating the queue, storing the absolute path to it in the redis database. Using a single global value breaks down when that directory is mounted on multiple machines but at differing paths.
The most spectacular failure case happens when configuring the queue from a linux machine and using it to run on a Windows cluster. When this happens, the offload path looks something like
/home/<user>/path/to/project/hipercow/rrq/offload, which when used from a Windows machine obviously does not work.This is solved by configuring each instance of an
rrq_controllerwith its own path to the offload directory, relative to the hipercow root. This root is already automatically adjusted based on where we are running.