artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 17:11:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

[update] Remove sleep in goroutine for webhook handlers (#63940)

Browse Source 
Since permission sync jobs are database backed, we no longer need to
sleep in the webhook handler goroutines, since the permission sync jobs
have a sleep time themselves.

## Test plan

Tests still pass, no real functional changes.

<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
This commit is contained in:
Petri-Johan Last 2024-07-19 18:45:16 +02:00 committed by GitHub
parent 0212a47690
commit 2994636bbc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 18 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
cmd/frontend/internal/authz/webhooks/github.go
View File

@ -53,31 +53,21 @@ func TestSetGitHubHandlerSleepTime(t *testing.T, val time.Duration) {
sleepTime = val
}
func (h *GitHubWebhook) handleGitHubWebhook(_ context.Context, db database.DB, codeHostURN extsvc.CodeHostBaseURL, payload any) error {
// TODO: This MUST be removed once permissions syncing jobs are database backed!
// If we react too quickly to a webhook, the changes may not yet have properly
// propagated on GitHub's system, and we'll get old results, making the
// webhook useless.
// We have to wait some amount of time to process the webhook to ensure
// that we are getting fresh results.
go func() {
time.Sleep(sleepTime)
eventContext, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
defer cancel()
switch e := payload.(type) {
case *gh.RepositoryEvent:
_ = h.handleRepositoryEvent(eventContext, db, e)
case *gh.MemberEvent:
_ = h.handleMemberEvent(eventContext, db, e, codeHostURN)
case *gh.OrganizationEvent:
_ = h.handleOrganizationEvent(eventContext, db, e, codeHostURN)
case *gh.MembershipEvent:
_ = h.handleMembershipEvent(eventContext, db, e, codeHostURN)
case *gh.TeamEvent:
_ = h.handleTeamEvent(eventContext, e, db)
}
}()
func (h *GitHubWebhook) handleGitHubWebhook(ctx context.Context, db database.DB, codeHostURN extsvc.CodeHostBaseURL, payload any) error {
// TODO: Should we return errors here? We could receive a webhook for a repo
// that's not on Sourcegraph, so probably not?
switch e := payload.(type) {
case *gh.RepositoryEvent:
_ = h.handleRepositoryEvent(ctx, db, e)
case *gh.MemberEvent:
_ = h.handleMemberEvent(ctx, db, e, codeHostURN)
case *gh.OrganizationEvent:
_ = h.handleOrganizationEvent(ctx, db, e, codeHostURN)
case *gh.MembershipEvent:
_ = h.handleMembershipEvent(ctx, db, e, codeHostURN)
case *gh.TeamEvent:
_ = h.handleTeamEvent(ctx, e, db)
}
return nil
}

4
cmd/frontend/internal/authz/webhooks/github_test.go
View File

@ -273,7 +273,9 @@ func TestGitHubWebhooks(t *testing.T) {
req := newReq(t, webhookTest.eventType, webhookTest.event)
responseRecorder := httptest.NewRecorder()
hook.ServeHTTP(responseRecorder, req)
go func() {
hook.ServeHTTP(responseRecorder, req)
}()
waitUntil(t, webhookCalled)
})
}