Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap httr out for httr2 #25

Merged
merged 11 commits into from
Aug 15, 2024
12 changes: 6 additions & 6 deletions DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
Package: proofr
Title: Client for the PROOF API
Version: 0.2.0.95
Version: 0.2.1.91
Authors@R:
person("Scott", "Chamberlain", , "[email protected]", role = c("aut", "cre"),
comment = c(ORCID = "0000-0003-1444-9135"))
Description: Client for the PROOF API.
URL: http://getwilds.org/proofr/ (website)
https://github.com/getwilds/proofr (devel)
URL: http://getwilds.org/proofr/, https://github.com/getwilds/proofr
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
RoxygenNote: 7.3.2
Depends: R (>= 4.1.0)
Imports:
cli,
glue,
httr
httr2
Suggests:
jsonlite,
knitr,
rmarkdown,
testthat (>= 3.0.0),
webmockr,
webmockr (>= 1.0.0),
withr
Config/testthat/edition: 3
VignetteBuilder: knitr
19 changes: 9 additions & 10 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ export(proof_status)
export(proof_timeout)
importFrom(cli,cli_progress_bar)
importFrom(cli,cli_progress_update)
importFrom(httr,DELETE)
importFrom(httr,GET)
importFrom(httr,POST)
importFrom(httr,add_headers)
importFrom(httr,content)
importFrom(httr,http_error)
importFrom(httr,http_status)
importFrom(httr,status_code)
importFrom(httr,stop_for_status)
importFrom(httr,timeout)
importFrom(httr2,req_body_json)
importFrom(httr2,req_error)
importFrom(httr2,req_headers)
importFrom(httr2,req_method)
importFrom(httr2,req_perform)
importFrom(httr2,req_timeout)
importFrom(httr2,request)
importFrom(httr2,resp_body_json)
importFrom(httr2,resp_status)
26 changes: 12 additions & 14 deletions R/auth.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ find_token <- function(token = NULL) {
#' Get header for PROOF API calls
#'
#' @export
#' @param req An `httr2` request. required
#' @param token (character) PROOF API token. optional
#' @return A `request` S3 class with the HTTP header that can be passed
#' to `httr::GET()`, `httr::POST()`, etc.
proof_header <- function(token = NULL) {
add_headers(Authorization = paste0("Bearer ", find_token(token)))
#' @return An `httr2_request` S3 class adding an HTTP header for
#' `Authorization` with the value in `token`
proof_header <- function(req, token = NULL) {
req_headers(req, Authorization = paste0("Bearer ", find_token(token)))
}

#' Authenticate with PROOF API
Expand All @@ -31,16 +32,13 @@ proof_authenticate <- function(username, password) {
assert(username, "character")
assert(password, "character")

response <- POST(make_url("authenticate"),
body = list(
request(make_url("authenticate")) |>
req_body_json(list(
username = username,
password = password
),
encode = "json",
timeout(proofr_env$timeout_sec)
)
stop_for_status(response)
parsed <- content(response, as = "parsed")
token <- parsed$token
token
)) |>
req_timeout(proofr_env$timeout_sec) |>
req_perform() |>
resp_body_json() |>
{\(x) x$token}()
sckott marked this conversation as resolved.
Show resolved Hide resolved
}
14 changes: 7 additions & 7 deletions R/cancel.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
#'
#' If run when there's no Cromwell server running, an HTTP error
proof_cancel <- function(token = NULL) {
response <- DELETE(
make_url("cromwell-server"),
proof_header(token),
timeout(proofr_env$timeout_sec)
)
stop_for_message(response)
content(response, as = "parsed")
request(make_url("cromwell-server")) |>
req_method("DELETE") |>
proof_header(token) |>
req_timeout(proofr_env$timeout_sec) |>
req_error(body = error_body) |>
req_perform() |>
resp_body_json()
}
17 changes: 5 additions & 12 deletions R/http.R
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
stop_for_message <- function(response) {
if (http_error(response)) {
parsed <- tryCatch(content(response), error = function(e) e)
if (inherits(parsed, "error")) stop_for_status(response)
if (!is.list(parsed)) stop_for_status(response)
msg <- glue::glue(
"{http_status(response)$reason}",
" (HTTP {status_code(response)})",
" {parsed$message}"
)
stop(msg, call. = FALSE)
}
error_body <- function(response) {
parsed <- resp_body_json(response)
glue::glue(
"Additional context: {parsed$message}"
)
}
11 changes: 5 additions & 6 deletions R/info.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
#' - `commit_message` (character): the commit message of the API's most recent commit
#' - `tag` (character): tag of most recent commit; if this does not contain a hyphen, it's a release version
proof_info <- function() {
response <- GET(
make_url("info"),
timeout(proofr_env$timeout_sec)
)
stop_for_message(response)
content(response, as = "parsed")
request(make_url("info")) |>
req_timeout(proofr_env$timeout_sec) |>
req_error(body = error_body) |>
req_perform() |>
resp_body_json()
}

6 changes: 3 additions & 3 deletions R/proofr-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"_PACKAGE"

## usethis namespace: start
#' @importFrom httr GET POST DELETE add_headers content
#' stop_for_status timeout http_status status_code
#' http_error
#' @importFrom httr2 request req_perform req_headers
#' req_timeout req_body_json req_method req_error
#' resp_status resp_body_json
## usethis namespace: end
NULL
16 changes: 7 additions & 9 deletions R/start.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,11 @@
#' - `job_id` (character) - the job ID
#' - `info` (character) - message
proof_start <- function(slurm_account = NULL, token = NULL) {
response <- POST(
make_url("cromwell-server"),
proof_header(token),
body = list(slurm_account = slurm_account),
encode = "json",
timeout(proofr_env$timeout_sec)
)
stop_for_message(response)
content(response, as = "parsed")
request(make_url("cromwell-server")) |>
req_body_json(list(slurm_account = slurm_account)) |>
proof_header(token) |>
req_timeout(proofr_env$timeout_sec) |>
req_error(body = error_body) |>
req_perform() |>
resp_body_json()
}
13 changes: 6 additions & 7 deletions R/status.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ proof_status <- function(wait = FALSE, token = NULL) {
}

fetch_status <- function(token = NULL) {
response <- GET(
make_url("cromwell-server"),
proof_header(token),
timeout(proofr_env$timeout_sec)
)
stop_for_message(response)
content(response, as = "parsed")
request(make_url("cromwell-server")) |>
proof_header(token) |>
req_timeout(proofr_env$timeout_sec) |>
req_error(body = error_body) |>
req_perform() |>
resp_body_json()
}

fetch_wait <- function(token) {
Expand Down
4 changes: 2 additions & 2 deletions R/timeout.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
#'
#' @export
#' @param sec (integer/numeric) number of seconds after which
#' requests will timeout. default: 5 sec (5000 ms)
#' requests will timeout. default: 10 sec (10000 ms)
#' @references <https://httr.r-lib.org/reference/timeout.html>
#' @return nothing, side effect of setting the timeout for requests
proof_timeout <- function(sec = 5) {
proof_timeout <- function(sec = 10) {
assert(sec, c("integer", "numeric"))
proofr_env$timeout_sec <- sec
}
2 changes: 1 addition & 1 deletion R/utils.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

make_url <- function(...) {
proof_base <- Sys.getenv("PROOF_API_BASE_URL",
"https://proof-api.fredhutch.org")
"https://proof-api.fredhutch.org")
file.path(proof_base, ...)
}

Expand Down
8 changes: 5 additions & 3 deletions man/proof_header.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions man/proof_timeout.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion man/proofr-package.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions tests/testthat/test-proof_cancel.R
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
test_that("proof_cancel - success", {
stub_registry_clear()
stub_request("delete", make_url("cromwell-server")) %>%
stub_request("delete", make_url("cromwell-server")) |>
to_return(
body = jsonlite::toJSON(response_cancel_success, auto_unbox = TRUE),
status = 200L,
headers = list("Content-type" = "application/json")
)
stub_registry()
# stub_registry()

enable(quiet = TRUE)

Expand All @@ -24,7 +24,7 @@ test_that("proof_cancel - success", {
})

test_that("proof_cancel - not running, can not cancel", {
stub_request("delete", make_url("cromwell-server")) %>%
stub_request("delete", make_url("cromwell-server")) |>
to_return(
body = jsonlite::toJSON(response_cancel_conflict, auto_unbox = TRUE),
status = 409L,
Expand All @@ -34,7 +34,8 @@ test_that("proof_cancel - not running, can not cancel", {
enable(quiet = TRUE)

withr::with_envvar(c("PROOF_TOKEN" = "notarealtoken"), {
expect_error(proof_cancel(), "Job is not running, nothing to delete")
expect_error(proof_cancel(), "HTTP 409 Conflict")
expect_error(proof_cancel(), "Additional context")
})


Expand Down
7 changes: 4 additions & 3 deletions tests/testthat/test-proof_header.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

test_that("proof_header", {
# errors if no env var set and no string supplied
expect_error(proof_header(), "token not found")
expect_error(proof_header(request("")), "token not found")

# returns token if given
expect_match(proof_header("adf")$headers[[1]], "adf")
expect_match(proof_header(request(""), "adf")$headers[[1]], "adf")

# If PROOF_TOKEN env var set, fxn can find it
withr::with_envvar(c("PROOF_TOKEN" = "notarealtoken"), {
expect_match(proof_header()$headers[[1]], "notarealtoken")
expect_match(proof_header(request(""))$headers[[1]],
"notarealtoken")
})
})
5 changes: 5 additions & 0 deletions tests/testthat/test-proof_info.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ test_that("proof_info - success", {

expect_type(info_res, "list")

# also works w/o token as it's not needed for this fxn
info_res_wo_token <- proof_info()

expect_type(info_res_wo_token, "list")


stub_registry_clear()
disable(quiet = TRUE)
Expand Down
7 changes: 4 additions & 3 deletions tests/testthat/test-proof_start.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test_that("proof_start - success", {
stub_request("post", make_url("cromwell-server")) %>%
stub_request("post", make_url("cromwell-server")) |>
to_return(
body = jsonlite::toJSON(response_start_success, auto_unbox = TRUE),
status = 200L,
Expand All @@ -22,7 +22,7 @@ test_that("proof_start - success", {
})

test_that("proof_start - already running", {
stub_request("post", make_url("cromwell-server")) %>%
stub_request("post", make_url("cromwell-server")) |>
to_return(
body = jsonlite::toJSON(response_start_conflict, auto_unbox = TRUE),
status = 409L,
Expand All @@ -32,7 +32,8 @@ test_that("proof_start - already running", {
enable(quiet = TRUE)

withr::with_envvar(c("PROOF_TOKEN" = "notarealtoken"), {
expect_error(proof_start(), "Job is already running")
expect_error(proof_start(), "409")
expect_error(proof_cancel(), "Job is already running")
})


Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-proof_status.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
test_that("proof_status - server IS running", {
stub_request("get", make_url("cromwell-server")) %>%
stub_request("get", make_url("cromwell-server")) |>
to_return(
body = jsonlite::toJSON(
response_status_running,
Expand Down Expand Up @@ -30,7 +30,7 @@ test_that("proof_status - server IS running", {
})

test_that("proof_status - server IS NOT running", {
stub_request("get", make_url("cromwell-server")) %>%
stub_request("get", make_url("cromwell-server")) |>
to_return(
body = jsonlite::toJSON(
response_status_not_running,
Expand Down
Loading