mrc-4177 include dependencies #113
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #113 +/- ##
==========================================
- Coverage 96.88% 96.82% -0.07%
==========================================
Files 14 14
Lines 1221 1227 +6
==========================================
+ Hits 1183 1188 +5
- Misses 38 39 +1
☔ View full report in Codecov by Sentry. |
DESCRIPTION
Outdated
| mrc-ide/context@mrc-4177, | ||
| mrc-ide/queuer@mrc-4177, |
There was a problem hiding this comment.
| mrc-ide/context@mrc-4177, | |
| mrc-ide/queuer@mrc-4177, | |
| mrc-ide/context, | |
| mrc-ide/queuer, |
once mrc-ide/context#21 and mrc-ide/queuer#31 are merged (both approved but unmerged at present)
R/client.R
Outdated
| ##' @param resource_count The number of resources to request | ||
| submit = function(path, name, template, cluster = NULL, | ||
| resource_type = "Cores", resource_count = 1) { | ||
| resource_type = "Cores", resource_count = 1, deps = "") { |
There was a problem hiding this comment.
we have to deal with this at some point, but why at the outside of this function? If you took NULL or a character vector then paste(deps, collapse = ",") would return the expected format for the endpoint, and would be possibly nicer to work with - that might even be better in client_body_submit as it's not totally different in spirit to the base64 encoding that happens there
There was a problem hiding this comment.
Also consider depends_on for consistency with below
There was a problem hiding this comment.
yeah fair, ok have moved close as possible to usage
R/queue_didehpc.R
Outdated
| } else { | ||
| deps <- obj$dide_id(depends_on[[id]]) | ||
| } | ||
| deps <- ifelse(length(deps) > 0, paste0(deps, collapse = ","), "") |
There was a problem hiding this comment.
The conditional is not needed (also ifelse is generally worth avoiding over an in-line if / else
)
Co-authored-by: Rich FitzJohn <rich.fitzjohn@gmail.com>
| @@ -15,13 +15,13 @@ Depends: | |||
| Imports: | |||
| conan (>= 0.1.1), | |||
| crayon, | |||
| context (>= 0.3.0), | |||
| context (>= 0.5.0), | |||
There was a problem hiding this comment.
Above on line 3 - bump version no to 0.3.21
Pass task dependencies to the web client. Depends on mrc-ide/queuer#31.