frontend: use #40827 to provide the src-cli version and download endpoints#40828
Conversation
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff f38ebd5...60ebdac.
|
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode < 200 || resp.StatusCode > 299 { |
There was a problem hiding this comment.
Same goes for status. The standard library has constants (http.StatusOk). But in this case, there is no 299 code. So I feel this is fine.
But could always do,
// http.StatusMultipleChoices = 300
if resp.StatusCode < http.StatusOk || resp.StatusCode >= http.StatusMultipleChoices {
}There was a problem hiding this comment.
This is a case where I actually prefer having the integers rather than using the constants, since the ordering of the constants is less immediately apparent.
| } | ||
|
|
||
| func (h *srcCliVersionHandler) handleVersion(w http.ResponseWriter) { | ||
| writeJSON(w, h.Version()) |
There was a problem hiding this comment.
Did you intentionally switch from
func srcCliVersionServe(w http.ResponseWriter, r *http.Request) error {
return writeJSON(w, &struct {
Version string `json:"version"`
}{
Version: srcCliVersion(),
})to
writeJSON(w, h.Version())here?
https://sourcegraph.slack.com/archives/C03DWK5QJUS/p1661505252886269
There was a problem hiding this comment.
|
Side question not directly related to this PR, but: is our REST API documented anywhere in particular? |
|
it's not! |
This was a _very_ late change in developing #40828, and was totally my fault that it slipped through. Basically, no request to the `src-cli` version cache could succeed, because it would already have been handled (and 404ed) by the normal `src-cli` version endpoint.
This was a _very_ late change in developing #40828, and was totally my fault that it slipped through. Basically, no request to the `src-cli` version cache could succeed, because it would already have been handled (and 404ed) by the normal `src-cli` version endpoint.
This PR uses the sourcegraph.com API added in #40827 to implement the
/.api/src-cli/versionand/.api/src-cli/{download}endpoints, rather than calling out to GitHub directly.There's also some new caching added that should mean that Sourcegraph instances don't hammer sourcegraph.com with these requests either.
If the API is unavailable or unreachable, the previous behaviour of falling back to the
MinimumVersionconstant is preserved. This is mostly going to come into play with any early rollouts of Sourcegraph 4.0 that occur before sourcegraph.com is upgraded to 4.0, but I don't think the behavioural change is significant enough to be a problem.Fixes #39468.
Test plan
There's good test coverage, and I've done a bunch of testing locally of the various scenarios, monitoring the requests with ngrok to ensure that the cache behaves as expected and the sourcegraph.com API isn't hit more often than expected.