chore: Use v1 APIs for managing content git repo settings #459
Conversation
|
CI is no longer failing but that does not mean there is enough test coverage for this yet. In poking at this some more, there are some semantic differences in how the v1 and v0 APIs work. Importantly, "this content is not git backed" is indicated if Since the v1 APIs have been around since October 2022, we might just want to make a clean break here and swap over to the v1 APIs with no fallback. We could try to be more clever and detect server version, though that's known to be imperfect, and maybe it's not worth it? 2022 is well outside the support window. So I'm leaning towards just dropping the v0 entirely. Thoughts @toph-allen @karawoo @jonkeane ? |
|
I'm a 👍 on dropping the V0 |
| "variants" | ||
| ) | ||
| guid <- self$get_content()$guid | ||
| url <- unversioned_url("applications", guid, "variants") |
There was a problem hiding this comment.
This is just reformatting so that I can see the whole URL on one line--helps to grep the repo to find the remaining v0 endpoints in use.
toph-allen
left a comment
toph-allen
left a comment
There was a problem hiding this comment.
A few questions or requests about patterns, and one about version checks. Other than that, it looks good, and thanks for moving this forward.
R/content.R
Outdated
| polling = FALSE | ||
| ) { | ||
| guid <- self$get_content()$guid | ||
| self$get_connect()$PUT( |
There was a problem hiding this comment.
| self$get_connect()$PUT( | |
| self$connect$PUT( |
R/content.R
Outdated
| #' - last_error | ||
| #' - last_known_commit | ||
| repository = function() { | ||
| con <- self$get_connect() |
There was a problem hiding this comment.
At a cursory glance, it looks like the v1 repo endpoints were added in 2024.05.0. Should we add a version check or do something else for back compat here? (ref)
[edit] I could be wrong about that date! The integration tests seem to suggest stuff's fine.
There was a problem hiding this comment.
Judging from https://github.com/posit-dev/connect/issues/22252, they were actually added in October 2022. We just didn't document them until 2024.05 (https://github.com/posit-dev/connect/issues/26982) 🤦
There was a problem hiding this comment.
Aaaah that explains my confusion. Thanks!
Still — perhaps good to error_if_less_than() here?
toph-allen
left a comment
toph-allen
left a comment
There was a problem hiding this comment.
Approving contingent on the $get_content() -> $content changes — no need to re-request review after you've made those! :)
nealrichardson
force-pushed
the
rm-v0-content-repo
branch
from
0a25846 to
657c49f
Compare
nealrichardson
force-pushed
the
rm-v0-content-repo
branch
from
b7d8081 to
5df0b30
Compare
Intent
Remove calls to unversioned APIs in service of managing the git-backedness of content.
Fixes #416
Fixes #278
Approach
Initially added tryCatches and fallbacks to the v0 APIs, but that didn't work out across the board because in the v1 APIs, 404 on GET means that the content is not git backed, not that the v1 API is unavailable. Since the v1s have been around for 3 years, we've decided to just drop the v0 fallbacks.
I also added some mock tests for the main code paths. There is some amount of coverage coming from the integration tests, which I did not substantively alter.
Checklist
NEWS.md(referencing the connected issue if necessary)?devtools::document()?