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

New headers argument to board_url() #732

Merged
merged 15 commits into from
Apr 14, 2023
Merged

Conversation

juliasilge
Copy link
Member

This PR adds a new add_headers() argument to board_url(), mostly for authentication.

Addresses #600 for private GH repos:

library(pins)
folder_on_gh <- "https://raw.githubusercontent.com/juliasilge/pkgpins/main/pkgdown/assets/pins-board/_pins.yaml"
my_headers <- httr::add_headers(Authorization = paste("token", "github_pat_XXXX"))
b <- board_url(folder_on_gh, add_headers = my_headers)
b %>% pin_list()
#> [1] "mtcars_metric"

Created on 2023-04-10 with reprex v2.0.2

Addresses #616 for Posit Connect vanity URLs that are not set to "Anyone - no login required":

library(pins)
board <- board_url(
  c(my_vanity_url_pin = "https://colorado.posit.co/rsc/some-nice-numbers/"),
  add_headers = httr::add_headers(Authorization = paste("Key", Sys.getenv("CONNECT_API_KEY")))
)
board %>% pin_read("my_vanity_url_pin")
#>  [1]  1  2  3  4  5  6  7  8  9 10

Created on 2023-04-10 with reprex v2.0.2

@juliasilge
Copy link
Member Author

juliasilge commented Apr 10, 2023

Upon further consideration, the solution in #706 doesn't really get us enough of what folks are wanting, and is quite a big change. This more simple change results in more consistent versioning options (does require a manifest for reading different versions; otherwise reads latest) and supports more use cases.

Ideally in the long term, we will have better options on Connect like service accounts that help with some of the same user needs. I think this PR is a good option to implement now, because it fits in with several use cases and is not a burdensome change.

@juliasilge juliasilge requested a review from hadley April 10, 2023 20:43
@juliasilge
Copy link
Member Author

Partly to get some more thorough testing of whether this all works (but also because folks continue to ask), I ended up adding back in a board_connect_url() function which is a very thin wrapper around board_url() with the new headers arg. It only authenticates to Connect via environment variable, which SE folks say is good because it is the only method that will work went content gets deployed to Connect.

We talked about whether it is possible to catch these kind of errors and provide a more helpful error message:

library(pins)

b2 <- board_url(
  c(non_public_numbers = "https://colorado.posit.co/rsc/some-nice-numbers/"),
)
b2 %>% pin_read("non_public_numbers")
#> Error in http_download(url = paste0(url, "data.txt"), path_dir = cache_dir, : Not Found (HTTP 404).

Created on 2023-04-12 with reprex v2.0.2

I don't know that we can for now, because Connect is returning a 404 where the real problem is lack of authentication (403).

@@ -0,0 +1,42 @@
skip_if_not_installed("rsconnect")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed because the test boards use connect_has_colorado(), not because board_connect_url() uses the rsconnect package (it does not).

NEWS.md Outdated Show resolved Hide resolved
R/board_url.R Outdated
@@ -14,6 +14,8 @@
#' use the last cached version? Defaults to `is_interactive()` so you'll
#' be robust to poor internet connectivity when exploring interactively,
#' but you'll get clear errors when the code is deployed.
#' @param add_headers Additional headers (such as for authentication) created
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just call it headers? It bit feels weird to make it a verb.

It think overall this would be a bit simpler if you just made it a named character vector.

R/board_url.R Outdated
@@ -304,11 +329,12 @@ http_download <- function(url, path_dir, path_file, ...,
}

headers <- httr::add_headers(
add_headers$headers,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
add_headers$headers,
add_headers,

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because as it is here, the add_headers object was created with httr::add_headers() and you can't put those together:

h <- httr::add_headers(a = 1, b = 2)
h
#> <request>
#> Headers:
#> * a: 1
#> * b: 2

httr::add_headers(h, c = 3)
#> Error in f(init, x[[i]]): is.request(y) is not TRUE
httr::add_headers(h$headers, c = 3)
#> <request>
#> Headers:
#> * a: 1
#> * b: 2
#> * c: 3

Created on 2023-04-14 with reprex v2.0.2

I see what you're saying about having the argument use a named character instead and I'll check that out.

R/board_url.R Outdated Show resolved Hide resolved
@juliasilge juliasilge merged commit 72621cd into main Apr 14, 2023
13 checks passed
@juliasilge juliasilge deleted the add-headers-to-board-url branch April 14, 2023 20:43
@juliasilge juliasilge changed the title New add_headers argument to board_url() New headers argument to board_url() Apr 14, 2023
@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants