From 4baa1974a9a130fb8a867021feed3a5084fdce15 Mon Sep 17 00:00:00 2001 From: Austin Dickey Date: Fri, 20 Oct 2023 14:03:06 -0500 Subject: [PATCH 1/3] Add pagination to runs() --- DESCRIPTION | 2 +- NEWS.md | 6 ++++ R/benchmark-results.R | 2 +- R/hardware.R | 2 +- R/history.R | 2 +- R/info.R | 4 +-- R/runs.R | 35 +++++++++++++------ man/conbenchcoms-package.Rd | 1 + man/runs.Rd | 16 ++++----- .../resp-class/localhost/api/runs-d269db.json | 8 ----- .../resp-class/localhost/api/runs-dce918.json | 16 --------- .../resp-class/localhost/api/runs-ebe91f.json | 13 +++++++ .../resp-class/localhost/api/runs-f23dbb.json | 14 ++++++++ .../resp-class/localhost/api/runs-f323bc.json | 14 ++++++++ tests/testthat/test-runs.R | 27 ++++---------- 15 files changed, 92 insertions(+), 70 deletions(-) delete mode 100644 tests/testthat/resp-class/localhost/api/runs-d269db.json delete mode 100644 tests/testthat/resp-class/localhost/api/runs-dce918.json create mode 100644 tests/testthat/resp-class/localhost/api/runs-ebe91f.json create mode 100644 tests/testthat/resp-class/localhost/api/runs-f23dbb.json create mode 100644 tests/testthat/resp-class/localhost/api/runs-f323bc.json diff --git a/DESCRIPTION b/DESCRIPTION index 0ae78af..7f2753d 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: conbenchcoms Title: An API wrapper for conbench communications -Version: 0.0.6 +Version: 0.0.7 Authors@R: c( person("Jonathan", "Keane", , "jon@voltrondata.com", role = c("aut", "ctb"), comment = c(ORCID = "0000-0001-7087-9776")), diff --git a/NEWS.md b/NEWS.md index 2b4c974..ec71695 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,9 @@ +# conbenchcoms 0.0.7 +* Remove `sha`, `simplifyVector`, and `flatten` arguments from `runs` +* Add `commit_hashes` argument to `runs` +* `runs` now always returns a `tibble` +* `runs` now paginates until all matching data is returned, which works only with Conbench servers of at least version [7e4d9d0](https://github.com/conbench/conbench/commit/7e4d9d0) + # conbenchcoms 0.0.6 * Remove `batch_id`, `limit`, `days`, `simplifyVector`, and `flatten` arguments from `benchmark_results` * Add `earliest_timestamp` and `latest_timestamp` arguments to `benchmark_results` diff --git a/R/benchmark-results.R b/R/benchmark-results.R index a71da83..14ec760 100644 --- a/R/benchmark-results.R +++ b/R/benchmark-results.R @@ -33,7 +33,6 @@ #' @param run_reason a string to specify the run reason (default: `NULL`, list all) #' @param earliest_timestamp the earliest benchmark result timestamp (default: `NULL`, go back as far as possible) #' @param latest_timestamp the latest benchmark result timestamp (default: `NULL`, go up to the current time) -#' @inheritParams runs #' #' @return a tibble of benchmark results #' @export @@ -79,6 +78,7 @@ benchmark_results <- function( #' retired in a future release. Please use `benchmark_results` instead. #' @rdname benchmark_results #' @param batch_id deprecated +#' @inheritParams jsonlite::fromJSON #' @export benchmarks <- function(run_id = NULL, batch_id = NULL, run_reason = NULL, simplifyVector = TRUE, flatten = TRUE, ...) { .Deprecated("benchmark_results") diff --git a/R/hardware.R b/R/hardware.R index c4060bc..9293026 100644 --- a/R/hardware.R +++ b/R/hardware.R @@ -1,7 +1,7 @@ #' Get hardware #' #' @param id the id of one specific hardware to get data for. -#' @inheritParams runs +#' @inheritParams jsonlite::fromJSON #' #' @return the response #' @export diff --git a/R/history.R b/R/history.R index d321a60..26c3215 100644 --- a/R/history.R +++ b/R/history.R @@ -1,7 +1,7 @@ #' Get history for a benchmark #' #' @param benchmark_id the hash of a benchmark to get the history for -#' @inheritParams runs +#' @inheritParams jsonlite::fromJSON #' #' @return the response #' @export diff --git a/R/info.R b/R/info.R index f9beb0b..922ba76 100644 --- a/R/info.R +++ b/R/info.R @@ -1,7 +1,7 @@ #' Get a list of benchmark info #' #' @param id the info id for benchmark info -#' @inheritParams runs +#' @inheritParams jsonlite::fromJSON #' #' @return the response #' @export @@ -17,5 +17,3 @@ info <- function(id = NULL, simplifyVector = TRUE, flatten = TRUE, ...) { resp_body_json(resp, simplifyVector = simplifyVector, flatten = flatten, ...) } - - diff --git a/R/runs.R b/R/runs.R index ebe0fb5..5a1b7c6 100644 --- a/R/runs.R +++ b/R/runs.R @@ -1,18 +1,33 @@ -#' Get a list of runs +#' Get runs #' -#' @param sha the hash of a commit to search for (default: `NULL`, list all) -#' @inheritParams jsonlite::fromJSON +#' @param commit_hashes the commit hashes to search for (required) #' @inheritDotParams httr2::resp_body_json #' -#' @return the response +#' @return a tibble of runs #' @export -runs <- function(sha = NULL, simplifyVector = TRUE, flatten = TRUE, ...) { +runs <- function(commit_hashes, ...) { req <- req_url_path_append(conbench_request(), "runs") + req <- req_url_query( + req, + page_size = 1000, + commit_hash = paste0(commit_hashes, collapse = ",") + ) + resp <- conbench_perform(req) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE, ...) + data <- dplyr::as_tibble(json[["data"]]) - if (!is.null(sha)) { - req <- req_url_query(req, sha = paste0(sha, collapse = ",")) + while (!is.null(json[["metadata"]][["next_page_cursor"]])) { + req <- req_url_path_append(conbench_request(), "runs") + req <- req_url_query( + req, + page_size = 1000, + commit_hash = paste0(commit_hashes, collapse = ","), + cursor = json[["metadata"]][["next_page_cursor"]] + ) + resp <- conbench_perform(req) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE, ...) + data <- dplyr::bind_rows(data, dplyr::as_tibble(json[["data"]])) } - resp <- conbench_perform(req) - resp_body_json(resp, simplifyVector = simplifyVector, flatten = flatten, ...) -} \ No newline at end of file + data +} diff --git a/man/conbenchcoms-package.Rd b/man/conbenchcoms-package.Rd index d98bfd0..91dca59 100644 --- a/man/conbenchcoms-package.Rd +++ b/man/conbenchcoms-package.Rd @@ -15,6 +15,7 @@ Authors: \itemize{ \item Jonathan Keane \email{jon@voltrondata.com} (\href{https://orcid.org/0000-0001-7087-9776}{ORCID}) [contributor] \item Edward Visel \email{edward@voltrondata.com} (\href{https://orcid.org/0000-0002-2811-6254}{ORCID}) [contributor] + \item Austin Dickey \email{austin@voltrondata.com} [contributor] } Other contributors: diff --git a/man/runs.Rd b/man/runs.Rd index 31da698..778f33d 100644 --- a/man/runs.Rd +++ b/man/runs.Rd @@ -2,16 +2,12 @@ % Please edit documentation in R/runs.R \name{runs} \alias{runs} -\title{Get a list of runs} +\title{Get runs} \usage{ -runs(sha = NULL, simplifyVector = TRUE, flatten = TRUE, ...) +runs(commit_hashes, ...) } \arguments{ -\item{sha}{the hash of a commit to search for (default: \code{NULL}, list all)} - -\item{simplifyVector}{coerce JSON arrays containing only primitives into an atomic vector} - -\item{flatten}{automatically \code{\link[jsonlite:flatten]{flatten()}} nested data frames into a single non-nested data frame} +\item{commit_hashes}{the commit hashes to search for (required)} \item{...}{ Arguments passed on to \code{\link[httr2:resp_body_raw]{httr2::resp_body_json}} @@ -19,11 +15,13 @@ runs(sha = NULL, simplifyVector = TRUE, flatten = TRUE, ...) \item{\code{resp}}{A response object.} \item{\code{check_type}}{Check that response has expected content type? Set to \code{FALSE} to suppress the automated check} + \item{\code{simplifyVector}}{Should JSON arrays containing only primitives (i.e. +booleans, numbers, and strings) be caused to atomic vectors?} }} } \value{ -the response +a tibble of runs } \description{ -Get a list of runs +Get runs } diff --git a/tests/testthat/resp-class/localhost/api/runs-d269db.json b/tests/testthat/resp-class/localhost/api/runs-d269db.json deleted file mode 100644 index b60d371..0000000 --- a/tests/testthat/resp-class/localhost/api/runs-d269db.json +++ /dev/null @@ -1,8 +0,0 @@ -[ - { - "commit": { - "sha": "babb1ed", - "timestamp": "2022-03-20T21:05:12" - } - } -] diff --git a/tests/testthat/resp-class/localhost/api/runs-dce918.json b/tests/testthat/resp-class/localhost/api/runs-dce918.json deleted file mode 100644 index 70bc88b..0000000 --- a/tests/testthat/resp-class/localhost/api/runs-dce918.json +++ /dev/null @@ -1,16 +0,0 @@ -[ - { - "commit": { - "id": "id1", - "sha": "babb1ed", - "timestamp": "2022-03-20T21:05:12" - } - }, - { - "commit": { - "id": "id2", - "sha": "5eaf00d", - "timestamp": "2022-05-20T21:05:12" - } - } -] diff --git a/tests/testthat/resp-class/localhost/api/runs-ebe91f.json b/tests/testthat/resp-class/localhost/api/runs-ebe91f.json new file mode 100644 index 0000000..fdb7241 --- /dev/null +++ b/tests/testthat/resp-class/localhost/api/runs-ebe91f.json @@ -0,0 +1,13 @@ +{ + "data": [ + { + "commit": { + "sha": "babb1ed", + "timestamp": "2022-03-20T21:05:12" + } + } + ], + "metadata": { + "next_page_cursor": null + } +} diff --git a/tests/testthat/resp-class/localhost/api/runs-f23dbb.json b/tests/testthat/resp-class/localhost/api/runs-f23dbb.json new file mode 100644 index 0000000..99f6313 --- /dev/null +++ b/tests/testthat/resp-class/localhost/api/runs-f23dbb.json @@ -0,0 +1,14 @@ +{ + "data": [ + { + "commit": { + "id": "id1", + "sha": "babb1ed", + "timestamp": "2022-03-20T21:05:12" + } + } + ], + "metadata": { + "next_page_cursor": "curse" + } +} diff --git a/tests/testthat/resp-class/localhost/api/runs-f323bc.json b/tests/testthat/resp-class/localhost/api/runs-f323bc.json new file mode 100644 index 0000000..a384ed7 --- /dev/null +++ b/tests/testthat/resp-class/localhost/api/runs-f323bc.json @@ -0,0 +1,14 @@ +{ + "data": [ + { + "commit": { + "id": "id2", + "sha": "5eaf00d", + "timestamp": "2022-05-20T21:05:12" + } + } + ], + "metadata": { + "next_page_cursor": null + } +} diff --git a/tests/testthat/test-runs.R b/tests/testthat/test-runs.R index 80e5386..cb99d51 100644 --- a/tests/testthat/test-runs.R +++ b/tests/testthat/test-runs.R @@ -1,13 +1,8 @@ with_mock_dir(test_path("logged-in"), { test_that("runs()", { expect_GET( - runs(), - "http://localhost/api/runs" - ) - - expect_GET( - runs(sha = "C0C0A"), - "http://localhost/api/runs?sha=C0C0A" + runs(commit_hashes = c("C0C0A", "BEEF")), + "http://localhost/api/runs?page_size=1000&commit_hash=C0C0A%2CBEEF" ) }) }) @@ -15,31 +10,23 @@ with_mock_dir(test_path("logged-in"), { with_mock_dir(test_path("resp-class"), { sha_1 <- "babb1ed" sha_2 <- "5eaf00d" - test_that("runs() returns a data.frame", { - the_run <- runs(sha = sha_1) + test_that("runs() returns a tibble", { + the_run <- runs(commit_hashes = sha_1) expect_s3_class( the_run, - "data.frame" + "tbl_df" ) expect_identical(sha_1, the_run$commit.sha) }) test_that("runs() accepts and returns multiple values", { shas <- c(sha_1, sha_2) - runs <- runs(sha = shas) + runs <- runs(commit_hashes = shas) expect_s3_class( runs, - "data.frame" + "tbl_df" ) expect_identical(shas, unique(runs$commit.sha)) }) - test_that("runs() returns a list", { - the_list_run <- runs(sha = sha_1, simplifyVector = FALSE, flatten = FALSE) - expect_type( - the_list_run, - "list" - ) - expect_identical(sha_1, the_list_run[[1]]$commit$sha) - }) }) From 6502beae67434ae30be25e4e5034c8356837b4ad Mon Sep 17 00:00:00 2001 From: Sam Albers Date: Fri, 20 Oct 2023 13:24:17 -0700 Subject: [PATCH 2/3] drop json flattening arguments --- R/benchmark-results.R | 8 +++----- R/runs.R | 7 +++---- man/benchmark_results.Rd | 9 +-------- man/runs.Rd | 12 +----------- 4 files changed, 8 insertions(+), 28 deletions(-) diff --git a/R/benchmark-results.R b/R/benchmark-results.R index 14ec760..008538c 100644 --- a/R/benchmark-results.R +++ b/R/benchmark-results.R @@ -40,8 +40,7 @@ benchmark_results <- function( run_id = NULL, run_reason = NULL, earliest_timestamp = NULL, - latest_timestamp = NULL, - ...) { + latest_timestamp = NULL) { ## Assert that run_id is a string if (length(run_id) > 1) { stop("Too many run_ids, please limit to 1.", call. = FALSE) @@ -55,7 +54,7 @@ benchmark_results <- function( cursor = NULL ) resp <- conbench_perform(req) - json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE, ...) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE) data <- dplyr::as_tibble(json[["data"]]) while (!is.null(json[["metadata"]][["next_page_cursor"]])) { @@ -67,7 +66,7 @@ benchmark_results <- function( cursor = json[["metadata"]][["next_page_cursor"]] ) resp <- conbench_perform(req) - json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE, ...) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE) data <- dplyr::bind_rows(data, dplyr::as_tibble(json[["data"]])) } @@ -78,7 +77,6 @@ benchmark_results <- function( #' retired in a future release. Please use `benchmark_results` instead. #' @rdname benchmark_results #' @param batch_id deprecated -#' @inheritParams jsonlite::fromJSON #' @export benchmarks <- function(run_id = NULL, batch_id = NULL, run_reason = NULL, simplifyVector = TRUE, flatten = TRUE, ...) { .Deprecated("benchmark_results") diff --git a/R/runs.R b/R/runs.R index 5a1b7c6..7aa6540 100644 --- a/R/runs.R +++ b/R/runs.R @@ -1,11 +1,10 @@ #' Get runs #' #' @param commit_hashes the commit hashes to search for (required) -#' @inheritDotParams httr2::resp_body_json #' #' @return a tibble of runs #' @export -runs <- function(commit_hashes, ...) { +runs <- function(commit_hashes) { req <- req_url_path_append(conbench_request(), "runs") req <- req_url_query( req, @@ -13,7 +12,7 @@ runs <- function(commit_hashes, ...) { commit_hash = paste0(commit_hashes, collapse = ",") ) resp <- conbench_perform(req) - json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE, ...) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE) data <- dplyr::as_tibble(json[["data"]]) while (!is.null(json[["metadata"]][["next_page_cursor"]])) { @@ -25,7 +24,7 @@ runs <- function(commit_hashes, ...) { cursor = json[["metadata"]][["next_page_cursor"]] ) resp <- conbench_perform(req) - json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE, ...) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE) data <- dplyr::bind_rows(data, dplyr::as_tibble(json[["data"]])) } diff --git a/man/benchmark_results.Rd b/man/benchmark_results.Rd index 9024a38..5400099 100644 --- a/man/benchmark_results.Rd +++ b/man/benchmark_results.Rd @@ -9,8 +9,7 @@ benchmark_results( run_id = NULL, run_reason = NULL, earliest_timestamp = NULL, - latest_timestamp = NULL, - ... + latest_timestamp = NULL ) benchmarks( @@ -31,13 +30,7 @@ benchmarks( \item{latest_timestamp}{the latest benchmark result timestamp (default: \code{NULL}, go up to the current time)} -\item{...}{arguments passed on to class specific \code{print} methods} - \item{batch_id}{deprecated} - -\item{simplifyVector}{coerce JSON arrays containing only primitives into an atomic vector} - -\item{flatten}{automatically \code{\link[jsonlite:flatten]{flatten()}} nested data frames into a single non-nested data frame} } \value{ a tibble of benchmark results diff --git a/man/runs.Rd b/man/runs.Rd index 778f33d..a4306f2 100644 --- a/man/runs.Rd +++ b/man/runs.Rd @@ -4,20 +4,10 @@ \alias{runs} \title{Get runs} \usage{ -runs(commit_hashes, ...) +runs(commit_hashes) } \arguments{ \item{commit_hashes}{the commit hashes to search for (required)} - -\item{...}{ - Arguments passed on to \code{\link[httr2:resp_body_raw]{httr2::resp_body_json}} - \describe{ - \item{\code{resp}}{A response object.} - \item{\code{check_type}}{Check that response has expected content type? Set to -\code{FALSE} to suppress the automated check} - \item{\code{simplifyVector}}{Should JSON arrays containing only primitives (i.e. -booleans, numbers, and strings) be caused to atomic vectors?} - }} } \value{ a tibble of runs From b844a8303bb2e8698ec0537931d2f82cf8ef3ac3 Mon Sep 17 00:00:00 2001 From: Sam Albers Date: Fri, 20 Oct 2023 13:59:12 -0700 Subject: [PATCH 3/3] make benchmarks function fully defunct --- NEWS.md | 1 + R/benchmark-results.R | 21 ++------------------- man/benchmark_results.Rd | 9 +-------- 3 files changed, 4 insertions(+), 27 deletions(-) diff --git a/NEWS.md b/NEWS.md index ec71695..c028c92 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,7 @@ * Add `commit_hashes` argument to `runs` * `runs` now always returns a `tibble` * `runs` now paginates until all matching data is returned, which works only with Conbench servers of at least version [7e4d9d0](https://github.com/conbench/conbench/commit/7e4d9d0) +* Make `benchmarks` defunct # conbenchcoms 0.0.6 * Remove `batch_id`, `limit`, `days`, `simplifyVector`, and `flatten` arguments from `benchmark_results` diff --git a/R/benchmark-results.R b/R/benchmark-results.R index 008538c..af5d693 100644 --- a/R/benchmark-results.R +++ b/R/benchmark-results.R @@ -78,23 +78,6 @@ benchmark_results <- function( #' @rdname benchmark_results #' @param batch_id deprecated #' @export -benchmarks <- function(run_id = NULL, batch_id = NULL, run_reason = NULL, simplifyVector = TRUE, flatten = TRUE, ...) { - .Deprecated("benchmark_results") - - req <- req_url_path_append(conbench_request(), "benchmarks") - - if (!is.null(run_reason)) { - req <- req_url_query(req, run_reason = run_reason) - } - - if (!is.null(batch_id)) { - req <- req_url_query(req, batch_id = paste0(batch_id, collapse = ",")) - } - - if (!is.null(run_id)) { - req <- req_url_query(req, run_id = paste0(run_id, collapse = ",")) - } - resp <- conbench_perform(req) - - resp_body_json(resp, simplifyVector = simplifyVector, flatten = flatten, ...) +benchmarks <- function(run_id = NULL, batch_id = NULL, run_reason = NULL) { + .Defunct("benchmark_results") } diff --git a/man/benchmark_results.Rd b/man/benchmark_results.Rd index 5400099..9cd1ccc 100644 --- a/man/benchmark_results.Rd +++ b/man/benchmark_results.Rd @@ -12,14 +12,7 @@ benchmark_results( latest_timestamp = NULL ) -benchmarks( - run_id = NULL, - batch_id = NULL, - run_reason = NULL, - simplifyVector = TRUE, - flatten = TRUE, - ... -) +benchmarks(run_id = NULL, batch_id = NULL, run_reason = NULL) } \arguments{ \item{run_id}{the run_id to get benchmarks for (default: \code{NULL}, list all)}