From 87bfacb0dedc96e0261b160a9a0882e66bd1d6f0 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 17:51:03 +0100 Subject: [PATCH 01/21] add json schema for req and res --- inst/schema/report_run_request.json | 25 +++++++++++++++++++++++++ inst/schema/report_run_response.json | 9 +++++++++ 2 files changed, 34 insertions(+) create mode 100644 inst/schema/report_run_request.json create mode 100644 inst/schema/report_run_response.json diff --git a/inst/schema/report_run_request.json b/inst/schema/report_run_request.json new file mode 100644 index 0000000..fab69f3 --- /dev/null +++ b/inst/schema/report_run_request.json @@ -0,0 +1,25 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "branch": { + "type": "string" + }, + "hash": { + "type": "string" + }, + "parameters": { + "oneOf": [ + { "type": "null" }, + { + "type": "object", + "properties": {}, + "additionalProperties": true + } + ] + } + } +} diff --git a/inst/schema/report_run_response.json b/inst/schema/report_run_response.json new file mode 100644 index 0000000..ea79816 --- /dev/null +++ b/inst/schema/report_run_response.json @@ -0,0 +1,9 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "job_id": { + "type": "string" + } + } +} From 96c68f37441df548c171174a442f591cd607dfe9 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 17:51:41 +0100 Subject: [PATCH 02/21] add documentation on submit function --- R/queue.R | 9 +++++++++ man/Queue.Rd | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/R/queue.R b/R/queue.R index 09a37f4..7badc1c 100644 --- a/R/queue.R +++ b/R/queue.R @@ -34,6 +34,15 @@ Queue <- R6::R6Class("Queue", #nolint controller = self$controller) }, + #' @description + #' Submit a job the Redis queue for runner to run. + #' + #' @param reportname Name of orderly report. + #' @param parameters Parameters to run the report with (default NULL) + #' @param branch Name of git branch to checkout the repository + #' (default master) + #' @param ref Git commit-ish value (e.g HEAD or commit hash or branch name). + #' We reset hard to this ref and run the report. (default HEAD) submit = function(reportname, parameters = NULL, branch = "master", ref = "HEAD") { run_args <- list( diff --git a/man/Queue.Rd b/man/Queue.Rd index 9a10d7f..983c628 100644 --- a/man/Queue.Rd +++ b/man/Queue.Rd @@ -24,6 +24,7 @@ Object for managing running jobs on the redis queue \subsection{Public methods}{ \itemize{ \item \href{#method-Queue-new}{\code{Queue$new()}} +\item \href{#method-Queue-submit}{\code{Queue$submit()}} \item \href{#method-Queue-number_of_workers}{\code{Queue$number_of_workers()}} \item \href{#method-Queue-finalize}{\code{Queue$finalize()}} } @@ -49,6 +50,31 @@ if NULL (default NULL)} } } \if{html}{\out{
}} +\if{html}{\out{}} +\if{latex}{\out{\hypertarget{method-Queue-submit}{}}} +\subsection{Method \code{submit()}}{ +Submit a job the Redis queue for runner to run. +\subsection{Usage}{ +\if{html}{\out{
}}\preformatted{Queue$submit(reportname, parameters = NULL, branch = "master", ref = "HEAD")}\if{html}{\out{
}} +} + +\subsection{Arguments}{ +\if{html}{\out{
}} +\describe{ +\item{\code{reportname}}{Name of orderly report.} + +\item{\code{parameters}}{Parameters to run the report with (default NULL)} + +\item{\code{branch}}{Name of git branch to checkout the repository +(default master)} + +\item{\code{ref}}{Git commit-ish value (e.g HEAD or commit hash or branch name). +We reset hard to this ref and run the report. (default HEAD)} +} +\if{html}{\out{
}} +} +} +\if{html}{\out{
}} \if{html}{\out{}} \if{latex}{\out{\hypertarget{method-Queue-number_of_workers}{}}} \subsection{Method \code{number_of_workers()}}{ From 570ae374829d05bd62548a7ca25788d1cb0390b8 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 17:52:35 +0100 Subject: [PATCH 03/21] add porcelain endpoint for submitting report run and add queue to state --- R/api.R | 22 +++++++++++++++++++++- R/porcelain.R | 10 ++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/R/api.R b/R/api.R index 5d2a5d4..0a1a5cb 100644 --- a/R/api.R +++ b/R/api.R @@ -17,8 +17,12 @@ ##' @export api <- function(root, validate = NULL, log_level = "info") { logger <- porcelain::porcelain_logger(log_level) + + # Set ORDERLY_RUNNER_QUEUE_ID to specify existing queue id + queue <- Queue$new(root) + api <- porcelain::porcelain$new(validate = validate, logger = logger) - api$include_package_endpoints(state = list(root = root)) + api$include_package_endpoints(state = list(root = root, queue = queue)) api } @@ -70,3 +74,19 @@ report_parameters <- function(root, ref, name) { ) }) } + +##' @porcelain +##' POST /report/run => json(report_run_response) +##' state root :: root +##' state queue :: queue +##' body data :: json(report_run_request) +submit_report_run <- function(root, queue, data) { + job_id <- queue$submit( + data$name, + branch = data$branch, + ref = data$hash, + parameters = data$parameters + ) + + list(job_id = job_id) +} diff --git a/R/porcelain.R b/R/porcelain.R index c073969..6bf4687 100644 --- a/R/porcelain.R +++ b/R/porcelain.R @@ -28,5 +28,15 @@ porcelain::porcelain_state(root = state$root), returning = porcelain::porcelain_returning_json("report_parameters"), validate = validate) + }, + "POST /report/run" = function(state, validate) { + porcelain::porcelain_endpoint$new( + "POST", + "/report/run", + submit_report_run, + porcelain::porcelain_input_body_json("data", "report_run_request"), + porcelain::porcelain_state(root = state$root, queue = state$queue), + returning = porcelain::porcelain_returning_json("report_run_response"), + validate = validate) }) } From 01e9bbc15a0662061999a358fc7caa0c04ba44d6 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 22:51:17 +0100 Subject: [PATCH 04/21] scalar job_id --- R/api.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/api.R b/R/api.R index 0a1a5cb..cf66449 100644 --- a/R/api.R +++ b/R/api.R @@ -88,5 +88,5 @@ submit_report_run <- function(root, queue, data) { parameters = data$parameters ) - list(job_id = job_id) + list(job_id = scalar(job_id)) } From 5afb8c7e3504de1e4a547be4ce275ea49dc9bce3 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 22:51:28 +0100 Subject: [PATCH 05/21] test --- tests/testthat/helper-orderly-runner.R | 11 +++-- tests/testthat/test-api.R | 64 +++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index 20cda70..b81e392 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -1,7 +1,10 @@ orderly_runner_endpoint <- function(method, path, root, validate = TRUE) { - porcelain::porcelain_package_endpoint("orderly.runner", method, path, - state = list(root = root), - validate = validate) + queue <- Queue$new(root) + porcelain::porcelain_package_endpoint( + "orderly.runner", method, path, + state = list(root = root, queue = queue), + validate = validate + ) } @@ -50,6 +53,7 @@ test_prepare_orderly_remote_example <- function(examples, ...) { path_local <- tempfile() withr::defer_parent(unlink(path_local, recursive = TRUE)) gert::git_clone(path_remote, path_local) + orderly2::orderly_init(root = path_local, force = TRUE) list( remote = path_remote, local = path_local @@ -69,6 +73,7 @@ copy_examples <- function(examples, path_src) { helper_add_git <- function(path, add = ".") { gert::git_init(path) + orderly2::orderly_gitignore_update("(root)", root = path) sha <- git_add_and_commit(path, add) branch <- gert::git_branch(repo = path) url <- "https://example.com/git" diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index 998ec1b..44e09d1 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -1,6 +1,9 @@ test_that("root data returns sensible, validated, data", { + # need this because queue is created when api starts + # and it expects an orderly directory + repo <- test_prepare_orderly_remote_example("data") ## Just hello world for the package really - endpoint <- orderly_runner_endpoint("GET", "/", NULL) + endpoint <- orderly_runner_endpoint("GET", "/", repo$local) res <- endpoint$run() expect_true(res$validated) expect_true(all(c("orderly2", "orderly.runner") %in% @@ -10,8 +13,8 @@ test_that("root data returns sensible, validated, data", { test_that("Can construct the api", { - root <- create_temporary_root(use_file_store = TRUE) - obj <- api(root) + repo <- test_prepare_orderly_remote_example("data") + obj <- api(repo$local) result <- evaluate_promise(value <- obj$request("GET", "/")$status) expect_equal(value, 200) logs <- lapply(strsplit(result$output, "\n")[[1]], jsonlite::parse_json) @@ -61,8 +64,11 @@ test_that("can list orderly reports", { test_that("can get parameters for a report", { repo <- test_prepare_orderly_remote_example(c("data", "parameters")) - endpoint <- orderly_runner_endpoint("GET", "/report//parameters", - repo$local) + endpoint <- orderly_runner_endpoint( + "GET", + "/report//parameters", + repo$local + ) res <- endpoint$run("HEAD", "data") expect_equal(res$status_code, 200) @@ -73,6 +79,52 @@ test_that("can get parameters for a report", { expect_equal(res$data, list( list(name = scalar("a"), value = NULL), list(name = scalar("b"), value = scalar("2")), - list(name = scalar("c"), value = NULL)) + list(name = scalar("c"), value = NULL) + )) +}) + +test_that("can run orderly reports", { + queue_id <- "orderly.runner:cute-animal" + repo <- test_prepare_orderly_example(c("data", "parameters")) + gert::git_init(repo) + orderly2::orderly_gitignore_update("(root)", root = repo) + git_add_and_commit(repo, ".") + queue <- Queue$new(repo, queue_id = queue_id) + worker_manager <- start_queue_workers_quietly( + 1, queue$controller + ) + make_worker_dirs(repo, worker_manager$id) + + endpoint <- withr::with_envvar( + c(ORDERLY_RUNNER_QUEUE_ID = queue_id), + orderly_runner_endpoint("POST", "/report/run", repo) + ) + + req <- list( + name = "data", + branch = gert::git_branch(repo = repo), + hash = gert::git_commit_id(repo = repo), + parameters = NULL + ) + + res <- endpoint$run(req) + rrq::rrq_task_wait(res$data$job_id, controller = queue$controller) + expect_equal( + rrq::rrq_task_status(res$data$job_id, controller = queue$controller), + "COMPLETE" + ) + + req <- list( + name = "parameters", + branch = gert::git_branch(repo = repo), + hash = gert::git_commit_id(repo = repo), + parameters = list(a = 1, c = 3) + ) + + res <- endpoint$run(req) + rrq::rrq_task_wait(res$data$job_id, controller = queue$controller) + expect_equal( + rrq::rrq_task_status(res$data$job_id, controller = queue$controller), + "COMPLETE" ) }) From e0956ac1442c3f95cb59c4f5d06dfc8b3199f6fa Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 22:58:13 +0100 Subject: [PATCH 06/21] revert gitignore update --- tests/testthat/helper-orderly-runner.R | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index b81e392..f815d8b 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -73,7 +73,6 @@ copy_examples <- function(examples, path_src) { helper_add_git <- function(path, add = ".") { gert::git_init(path) - orderly2::orderly_gitignore_update("(root)", root = path) sha <- git_add_and_commit(path, add) branch <- gert::git_branch(repo = path) url <- "https://example.com/git" From 4b314d8aac071484dcf79381f13fdaed49118f8d Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 23:07:42 +0100 Subject: [PATCH 07/21] add ability to skip queue creation for helper --- tests/testthat/helper-orderly-runner.R | 8 ++++++-- tests/testthat/test-api.R | 16 +++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index f815d8b..4c6e405 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -1,5 +1,9 @@ -orderly_runner_endpoint <- function(method, path, root, validate = TRUE) { - queue <- Queue$new(root) +orderly_runner_endpoint <- function( + method, path, root, + validate = TRUE, + skip_queue_creation = FALSE +) { + queue <- ifelse(skip_queue_creation, NULL, Queue$new(root)) porcelain::porcelain_package_endpoint( "orderly.runner", method, path, state = list(root = root, queue = queue), diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index 44e09d1..9c759f9 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -1,7 +1,4 @@ test_that("root data returns sensible, validated, data", { - # need this because queue is created when api starts - # and it expects an orderly directory - repo <- test_prepare_orderly_remote_example("data") ## Just hello world for the package really endpoint <- orderly_runner_endpoint("GET", "/", repo$local) res <- endpoint$run() @@ -13,8 +10,7 @@ test_that("root data returns sensible, validated, data", { test_that("Can construct the api", { - repo <- test_prepare_orderly_remote_example("data") - obj <- api(repo$local) + obj <- api(root) result <- evaluate_promise(value <- obj$request("GET", "/")$status) expect_equal(value, 200) logs <- lapply(strsplit(result$output, "\n")[[1]], jsonlite::parse_json) @@ -25,7 +21,10 @@ test_that("Can construct the api", { test_that("can list orderly reports", { repo <- test_prepare_orderly_remote_example(c("data", "parameters")) - endpoint <- orderly_runner_endpoint("GET", "/report/list", repo$local) + endpoint <- orderly_runner_endpoint( + "GET", "/report/list", + repo$local, skip_queue_creation = TRUE + ) res <- endpoint$run(gert::git_branch(repo$local)) expect_equal(res$status_code, 200) @@ -65,9 +64,8 @@ test_that("can list orderly reports", { test_that("can get parameters for a report", { repo <- test_prepare_orderly_remote_example(c("data", "parameters")) endpoint <- orderly_runner_endpoint( - "GET", - "/report//parameters", - repo$local + "GET", "/report//parameters", + repo$local, skip_queue_creation = TRUE ) res <- endpoint$run("HEAD", "data") From 851f0c198f5cc3fc1bc074e85013c13dd868c05a Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 23:09:28 +0100 Subject: [PATCH 08/21] skip if no redis --- tests/testthat/test-api.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index 9c759f9..917201e 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -1,6 +1,6 @@ test_that("root data returns sensible, validated, data", { ## Just hello world for the package really - endpoint <- orderly_runner_endpoint("GET", "/", repo$local) + endpoint <- orderly_runner_endpoint("GET", "/", NULL) res <- endpoint$run() expect_true(res$validated) expect_true(all(c("orderly2", "orderly.runner") %in% @@ -82,6 +82,8 @@ test_that("can get parameters for a report", { }) test_that("can run orderly reports", { + skip_if_no_redis() + queue_id <- "orderly.runner:cute-animal" repo <- test_prepare_orderly_example(c("data", "parameters")) gert::git_init(repo) From dc5ea275ca07bf1ca7789611aac1866c8a3f67fb Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 23:24:42 +0100 Subject: [PATCH 09/21] fix tests --- tests/testthat/helper-orderly-runner.R | 11 ++++++++--- tests/testthat/test-api.R | 10 +++++++--- tests/testthat/test-git.R | 4 +++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index 4c6e405..7aa9173 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -51,9 +51,11 @@ test_prepare_orderly_example <- function(examples, ...) { } -test_prepare_orderly_remote_example <- function(examples, ...) { +test_prepare_orderly_remote_example <- function( + examples, orderly_gitignore = FALSE, ... +) { path_remote <- test_prepare_orderly_example(examples, ...) - helper_add_git(path_remote) + helper_add_git(path_remote, orderly_gitignore) path_local <- tempfile() withr::defer_parent(unlink(path_local, recursive = TRUE)) gert::git_clone(path_remote, path_local) @@ -75,8 +77,11 @@ copy_examples <- function(examples, path_src) { } -helper_add_git <- function(path, add = ".") { +helper_add_git <- function(path, add = ".", orderly_gitignore = FALSE) { gert::git_init(path) + if (orderly_gitignore) { + orderly2::orderly_gitignore_update("(root)", root = path) + } sha <- git_add_and_commit(path, add) branch <- gert::git_branch(repo = path) url <- "https://example.com/git" diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index 917201e..ed6200a 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -1,6 +1,7 @@ test_that("root data returns sensible, validated, data", { ## Just hello world for the package really - endpoint <- orderly_runner_endpoint("GET", "/", NULL) + endpoint <- orderly_runner_endpoint("GET", "/", NULL, + skip_queue_creation = TRUE) res <- endpoint$run() expect_true(res$validated) expect_true(all(c("orderly2", "orderly.runner") %in% @@ -10,6 +11,7 @@ test_that("root data returns sensible, validated, data", { test_that("Can construct the api", { + root <- create_temporary_root(use_file_store = TRUE) obj <- api(root) result <- evaluate_promise(value <- obj$request("GET", "/")$status) expect_equal(value, 200) @@ -62,7 +64,9 @@ test_that("can list orderly reports", { test_that("can get parameters for a report", { - repo <- test_prepare_orderly_remote_example(c("data", "parameters")) + repo <- test_prepare_orderly_remote_example( + c("data", "parameters"), orderly_gitignore = TRUE + ) endpoint <- orderly_runner_endpoint( "GET", "/report//parameters", repo$local, skip_queue_creation = TRUE @@ -88,7 +92,7 @@ test_that("can run orderly reports", { repo <- test_prepare_orderly_example(c("data", "parameters")) gert::git_init(repo) orderly2::orderly_gitignore_update("(root)", root = repo) - git_add_and_commit(repo, ".") + git_add_and_commit(repo) queue <- Queue$new(repo, queue_id = queue_id) worker_manager <- start_queue_workers_quietly( 1, queue$controller diff --git a/tests/testthat/test-git.R b/tests/testthat/test-git.R index 2d49688..c95dbf6 100644 --- a/tests/testthat/test-git.R +++ b/tests/testthat/test-git.R @@ -23,7 +23,9 @@ test_that("can get default branch when remote origin is set", { test_that("can get files which have been modified", { testthat::skip_on_cran() - repo <- test_prepare_orderly_remote_example("data") + repo <- test_prepare_orderly_remote_example( + "data", orderly_gitignore = TRUE + ) copy_examples("parameters", repo$local) git_add_and_commit(repo$local) From dea652af0920c0d90abfd08a0ed51ee44eec26d3 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 23:54:13 +0100 Subject: [PATCH 10/21] fix tests --- R/api.R | 11 +++++++++-- tests/testthat/examples/data/data.R | 2 +- tests/testthat/examples/git-clean/git-clean.R | 2 +- tests/testthat/helper-orderly-runner.R | 12 +++++++----- tests/testthat/test-api.R | 6 ++---- tests/testthat/test-git.R | 4 +--- tests/testthat/test-zzz-e2e.R | 2 +- 7 files changed, 22 insertions(+), 17 deletions(-) diff --git a/R/api.R b/R/api.R index cf66449..915a0d2 100644 --- a/R/api.R +++ b/R/api.R @@ -15,11 +15,18 @@ ##' start the server ##' ##' @export -api <- function(root, validate = NULL, log_level = "info") { +api <- function( + root, validate = NULL, log_level = "info", + skip_queue_creation = FALSE +) { logger <- porcelain::porcelain_logger(log_level) # Set ORDERLY_RUNNER_QUEUE_ID to specify existing queue id - queue <- Queue$new(root) + if (skip_queue_creation) { + queue <- NULL + } else { + queue <- Queue$new(root) + } api <- porcelain::porcelain$new(validate = validate, logger = logger) api$include_package_endpoints(state = list(root = root, queue = queue)) diff --git a/tests/testthat/examples/data/data.R b/tests/testthat/examples/data/data.R index c7a6add..13ac9cf 100644 --- a/tests/testthat/examples/data/data.R +++ b/tests/testthat/examples/data/data.R @@ -1,3 +1,3 @@ -orderly2::orderly_artefact("Some data", "data.rds") +orderly2::orderly_artefact(description = "Some data", "data.rds") d <- data.frame(a = 1:10, x = runif(10), y = 1:10 + runif(10)) saveRDS(d, "data.rds") diff --git a/tests/testthat/examples/git-clean/git-clean.R b/tests/testthat/examples/git-clean/git-clean.R index 5115f7b..be10278 100644 --- a/tests/testthat/examples/git-clean/git-clean.R +++ b/tests/testthat/examples/git-clean/git-clean.R @@ -1,4 +1,4 @@ -orderly2::orderly_artefact("Some data", "data.rds") +orderly2::orderly_artefact(description = "Some data", "data.rds") d <- data.frame(a = 1:10, x = runif(10), y = 1:10 + runif(10)) write.table("test", file = file.path("..", "..", "inside_draft.txt")) write.table("test", file = file.path("..", "..", "..", "outside_draft.txt")) diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index 7aa9173..f80c3cf 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -3,7 +3,11 @@ orderly_runner_endpoint <- function( validate = TRUE, skip_queue_creation = FALSE ) { - queue <- ifelse(skip_queue_creation, NULL, Queue$new(root)) + if (skip_queue_creation) { + queue <- NULL + } else { + queue <- Queue$new(root) + } porcelain::porcelain_package_endpoint( "orderly.runner", method, path, state = list(root = root, queue = queue), @@ -51,11 +55,9 @@ test_prepare_orderly_example <- function(examples, ...) { } -test_prepare_orderly_remote_example <- function( - examples, orderly_gitignore = FALSE, ... -) { +test_prepare_orderly_remote_example <- function(examples, ...) { path_remote <- test_prepare_orderly_example(examples, ...) - helper_add_git(path_remote, orderly_gitignore) + helper_add_git(path_remote, orderly_gitignore = TRUE) path_local <- tempfile() withr::defer_parent(unlink(path_local, recursive = TRUE)) gert::git_clone(path_remote, path_local) diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index ed6200a..c182d6f 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -12,7 +12,7 @@ test_that("root data returns sensible, validated, data", { test_that("Can construct the api", { root <- create_temporary_root(use_file_store = TRUE) - obj <- api(root) + obj <- api(root, skip_queue_creation = TRUE) result <- evaluate_promise(value <- obj$request("GET", "/")$status) expect_equal(value, 200) logs <- lapply(strsplit(result$output, "\n")[[1]], jsonlite::parse_json) @@ -64,9 +64,7 @@ test_that("can list orderly reports", { test_that("can get parameters for a report", { - repo <- test_prepare_orderly_remote_example( - c("data", "parameters"), orderly_gitignore = TRUE - ) + repo <- test_prepare_orderly_remote_example(c("data", "parameters")) endpoint <- orderly_runner_endpoint( "GET", "/report//parameters", repo$local, skip_queue_creation = TRUE diff --git a/tests/testthat/test-git.R b/tests/testthat/test-git.R index c95dbf6..2d49688 100644 --- a/tests/testthat/test-git.R +++ b/tests/testthat/test-git.R @@ -23,9 +23,7 @@ test_that("can get default branch when remote origin is set", { test_that("can get files which have been modified", { testthat::skip_on_cran() - repo <- test_prepare_orderly_remote_example( - "data", orderly_gitignore = TRUE - ) + repo <- test_prepare_orderly_remote_example("data") copy_examples("parameters", repo$local) git_add_and_commit(repo$local) diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index fef2418..fcf2f88 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -1,6 +1,6 @@ skip_if_not_installed("httr") root <- test_prepare_orderly_example(c("data", "parameters")) -repo <- helper_add_git(root) +repo <- helper_add_git(root, orderly_gitignore = TRUE) bg <- porcelain::porcelain_background$new(api, list(root)) bg$start() on.exit(bg$stop()) From 319b4c6ef596fa2719e0d50be4f1af9639bbd8c2 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Thu, 1 Aug 2024 23:58:23 +0100 Subject: [PATCH 11/21] skip e2e test if no redis --- tests/testthat/test-zzz-e2e.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index fcf2f88..eece166 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -1,4 +1,6 @@ skip_if_not_installed("httr") +skip_if_no_redis() + root <- test_prepare_orderly_example(c("data", "parameters")) repo <- helper_add_git(root, orderly_gitignore = TRUE) bg <- porcelain::porcelain_background$new(api, list(root)) From 4cd8e9b903315a069b1ca8ec23e3175724a56305 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 2 Aug 2024 00:03:17 +0100 Subject: [PATCH 12/21] update docs --- R/api.R | 3 +++ man/api.Rd | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/R/api.R b/R/api.R index 915a0d2..cd84423 100644 --- a/R/api.R +++ b/R/api.R @@ -11,6 +11,9 @@ ##' @param log_level Logging level to use. Sensible options are "off", ##' "info" and "all". ##' +##' @param skip_queue_creation Skip queue creation, this is primarily +##' used for tests where we can't establish a Redis connection. +##' ##' @return A [porcelain::porcelain] object. Notably this does *not* ##' start the server ##' diff --git a/man/api.Rd b/man/api.Rd index e1fd003..72aa89e 100644 --- a/man/api.Rd +++ b/man/api.Rd @@ -4,7 +4,7 @@ \alias{api} \title{Create orderly runner} \usage{ -api(root, validate = NULL, log_level = "info") +api(root, validate = NULL, log_level = "info", skip_queue_creation = FALSE) } \arguments{ \item{root}{Orderly root} @@ -15,6 +15,9 @@ environments. See \link[porcelain:porcelain]{porcelain::porcelain} for details} \item{log_level}{Logging level to use. Sensible options are "off", "info" and "all".} + +\item{skip_queue_creation}{Skip queue creation, this is primarily +used for tests where we can't establish a Redis connection.} } \value{ A \link[porcelain:porcelain]{porcelain::porcelain} object. Notably this does \emph{not} From 2b50e2d50d199218aa31af2ad8d9c7361af43289 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 2 Aug 2024 01:16:43 +0100 Subject: [PATCH 13/21] add e2e test --- DESCRIPTION | 1 + R/api.R | 2 +- tests/testthat/test-zzz-e2e.R | 32 +++++++++++++++++++++++++++++--- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 34add46..add01c5 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -27,6 +27,7 @@ Imports: Suggests: fs, httr, + httr2, lgr, mockery, testthat (>= 3.0.0) diff --git a/R/api.R b/R/api.R index cd84423..a2a2a50 100644 --- a/R/api.R +++ b/R/api.R @@ -91,12 +91,12 @@ report_parameters <- function(root, ref, name) { ##' state queue :: queue ##' body data :: json(report_run_request) submit_report_run <- function(root, queue, data) { + data <- jsonlite::parse_json(data) job_id <- queue$submit( data$name, branch = data$branch, ref = data$hash, parameters = data$parameters ) - list(job_id = scalar(job_id)) } diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index eece166..1c15910 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -1,9 +1,15 @@ skip_if_not_installed("httr") +skip_if_not_installed("httr2") skip_if_no_redis() -root <- test_prepare_orderly_example(c("data", "parameters")) -repo <- helper_add_git(root, orderly_gitignore = TRUE) -bg <- porcelain::porcelain_background$new(api, list(root)) +root <- test_prepare_orderly_remote_example( + c("data", "parameters") +) +queue <- Queue$new(root$local, queue_id = queue_id) +worker_manager <- start_queue_workers_quietly(1, queue$controller) +make_worker_dirs(root$local, worker_manager$id) + +bg <- porcelain::porcelain_background$new(api, list(root$local)) bg$start() on.exit(bg$stop()) @@ -54,3 +60,23 @@ test_that("can get parameters", { list(name = "c", value = NULL) )) }) + +test_that("can run report", { + data <- list( + name = "data", + branch = gert::git_branch(repo = root$local), + hash = gert::git_commit_id(repo = root$local), + parameters = NULL + ) + + r <- httr2::request(bg$url("/report/run")) |> + httr2::req_body_json(list(data = data)) |> + httr2::req_perform() + + expect_equal(r$status_code, 200) + + dat <- r |> httr2::resp_body_json() + expect_equal(dat$status, "success") + expect_null(dat$errors) + expect_equal(is.character(dat$data$job_id), TRUE) +}) From b7f433753f84f0d11d34ec48bf889409771e6e37 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 2 Aug 2024 01:31:49 +0100 Subject: [PATCH 14/21] sync queue ids --- tests/testthat/test-zzz-e2e.R | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index 1c15910..733ec79 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -5,11 +5,15 @@ skip_if_no_redis() root <- test_prepare_orderly_remote_example( c("data", "parameters") ) +queue_id <- "orderly.runner:cute-animal" queue <- Queue$new(root$local, queue_id = queue_id) worker_manager <- start_queue_workers_quietly(1, queue$controller) make_worker_dirs(root$local, worker_manager$id) -bg <- porcelain::porcelain_background$new(api, list(root$local)) +bg <- withr::with_envvar( + c(ORDERLY_RUNNER_QUEUE_ID = queue_id), + porcelain::porcelain_background$new(api, list(root$local)) +) bg$start() on.exit(bg$stop()) From 3885f80d47b73bb7b312150322755b02f41ec380 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 2 Aug 2024 01:39:21 +0100 Subject: [PATCH 15/21] json string in endpoint --- tests/testthat/test-api.R | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index c182d6f..94864dd 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -103,13 +103,13 @@ test_that("can run orderly reports", { ) req <- list( - name = "data", - branch = gert::git_branch(repo = repo), - hash = gert::git_commit_id(repo = repo), - parameters = NULL + name = scalar("data"), + branch = scalar(gert::git_branch(repo = repo)), + hash = scalar(gert::git_commit_id(repo = repo)), + parameters = scalar(NULL) ) - res <- endpoint$run(req) + res <- endpoint$run(jsonlite::toJSON(req)) rrq::rrq_task_wait(res$data$job_id, controller = queue$controller) expect_equal( rrq::rrq_task_status(res$data$job_id, controller = queue$controller), @@ -117,13 +117,13 @@ test_that("can run orderly reports", { ) req <- list( - name = "parameters", - branch = gert::git_branch(repo = repo), - hash = gert::git_commit_id(repo = repo), - parameters = list(a = 1, c = 3) + name = scalar("parameters"), + branch = scalar(gert::git_branch(repo = repo)), + hash = scalar(gert::git_commit_id(repo = repo)), + parameters = list(a = scalar(1), c = scalar(3)) ) - res <- endpoint$run(req) + res <- endpoint$run(jsonlite::toJSON(req)) rrq::rrq_task_wait(res$data$job_id, controller = queue$controller) expect_equal( rrq::rrq_task_status(res$data$job_id, controller = queue$controller), From 018cfa9ea59ef4f75f57e78ae52dff5a110a6f83 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 2 Aug 2024 15:54:44 +0100 Subject: [PATCH 16/21] make schema stricter --- inst/schema/report_run_request.json | 4 +++- inst/schema/report_run_response.json | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/inst/schema/report_run_request.json b/inst/schema/report_run_request.json index fab69f3..76b01e0 100644 --- a/inst/schema/report_run_request.json +++ b/inst/schema/report_run_request.json @@ -21,5 +21,7 @@ } ] } - } + }, + "required": ["name", "branch", "hash", "parameters"], + "additionalProperties": false } diff --git a/inst/schema/report_run_response.json b/inst/schema/report_run_response.json index ea79816..1fd1405 100644 --- a/inst/schema/report_run_response.json +++ b/inst/schema/report_run_response.json @@ -5,5 +5,7 @@ "job_id": { "type": "string" } - } + }, + "required": ["job_id"], + "additionalProperties": false } From e15b628956f5d962ebf2e63740bb6c69531d2aca Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 2 Aug 2024 16:00:37 +0100 Subject: [PATCH 17/21] fix e2e test --- tests/testthat/test-zzz-e2e.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index 733ec79..7e4d99d 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -74,7 +74,7 @@ test_that("can run report", { ) r <- httr2::request(bg$url("/report/run")) |> - httr2::req_body_json(list(data = data)) |> + httr2::req_body_json(data) |> httr2::req_perform() expect_equal(r$status_code, 200) From 5549fcbf7bcb7454191cde57d82f05f23be0eced Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Sat, 3 Aug 2024 01:09:51 +0100 Subject: [PATCH 18/21] finally fix e2e test not working --- tests/testthat/helper-orderly-runner.R | 6 ++- tests/testthat/test-zzz-e2e.R | 57 +++++++++++++++++++------- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/tests/testthat/helper-orderly-runner.R b/tests/testthat/helper-orderly-runner.R index f80c3cf..b367907 100644 --- a/tests/testthat/helper-orderly-runner.R +++ b/tests/testthat/helper-orderly-runner.R @@ -119,8 +119,10 @@ start_queue_workers_quietly <- function(n_workers, worker_manager } -start_queue_with_workers <- function(root, n_workers, env = parent.frame()) { - q <- new_queue_quietly(root) +start_queue_with_workers <- function( + root, n_workers, env = parent.frame(), queue_id = NULL +) { + q <- new_queue_quietly(root, queue_id = queue_id) worker_manager <- start_queue_workers_quietly(n_workers, q$controller, env = env) make_worker_dirs(root, worker_manager$id) diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index 7e4d99d..a71a57a 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -2,17 +2,14 @@ skip_if_not_installed("httr") skip_if_not_installed("httr2") skip_if_no_redis() +queue_id <- "orderly.runner:cuteasdanimal" root <- test_prepare_orderly_remote_example( c("data", "parameters") ) -queue_id <- "orderly.runner:cute-animal" -queue <- Queue$new(root$local, queue_id = queue_id) -worker_manager <- start_queue_workers_quietly(1, queue$controller) -make_worker_dirs(root$local, worker_manager$id) - -bg <- withr::with_envvar( - c(ORDERLY_RUNNER_QUEUE_ID = queue_id), - porcelain::porcelain_background$new(api, list(root$local)) +queue <- start_queue_with_workers(root$local, 1, queue_id = queue_id) +bg <- porcelain::porcelain_background$new( + api, args = list(root$local), + env = c(ORDERLY_RUNNER_QUEUE_ID = queue_id) ) bg$start() on.exit(bg$stop()) @@ -70,17 +67,47 @@ test_that("can run report", { name = "data", branch = gert::git_branch(repo = root$local), hash = gert::git_commit_id(repo = root$local), - parameters = NULL + parameters = c(NULL) ) - r <- httr2::request(bg$url("/report/run")) |> - httr2::req_body_json(data) |> - httr2::req_perform() + body <- jsonlite::toJSON(data, null = "null", auto_unbox = TRUE) - expect_equal(r$status_code, 200) + r <- bg$request( + "POST", "/report/run", + body = body, + encode = "raw", + httr::content_type("application/json") + ) + + expect_equal(httr::status_code(r), 200) + dat <- httr::content(r) + + expect_equal(dat$status, "success") + expect_null(dat$errors) + expect_worker_task_complete(dat$data$job_id, queue$controller, 10) +}) + +test_that("can run report with params", { + data <- list( + name = "parameters", + branch = gert::git_branch(repo = root$local), + hash = gert::git_commit_id(repo = root$local), + parameters = list(a = 1, c = 3) + ) + + body <- jsonlite::toJSON(data, null = "null", auto_unbox = TRUE) + + r <- bg$request( + "POST", "/report/run", + body = body, + encode = "raw", + httr::content_type("application/json") + ) + + expect_equal(httr::status_code(r), 200) + dat <- httr::content(r) - dat <- r |> httr2::resp_body_json() expect_equal(dat$status, "success") expect_null(dat$errors) - expect_equal(is.character(dat$data$job_id), TRUE) + expect_worker_task_complete(dat$data$job_id, queue$controller, 10) }) From 5e533b6bfac251e45a2e875bcbd33d1d24020ddd Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Sat, 3 Aug 2024 01:13:24 +0100 Subject: [PATCH 19/21] clean up --- DESCRIPTION | 1 - tests/testthat/test-zzz-e2e.R | 1 - 2 files changed, 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index add01c5..34add46 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -27,7 +27,6 @@ Imports: Suggests: fs, httr, - httr2, lgr, mockery, testthat (>= 3.0.0) diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index a71a57a..cba5f01 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -1,5 +1,4 @@ skip_if_not_installed("httr") -skip_if_not_installed("httr2") skip_if_no_redis() queue_id <- "orderly.runner:cuteasdanimal" From fca17c82d60ade54b5df1b28b5b942954de475e6 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 16 Aug 2024 12:54:11 +0100 Subject: [PATCH 20/21] review comments --- .gitattributes | 1 + R/api.R | 4 ++-- inst/schema/report_run_request.json | 4 +++- inst/schema/report_run_response.json | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..7dc385e --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +R/porcelain.R linguist-generated=true diff --git a/R/api.R b/R/api.R index a2a2a50..02e2922 100644 --- a/R/api.R +++ b/R/api.R @@ -92,11 +92,11 @@ report_parameters <- function(root, ref, name) { ##' body data :: json(report_run_request) submit_report_run <- function(root, queue, data) { data <- jsonlite::parse_json(data) - job_id <- queue$submit( + task_id <- queue$submit( data$name, branch = data$branch, ref = data$hash, parameters = data$parameters ) - list(job_id = scalar(job_id)) + list(taskId = scalar(task_id)) } diff --git a/inst/schema/report_run_request.json b/inst/schema/report_run_request.json index 76b01e0..6f001d8 100644 --- a/inst/schema/report_run_request.json +++ b/inst/schema/report_run_request.json @@ -17,7 +17,9 @@ { "type": "object", "properties": {}, - "additionalProperties": true + "additionalProperties": { + "type": ["boolean", "integer", "number", "string"] + } } ] } diff --git a/inst/schema/report_run_response.json b/inst/schema/report_run_response.json index 1fd1405..e2cfd34 100644 --- a/inst/schema/report_run_response.json +++ b/inst/schema/report_run_response.json @@ -2,10 +2,10 @@ "$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "properties": { - "job_id": { + "taskId": { "type": "string" } }, - "required": ["job_id"], + "required": ["taskId"], "additionalProperties": false } From 104f5800991861476eb0f873f5727eef0792f1d4 Mon Sep 17 00:00:00 2001 From: M-Kusumgar Date: Fri, 16 Aug 2024 13:00:38 +0100 Subject: [PATCH 21/21] replace job_id with taskId --- tests/testthat/test-api.R | 8 ++++---- tests/testthat/test-zzz-e2e.R | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-api.R b/tests/testthat/test-api.R index 94864dd..1955759 100644 --- a/tests/testthat/test-api.R +++ b/tests/testthat/test-api.R @@ -110,9 +110,9 @@ test_that("can run orderly reports", { ) res <- endpoint$run(jsonlite::toJSON(req)) - rrq::rrq_task_wait(res$data$job_id, controller = queue$controller) + rrq::rrq_task_wait(res$data$taskId, controller = queue$controller) expect_equal( - rrq::rrq_task_status(res$data$job_id, controller = queue$controller), + rrq::rrq_task_status(res$data$taskId, controller = queue$controller), "COMPLETE" ) @@ -124,9 +124,9 @@ test_that("can run orderly reports", { ) res <- endpoint$run(jsonlite::toJSON(req)) - rrq::rrq_task_wait(res$data$job_id, controller = queue$controller) + rrq::rrq_task_wait(res$data$taskId, controller = queue$controller) expect_equal( - rrq::rrq_task_status(res$data$job_id, controller = queue$controller), + rrq::rrq_task_status(res$data$taskId, controller = queue$controller), "COMPLETE" ) }) diff --git a/tests/testthat/test-zzz-e2e.R b/tests/testthat/test-zzz-e2e.R index cba5f01..a690205 100644 --- a/tests/testthat/test-zzz-e2e.R +++ b/tests/testthat/test-zzz-e2e.R @@ -83,7 +83,7 @@ test_that("can run report", { expect_equal(dat$status, "success") expect_null(dat$errors) - expect_worker_task_complete(dat$data$job_id, queue$controller, 10) + expect_worker_task_complete(dat$data$taskId, queue$controller, 10) }) test_that("can run report with params", { @@ -108,5 +108,5 @@ test_that("can run report with params", { expect_equal(dat$status, "success") expect_null(dat$errors) - expect_worker_task_complete(dat$data$job_id, queue$controller, 10) + expect_worker_task_complete(dat$data$taskId, queue$controller, 10) })