Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

frontend: add a dot-com API endpoint caching src versions by branch#40827

Merged
LawnGnome merged 7 commits into
mainfrom
aharvey/version-cache-backend
Aug 25, 2022
Merged

frontend: add a dot-com API endpoint caching src versions by branch#40827
LawnGnome merged 7 commits into
mainfrom
aharvey/version-cache-backend

Conversation

@LawnGnome

@LawnGnome LawnGnome commented Aug 25, 2022

Copy link
Copy Markdown
Contributor

This PR adds a simple version cache to the unauthenticated external REST API that reports the most recent version of src-cli for the given branch. This is the backend that will address #39468, but #40828 also has to be merged for this to be fully fixed.

The intention is that the sourcegraph.com deployment will enable the cache, and no other deployments will ever configure it.

The minimal configuration that will be required on sourcegraph.com will be:

{
  "dotcom": {
    "srcCliVersionCache": {
      "enabled": true,
      "github": {
        "token": "TOKEN",
        "webhookSecret": "WEBHOOK_SECRET"
      }
    }
  }
}

This will need to be combined with a GitHub webhook on https://github.com/sourcegraph/src-cli that points to https://sourcegraph.com/.api/src-cli/versions/webhook, configured with the same WEBHOOK_SECRET and with the release event type enabled.

Probable questions

Why not use the SOURCEGRAPHDOTCOM_MODE environment variable?

This PR adds a small configuration section to the dotcom site configuration object that enables and configures the cache, but does not use the SOURCEGRAPHDOTCOM_MODE environment variable.

This is both to simplify testing and reduce the matrix of possible configuration options. With this implementation, there are only two possible states: configuration present and enabled, in which case this API will respond, and literally anything else, in which case this API will 404 like any other unknown endpoint.

Why is the handler always installed?

Originally, I had the handler only conditionally hooked up if the configuration was enabled. However, it's considerably simpler if it's always present, since it means we can handle all the enable/disable behaviour in a single configuration watcher, and from a user perspective the disabled handler is indistinguishable from the route not existing in the router.

Test plan

This has (IMHO) good test coverage, and I've given it a pretty good hammering in the last day or so locally as part of developing this and #40828.

@cla-bot cla-bot Bot added the cla-signed label Aug 25, 2022
@sourcegraph-bot

sourcegraph-bot commented Aug 25, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff bb543fa...2c274de.

Notify File(s)
@eseliger internal/extsvc/github/testdata/golden/Releases
internal/extsvc/github/testdata/vcr/Releases.yaml
internal/extsvc/github/v4.go
internal/extsvc/github/v4_test.go

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wow, so much stuff you implemented here, even webhooks to make it close to real time! Great work, also on all the testing!

Comment thread internal/extsvc/github/v4.go
Comment thread cmd/frontend/internal/httpapi/releasecache/testdata/golden/TestReleaseCache_Fetch Outdated
Comment thread cmd/frontend/internal/httpapi/releasecache/http.go
Comment thread cmd/frontend/internal/httpapi/releasecache/http.go
Comment thread schema/site.schema.json Outdated
Comment thread cmd/frontend/internal/httpapi/releasecache/cache.go
Comment thread cmd/frontend/internal/httpapi/releasecache/cache.go
@LawnGnome LawnGnome merged commit f38ebd5 into main Aug 25, 2022
@LawnGnome LawnGnome deleted the aharvey/version-cache-backend branch August 25, 2022 21:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants