From f5101640e9d179b6983d2ce8d6c652eeb6fde99b Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 24 Jul 2023 13:38:15 +0100 Subject: [PATCH 1/4] Drop configurability of global resources --- R/metadata.R | 20 +++++++++++--------- man/orderly_global_resource.Rd | 7 +++---- man/orderly_plugin_register.Rd | 12 ++++++++++-- tests/testthat/helper-orderly.R | 2 -- tests/testthat/test-config.R | 23 ----------------------- tests/testthat/test-root.R | 15 --------------- tests/testthat/test-run.R | 6 +++--- vignettes/introduction.Rmd | 21 +++++---------------- 8 files changed, 32 insertions(+), 74 deletions(-) diff --git a/R/metadata.R b/R/metadata.R index 99126b84..d7089266 100644 --- a/R/metadata.R +++ b/R/metadata.R @@ -298,10 +298,9 @@ static_orderly_dependency <- function(args) { ##' Copy global resources into a packet directory. You can use this to ##' share common resources (data or code) between multiple packets. ##' Additional metadata will be added to keep track of where the files -##' came from. Using this function requires that the orderly -##' repository has global resources enabled, with a -##' `global_resources:` section in the `orderly_config.yml`; an error -##' will be raised if this is not configured. +##' came from. Using this function requires the global resources +##' directory `global/` exists at the orderly root; an error will be +##' raised if this is not configured when we attempt to fetch files. ##' ##' @title Copy global resources into a packet directory ##' @@ -341,16 +340,19 @@ validate_global_resource <- function(args) { copy_global <- function(path_root, path_dest, config, files) { - if (is.null(config$global_resources)) { - stop(paste("'global_resources' is not supported;", - "please edit orderly_config.yml to enable"), - call. = FALSE) + ## This used to be configurable in orderly1, but almost everyone + ## just kept it as 'global'. We might make it configurable later. + global_dir <- "global" + global_path <- file.path(path_root, global_dir) + if (!is_directory(global_path)) { + cli::cli_abort(sprintf( + "The global resources directory '%s' does not exist at orderly's root", + global_dir)) } here <- names(files) there <- unname(files) - global_path <- file.path(path_root, config$global_resources) assert_file_exists( there, workdir = global_path, name = sprintf("Global resources in '%s'", global_path)) diff --git a/man/orderly_global_resource.Rd b/man/orderly_global_resource.Rd index 49b4c073..ac2452ff 100644 --- a/man/orderly_global_resource.Rd +++ b/man/orderly_global_resource.Rd @@ -18,8 +18,7 @@ Undefined Copy global resources into a packet directory. You can use this to share common resources (data or code) between multiple packets. Additional metadata will be added to keep track of where the files -came from. Using this function requires that the orderly -repository has global resources enabled, with a -\verb{global_resources:} section in the \code{orderly_config.yml}; an error -will be raised if this is not configured. +came from. Using this function requires the global resources +directory \verb{global/} exists at the orderly root; an error will be +raised if this is not configured when we attempt to fetch files. } diff --git a/man/orderly_plugin_register.Rd b/man/orderly_plugin_register.Rd index 92976d1f..6d24a725 100644 --- a/man/orderly_plugin_register.Rd +++ b/man/orderly_plugin_register.Rd @@ -4,7 +4,13 @@ \alias{orderly_plugin_register} \title{Register an orderly plugin} \usage{ -orderly_plugin_register(name, config, serialise, cleanup = NULL, schema = NULL) +orderly_plugin_register( + name, + config, + serialise = NULL, + cleanup = NULL, + schema = NULL +) } \arguments{ \item{name}{The name of the plugin, typically the package name} @@ -24,7 +30,9 @@ list of all entries pushed in via \code{\link[=orderly_plugin_add_metadata]{orderly_plugin_add_metadata()}}; this is a named list with names corresponding to the \code{field} argument to \code{orderly_plugin_add_metadata} and each list element being an -unnamed list with values corresponding to \code{data}.} +unnamed list with values corresponding to \code{data}. If \code{NULL}, +then no serialisation is done, and no metadata from your plugin +will be added.} \item{cleanup}{Optionally, a function to clean up any state that your plugin uses. You can call \code{orderly_plugin_context} from diff --git a/tests/testthat/helper-orderly.R b/tests/testthat/helper-orderly.R index 95b091c4..548532cd 100644 --- a/tests/testthat/helper-orderly.R +++ b/tests/testthat/helper-orderly.R @@ -10,8 +10,6 @@ test_prepare_orderly_example <- function(examples, ...) { config <- readLines(file.path(tmp, "orderly_config.yml")) if (any(c("global", "global-dir") %in% examples)) { - config <- c(config, - "global_resources: global") fs::dir_create(file.path(tmp, "global")) if ("global" %in% examples) { fs::file_copy(test_path("examples/explicit/data.csv"), diff --git a/tests/testthat/test-config.R b/tests/testthat/test-config.R index 814baf7b..42c3a4e9 100644 --- a/tests/testthat/test-config.R +++ b/tests/testthat/test-config.R @@ -24,29 +24,6 @@ test_that("environment files must be really simple", { }) -test_that("can interpolate values into an orderly configuration", { - ## This is a silly test because we'd never normally interpolate in - ## the global resources - this is really to support databases via - ## the plugin.... - path <- test_prepare_orderly_example("global") - expect_equal(orderly_root(path, FALSE)$config$global_resources, "global") - - writeLines(c(empty_config_contents(), - "global_resources: $PATH_GLOBAL"), - file.path(path, "orderly_config.yml")) - expect_error( - orderly_root(path, FALSE), - paste0("Environment variable 'PATH_GLOBAL' is not set\n\t", - "(used in orderly_config.yml$global_resources)"), - fixed = TRUE) - withr::with_envvar( - c(PATH_GLOBAL = "global"), - expect_equal(orderly_root(path, FALSE)$config$global_resources, "global")) - writeLines("PATH_GLOBAL: global", file.path(path, "orderly_envir.yml")) - expect_equal(orderly_root(path, FALSE)$config$global_resources, "global") -}) - - test_that("can validate minimum required version", { expect_error( orderly_config_validate_minimum_orderly_version("1.4.5", "orderly.yml"), diff --git a/tests/testthat/test-root.R b/tests/testthat/test-root.R index b76dd457..1eab9a86 100644 --- a/tests/testthat/test-root.R +++ b/tests/testthat/test-root.R @@ -67,18 +67,3 @@ test_that("can turn an outpack root into an orderly one", { expect_equal(root2$config, list(minimum_orderly_version = numeric_version("1.99.0"))) }) - - -test_that("Can validate global resources", { - tmp <- tempfile() - on.exit(unlink(tmp, recursive = TRUE)) - root <- orderly_init(tmp, logging_console = FALSE) - writeLines(c(empty_config_contents(), - "global_resources: global"), - file.path(tmp, "orderly_config.yml")) - expect_error(orderly_config(tmp), - "Global resource directory does not exist: 'global'") - dir.create(file.path(tmp, "global")) - cfg <- orderly_config(tmp) - expect_equal(cfg$global_resources, "global") -}) diff --git a/tests/testthat/test-run.R b/tests/testthat/test-run.R index 9e1e1ffb..91fda75d 100644 --- a/tests/testthat/test-run.R +++ b/tests/testthat/test-run.R @@ -237,15 +237,15 @@ test_that("can validate global resource arguments", { test_that("can't use global resources if not enabled", { path <- test_prepare_orderly_example("global") - writeLines(empty_config_contents(), file.path(path, "orderly_config.yml")) + unlink(file.path(path, "global"), recursive = TRUE) env <- new.env() path_src <- file.path(path, "src", "global") err <- expect_error( orderly_run("global", root = path, envir = env), - "'global_resources' is not supported; please edit orderly_config.yml") + "The global resources directory 'global' does not exist at orderly's root") expect_error( withr::with_dir(path_src, sys.source("orderly.R", env)), - "'global_resources' is not supported; please edit orderly_config.yml") + "The global resources directory 'global' does not exist at orderly's root") }) diff --git a/vignettes/introduction.Rmd b/vignettes/introduction.Rmd index 0dcaf75c..896f9d72 100644 --- a/vignettes/introduction.Rmd +++ b/vignettes/introduction.Rmd @@ -161,7 +161,7 @@ The function `orderly2::orderly_dependency()` is designed to operate while the p * `orderly2::orderly_description()`: Provide a longer name and description for your report; this can be reflected in tooling that uses orderly metadata to be much more informative than your short name. * `orderly2::orderly_parameters()`: Declares parameters that can be passed in to control the behaviour of the report. Parameters are key-value pairs of simple data (booleans, numbers, strings) which your report can respond to. They can also be used in queries to `orderly2::orderly_dependency()` to find packets that satisfy some criteria. * `orderly2::orderly_resource()`: Declares that a file is a *resource*; a file that is an input to the the report, and which comes from this source directory. By default, orderly treats all files in the directory as a resource, but it can be useful to mark these explicitly, and necessary to do so in "strict mode" (see below). Files that have been marked as a resource are **immutable** and may not be deleted or modified. -* `orderly2::orderly_global_resource()`: Copies a file from a "global resources" directory, which can be data files or source code located at the root of the orderly repository. This can be a reasonable way of sharing data or commonly used code among several reports. +* `orderly2::orderly_global_resource()`: Copies a file from the "global resources" directory `global/`, which can be data files or source code located at the root of the orderly repository. This can be a reasonable way of sharing data or commonly used code among several reports. * `orderly2::orderly_artefact()`: Declares that a file (or set of files) will be created by this report, before it is even run. Doing this makes it easier to check that the report behaves as expected and can allow reasoning about what a related set of reports will do without running them. By declaring something as an artefact (especially in conjunction with "strict mode") it is also easier to clean up `src` directories that have been used in interactive development (see below). * `orderly2::orderly_dependency()`: Copy files from one packet into this packet as it runs, as seen above. * `orderly2::orderly_strict_mode()`: Declares that this report will be run in "strict mode" (see below). @@ -281,25 +281,14 @@ See the outpack query documentation for much more detail on this. Sometimes it is useful to share data between different reports, for example some common source utilities that don't warrant their own package, or some common data. -To do this, you should set up a global resources directory by editing the `orderly_config.yml` file to say: - -```{r, echo = FALSE, results = "asis"} -contents <- readLines(file.path(path, "orderly_config.yml")) -writeLines(c( - contents, - "global_resources: global"), - file.path(path, "orderly_config.yml")) -fs::dir_create(file.path(path, "global")) -write.csv(data.frame(x = 1:10, y = runif(10)), - file.path(path, "global/data.csv")) -yaml_output(readLines(file.path(path, "orderly_config.yml"))) -``` - -and creating a directory `global` at the orderly root (you can use whatever name you want, the directory and the configuration option must match, and the directory must exist). +To do this, create a directory `global` at the orderly root and put in it any files or directories you might want to share. Suppose our global directory contains a file `data.csv`: ```{r, echo = FALSE} +fs::dir_create(file.path(path, "global")) +write.csv(data.frame(x = 1:10, y = runif(10)), + file.path(path, "global/data.csv")) dir_tree(path) ``` From c6ac3d5499055e476d30027b0b2eec72a19f2f0b Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 24 Jul 2023 13:41:44 +0100 Subject: [PATCH 2/4] Drop validation entirely --- R/config.R | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/R/config.R b/R/config.R index 9a24dad6..03eceafa 100644 --- a/R/config.R +++ b/R/config.R @@ -17,8 +17,7 @@ orderly_config_yml_read <- function(path) { check <- list( minimum_orderly_version = orderly_config_validate_minimum_orderly_version, - plugins = orderly_config_validate_plugins, - global_resources = orderly_config_validate_global_resources) + plugins = orderly_config_validate_plugins) required <- "minimum_orderly_version" optional <- setdiff(names(check), required) @@ -51,15 +50,6 @@ orderly_config_validate_minimum_orderly_version <- function(value, filename) { } -orderly_config_validate_global_resources <- function(global_resources, - filename) { - if (!is.null(global_resources)) { - assert_is_directory(global_resources, name = "Global resource directory") - global_resources - } -} - - orderly_config_validate_plugins <- function(plugins, filename) { if (is.null(plugins)) { return(NULL) From 488016ac381fbdd8d6c08dc0fe46d5f846f1d932 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Mon, 24 Jul 2023 14:02:45 +0100 Subject: [PATCH 3/4] Update docs --- man/orderly_plugin_register.Rd | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/man/orderly_plugin_register.Rd b/man/orderly_plugin_register.Rd index 6d24a725..92976d1f 100644 --- a/man/orderly_plugin_register.Rd +++ b/man/orderly_plugin_register.Rd @@ -4,13 +4,7 @@ \alias{orderly_plugin_register} \title{Register an orderly plugin} \usage{ -orderly_plugin_register( - name, - config, - serialise = NULL, - cleanup = NULL, - schema = NULL -) +orderly_plugin_register(name, config, serialise, cleanup = NULL, schema = NULL) } \arguments{ \item{name}{The name of the plugin, typically the package name} @@ -30,9 +24,7 @@ list of all entries pushed in via \code{\link[=orderly_plugin_add_metadata]{orderly_plugin_add_metadata()}}; this is a named list with names corresponding to the \code{field} argument to \code{orderly_plugin_add_metadata} and each list element being an -unnamed list with values corresponding to \code{data}. If \code{NULL}, -then no serialisation is done, and no metadata from your plugin -will be added.} +unnamed list with values corresponding to \code{data}.} \item{cleanup}{Optionally, a function to clean up any state that your plugin uses. You can call \code{orderly_plugin_context} from From 8f9e6581746762242a0975459d0445f5e5bce0e4 Mon Sep 17 00:00:00 2001 From: Rich FitzJohn Date: Wed, 19 Jul 2023 16:56:35 +0100 Subject: [PATCH 4/4] Install outpack_server on gha --- .github/workflows/R-CMD-check.yaml | 8 ++++++++ .github/workflows/test-coverage.yaml | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 5870d208..c762b057 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -42,6 +42,14 @@ jobs: http-user-agent: ${{ matrix.config.http-user-agent }} use-public-rspm: true + # This does take a minute or two to install, and we could cache + # it but that's not super easy while still allowing easy + # updating. Once things stabilise we might tag outpack server + # releases and then we can install and cache against that. + - name: setup server + run: | + cargo install --git https://github.com/mrc-ide/outpack_server --branch mrc-4364 + - uses: r-lib/actions/setup-r-dependencies@v2 with: extra-packages: any::rcmdcheck diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml index c4cee67e..96f9ebe6 100644 --- a/.github/workflows/test-coverage.yaml +++ b/.github/workflows/test-coverage.yaml @@ -21,6 +21,10 @@ jobs: with: use-public-rspm: true + - name: setup server + run: | + cargo install --git https://github.com/mrc-ide/outpack_server --branch mrc-4364 + - uses: r-lib/actions/setup-r-dependencies@v2 with: extra-packages: any::covr