rstudio / pins-r

Pin, discover, and share resources
https://pins.rstudio.com
Other
312 stars 63 forks source link

Fix the URL on servers that have hostname + path #471

Closed andrie closed 3 years ago

andrie commented 3 years ago

This attempts to fix #469, although some more work might be required, since the tests don't all pass locally.

The root cause is that the function board_rsconnect() constructs the server URL from hostname + __api__. However, this ignores the fact that the full domain name can contain hostname + path + __api__.

For example, if the real server is at colorado.rstudio.com/rsc, then the API endpoint is at colorado.rstudio.com/rsc/__api__, but the current code base ignores the /rsc path.

One solution to this is to use rsconnect::serverInfo() to construct the URL, like this:

url <- rsconnect::serverInfo(name = server)$url

instead of the current:

url <- paste0(server, "/__api__/") # remember to delete this line

Here is a reprex:

board <- board_rsconnect(server = "https://colorado.rstudio.com/rsc")
pin_read(board, "andrie/mtcars-test")
juliasilge commented 3 years ago

This looks like it is working for me now! 🤩

juliasilge commented 3 years ago

With this last commit ⬆️ I can now pin_write() as well.

hadley commented 3 years ago

I'll take it from here. Thanks for all your work on this!

hadley commented 3 years ago

Isn't the root cause of the problem that the RSC api is returning the wrong url? i.e. the content url for julia.silge/biv_svm is given as https://colorado.rstudio.com/content/971208fa-72aa-4a1d-ba00-7de2dd9a086a/ but it should actually be https://colorado.rstudio.com/rsc/content/971208fa-72aa-4a1d-ba00-7de2dd9a086a/?

hadley commented 3 years ago

Argh, or did I get confused by the cache

andrie commented 3 years ago

Argh, or did I get confused by the cache

Yes, you have to clear out any cache that may still refer to old URLs. Since I programmatically want to clear the cache regularly, without asking for confirmation, I defined locally:

cache_prune <- function(days = 30, confirm = FALSE) {
  to_delete <- cache_old(days)
  if (length(to_delete) == 0) {
    pins_inform("No stale pins found")
    return(invisible())
  }

  size <- dir_size(to_delete)

  if (!confirm) {
    pins_inform("Delete {length(to_delete)} pin versions, freeing {format(size)} ?")
  }

  if (confirm || utils::menu(c("Yes", "No")) == 1) {
    fs::dir_delete(to_delete)
  }
}
hadley commented 3 years ago

Thanks for working on this! I'm going to go with #477, but this helped me better understand the problem and catalysed me to fix it 😄

github-actions[bot] commented 2 years ago

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.