Skip to content

remove caching at github client layer in favor of httpcli caching

Warren Gifford requested to merge dt/use_etag into main

Created by: davejrt

Removes the use of a cache at the github client layer for https://github.com/sourcegraph/sourcegraph/issues/30656. We take advantage of caching in the httpcli package, specifically here which uses this function from the httpcache package.

GetRepository has been updated to use vcr tests.

Follow up will be to migrate other tests as part of https://github.com/sourcegraph/sourcegraph/issues/33839

Test plan

Tested locally, and new keys can be seen here in the redis cache (keys 7, 19 and 23), with the second test call taking less time due to a cache hit.


127.0.0.1:6379> KEYS *
➜  github git:(dt/use_etag) ✗ go test -v -run TestListMembers 
=== RUN   TestListMembers
=== RUN   TestListMembers/org_members
.....DAVE GET......
DBUG[03-30|09:57:00] TRACE github                             host=api.github.com path=/orgs/sourcegraph-vcr-repos/members code=200 duration=498.438042ms
<<<<<<DAVE SET>>>>>>
.....DAVE GET......
DBUG[03-30|09:57:00] TRACE github                             host=api.github.com path=/orgs/sourcegraph-vcr-repos/members code=200 duration=6.366042ms
=== RUN   TestListMembers/org_admins
.....DAVE GET......
DBUG[03-30|09:57:01] TRACE github                             host=api.github.com path=/orgs/sourcegraph-vcr-repos/members code=200 duration=175.071084ms
<<<<<<DAVE SET>>>>>>
=== RUN   TestListMembers/team_members
.....DAVE GET......
DBUG[03-30|09:57:01] TRACE github                             host=api.github.com path=/orgs/sourcegraph-vcr-repos/teams/private-access/members code=200 duration=222.961959ms
<<<<<<DAVE SET>>>>>>
--- PASS: TestListMembers (0.96s)
    --- PASS: TestListMembers/org_members (0.53s)
    --- PASS: TestListMembers/org_admins (0.19s)
    --- PASS: TestListMembers/team_members (0.24s)
PASS
ok      github.com/sourcegraph/sourcegraph/internal/extsvc/github       1.324s
➜  github git:(dt/use_etag) ✗ docker exec -ti dev-redis-1 redis-cli 
127.0.0.1:6379> KEYS * 
1) "v2:http:https://api.github.com/orgs/sourcegraph-vcr-repos/teams/private-access/members?page=1&per_page=100"
2) "v2:http:https://api.github.com/orgs/sourcegraph-vcr-repos/members?page=1&per_page=100"
3) "v2:http:https://api.github.com/orgs/sourcegraph-vcr-repos/members?page=1&per_page=100&role=admin"
127.0.0.1:6379>

Update to GetRepository to directly use the cache as well with a test below

➜  github git:(dt/use_etag) ✗ docker exec -ti dev-redis-1 redis-cli 
127.0.0.1:6379> KEYS * 
(empty list or set)
127.0.0.1:6379> exit
➜  github git:(dt/use_etag) ✗ go test -run TestClient_GetRepository
DBUG[04-12|17:32:44] TRACE github                             host=api.github.com path=/repos/sourcegraph-vcr-repos/private-org-repo-1 code=200 duration=524.473958ms
DBUG[04-12|17:32:44] TRACE github                             host=example.com path=/repos/owner/repo code=404 duration=1.75µs
PASS
ok      github.com/sourcegraph/sourcegraph/internal/extsvc/github       0.913s
➜  github git:(dt/use_etag) ✗ docker exec -ti dev-redis-1 redis-cli 
127.0.0.1:6379> KEYS * 
1) "v2:http:https://api.github.com/repos/sourcegraph-vcr-repos/private-org-repo-1"
127.0.0.1:6379>

Merge request reports