rstudio / pins-r

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

Refactor server discovery #477

Closed hadley closed 3 years ago

hadley commented 3 years ago

Includes several debugging enhancements:

Appears to fix #469, but I'm not sure why.

@juliasilge can you give this one a try please? @andrie this ended up looking rather different to your PR, but it seems to fix the problem, which surprises me.

andrie commented 3 years ago

This works for me, and I also find it hard to reason about what changed 🤷

One observation: we need to strip any trailing / in the server argument. Previously this wasn't necessary, since the httr machinery would remove any trailing /.

hadley commented 3 years ago

Ok, I'll add a couple of explicit tests around handling urls with and without trailing slashes (which seems likely to force me to refactor the code a little) and then merge this PR.

juliasilge commented 3 years ago

This works for me 🙌

Related to @andrie's comments ⬆️ I did have a trailing slash in my URL earlier (my .Renviron had CONNECT_SERVER = https://colorado.rstudio.com/rsc/ previously) and this worked with the old version of pins but was one of the sources of the problems I'd been experiencing.

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.