Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove sns stuff from gateway #766

Merged
merged 3 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions server/legacy/lyft/gateway/events_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ func NewVCSEventsController(
scope tally.Scope,
webhookSecret []byte,
allowDraftPRs bool,
snsWriter gateway_handlers.Writer,
commentParser events.CommentParsing,
repoAllowlistChecker *events.RepoAllowlistChecker,
vcsClient vcs.Client,
Expand All @@ -61,19 +60,10 @@ func NewVCSEventsController(
clientCreator githubapp.ClientCreator,
defaultTFVersion string,
) *VCSEventsController {
pullEventSNSProxy := gateway_handlers.NewSNSWorkerProxy(
snsWriter, logger,
)
legacyHandler := &gateway_handlers.LegacyPullHandler{
Logger: logger,
WorkerProxy: pullEventSNSProxy,
VCSStatusUpdater: vcsStatusUpdater,
}
prSignaler := &pr.WorkflowSignaler{TemporalClient: temporalClient, DefaultTFVersion: defaultTFVersion}
prRequirementChecker := requirement.NewPRAggregate(globalCfg)
modifiedPullHandler := gateway_handlers.NewModifiedPullHandler(logger, asyncScheduler, rootConfigBuilder, globalCfg, prRequirementChecker, prSignaler, legacyHandler)
modifiedPullHandler := gateway_handlers.NewModifiedPullHandler(logger, asyncScheduler, rootConfigBuilder, globalCfg, prRequirementChecker, prSignaler)
closedPullHandler := &gateway_handlers.ClosedPullRequestHandler{
WorkerProxy: pullEventSNSProxy,
Logger: logger,
PRCloseSignaler: prSignaler,
Scope: scope.SubScope("pull.closed"),
Expand Down Expand Up @@ -118,7 +108,6 @@ func NewVCSEventsController(
vcsClient,
gateway_handlers.NewCommentEventWorkerProxy(
logger,
snsWriter,
asyncScheduler,
prSignaler,
deploySignaler,
Expand Down Expand Up @@ -154,7 +143,6 @@ func NewVCSEventsController(

pullRequestReviewHandler := &gateway_handlers.PullRequestReviewWorkerProxy{
Scheduler: asyncScheduler,
SnsWriter: snsWriter,
Logger: logger,
CheckRunFetcher: checkRunFetcher,
WorkflowSignaler: prSignaler,
Expand Down
5 changes: 0 additions & 5 deletions server/neptune/gateway/event/closed_pull_request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,12 @@ type prCloseSignaler interface {
}

type ClosedPullRequestHandler struct {
WorkerProxy workerProxy
Logger logging.Logger
PRCloseSignaler prCloseSignaler
Scope tally.Scope
}

func (c *ClosedPullRequestHandler) Handle(ctx context.Context, request *http.BufferedRequest, event PullRequest) error {
if err := c.WorkerProxy.Handle(ctx, request, event); err != nil {
c.Logger.ErrorContext(ctx, err.Error())
}

if err := c.handlePlatformMode(ctx, event); err != nil {
return errors.Wrap(err, "handling platform mode")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ import (
)

func TestClosedPullHandler_Handle(t *testing.T) {
workerProxy := &mockWorkerProxy{}
signaler := &testCloseSignaler{
t: t,
expectedRepoName: "repo",
expectedPullNum: 1,
}
pullHandler := event.ClosedPullRequestHandler{
Logger: logging.NewNoopCtxLogger(t),
WorkerProxy: workerProxy,
PRCloseSignaler: signaler,
}
pr := event.PullRequest{
Expand All @@ -37,12 +35,10 @@ func TestClosedPullHandler_Handle(t *testing.T) {
}
err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, pr)
assert.True(t, signaler.called)
assert.True(t, workerProxy.called)
assert.NoError(t, err)
}

func TestClosedPullHandler_Handle_SignalError(t *testing.T) {
workerProxy := &mockWorkerProxy{}
signaler := &testCloseSignaler{
t: t,
err: assert.AnError,
Expand All @@ -51,7 +47,6 @@ func TestClosedPullHandler_Handle_SignalError(t *testing.T) {
}
pullHandler := event.ClosedPullRequestHandler{
Logger: logging.NewNoopCtxLogger(t),
WorkerProxy: workerProxy,
PRCloseSignaler: signaler,
}
pr := event.PullRequest{
Expand All @@ -65,12 +60,10 @@ func TestClosedPullHandler_Handle_SignalError(t *testing.T) {
}
err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, pr)
assert.True(t, signaler.called)
assert.True(t, workerProxy.called)
assert.Error(t, err)
}

func TestClosedPullHandler_Handle_SignalNotFoundError(t *testing.T) {
workerProxy := &mockWorkerProxy{}
signaler := &testCloseSignaler{
t: t,
expectedRepoName: "repo",
Expand All @@ -79,7 +72,6 @@ func TestClosedPullHandler_Handle_SignalNotFoundError(t *testing.T) {
}
pullHandler := event.ClosedPullRequestHandler{
Logger: logging.NewNoopCtxLogger(t),
WorkerProxy: workerProxy,
PRCloseSignaler: signaler,
Scope: tally.NewTestScope("", map[string]string{}),
}
Expand All @@ -94,7 +86,6 @@ func TestClosedPullHandler_Handle_SignalNotFoundError(t *testing.T) {
}
err := pullHandler.Handle(context.Background(), &http.BufferedRequest{}, pr)
assert.True(t, signaler.called)
assert.True(t, workerProxy.called)
assert.NoError(t, err)
}

Expand Down
11 changes: 1 addition & 10 deletions server/neptune/gateway/event/comment_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,10 @@ func (c Comment) GetRepo() models.Repo {
return c.BaseRepo
}

func NewCommentEventWorkerProxy(logger logging.Logger, snsWriter Writer, scheduler scheduler, prSignaler prSignaler, deploySignaler deploySignaler, commentCreator commentCreator, vcsStatusUpdater statusUpdater, globalCfg valid.GlobalCfg, rootConfigBuilder rootConfigBuilder, legacyErrorHandler errorHandler, neptuneErrorHandler errorHandler, requirementChecker requirementChecker) *CommentEventWorkerProxy {
func NewCommentEventWorkerProxy(logger logging.Logger, scheduler scheduler, prSignaler prSignaler, deploySignaler deploySignaler, commentCreator commentCreator, vcsStatusUpdater statusUpdater, globalCfg valid.GlobalCfg, rootConfigBuilder rootConfigBuilder, legacyErrorHandler errorHandler, neptuneErrorHandler errorHandler, requirementChecker requirementChecker) *CommentEventWorkerProxy {
return &CommentEventWorkerProxy{
logger: logger,
scheduler: scheduler,
legacyHandler: &LegacyCommentHandler{
logger: logger,
snsWriter: snsWriter,
globalCfg: globalCfg,
},
neptuneWorkerProxy: &NeptuneWorkerProxy{
logger: logger,
deploySignaler: deploySignaler,
Expand Down Expand Up @@ -177,7 +172,6 @@ type CommentEventWorkerProxy struct {
scheduler scheduler
vcsStatusUpdater statusUpdater
rootConfigBuilder rootConfigBuilder
legacyHandler *LegacyCommentHandler
neptuneWorkerProxy *NeptuneWorkerProxy
neptuneErrorHandler errorHandler
legacyErrorHandler errorHandler
Expand Down Expand Up @@ -209,9 +203,6 @@ func (p *CommentEventWorkerProxy) handle(ctx context.Context, request *http.Buff
}

fxns := []sync.Executor{
p.legacyErrorHandler.WrapWithHandling(ctx, event, cmd.CommandName().String(), func(ctx context.Context) error {
return p.legacyHandler.Handle(ctx, event, cmd, roots, request)
}),
p.neptuneErrorHandler.WrapWithHandling(ctx, event, cmd.CommandName().String(), func(ctx context.Context) error {
return p.neptuneWorkerProxy.Handle(ctx, event, cmd, roots, request)
}),
Expand Down
29 changes: 7 additions & 22 deletions server/neptune/gateway/event/comment_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ func TestCommentEventWorkerProxy_HandleForceApply(t *testing.T) {
},
},
}
writer := &mockSnsWriter{}
scheduler := &sync.SynchronousScheduler{Logger: logger}
commentCreator := &mockCommentCreator{
expectedT: t,
Expand All @@ -173,7 +172,7 @@ func TestCommentEventWorkerProxy_HandleForceApply(t *testing.T) {
prSignaler := &mockPRSignaler{
expectedT: t,
}
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
bufReq := buildRequest(t)
cmd := &command.Comment{
Name: command.Apply,
Expand All @@ -183,7 +182,6 @@ func TestCommentEventWorkerProxy_HandleForceApply(t *testing.T) {
assert.NoError(t, err)
assert.True(t, commentCreator.isCalled)
assert.True(t, testSignaler.called())
assert.False(t, writer.isCalled)
assert.False(t, statusUpdater.isCalled)
}

Expand Down Expand Up @@ -222,12 +220,11 @@ func TestCommentEventWorkerProxy_HandleApplyComment_RequirementsFailed(t *testin
expectedT: t,
}

writer := &mockSnsWriter{}
scheduler := &sync.SynchronousScheduler{Logger: logger}
commentCreator := &mockCommentCreator{}
statusUpdater := &mockStatusUpdater{}
cfg := valid.NewGlobalCfg("somedir")
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{
err: assert.AnError,
})
bufReq := buildRequest(t)
Expand All @@ -239,7 +236,6 @@ func TestCommentEventWorkerProxy_HandleApplyComment_RequirementsFailed(t *testin
assert.False(t, statusUpdater.isCalled)
assert.False(t, commentCreator.isCalled)
assert.False(t, testSignaler.called)
assert.False(t, writer.isCalled)
}

func TestCommentEventWorkerProxy_HandleApplyComment(t *testing.T) {
Expand Down Expand Up @@ -297,16 +293,14 @@ func TestCommentEventWorkerProxy_HandleApplyComment(t *testing.T) {
},
},
}

writer := &mockSnsWriter{}
scheduler := &sync.SynchronousScheduler{Logger: logger}
commentCreator := &mockCommentCreator{}
statusUpdater := &mockStatusUpdater{}
cfg := valid.NewGlobalCfg("somedir")
prSignaler := &mockPRSignaler{
expectedT: t,
}
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
bufReq := buildRequest(t)
cmd := &command.Comment{
Name: command.Apply,
Expand All @@ -316,7 +310,6 @@ func TestCommentEventWorkerProxy_HandleApplyComment(t *testing.T) {
assert.False(t, statusUpdater.isCalled)
assert.False(t, commentCreator.isCalled)
assert.True(t, testSignaler.called())
assert.False(t, writer.isCalled)
}

func TestCommentEventWorkerProxy_HandlePlanComment_NoCmds(t *testing.T) {
Expand All @@ -342,7 +335,6 @@ func TestCommentEventWorkerProxy_HandlePlanComment_NoCmds(t *testing.T) {
},
InstallationToken: 123,
}
writer := &mockSnsWriter{}
scheduler := &sync.SynchronousScheduler{Logger: logger}
commentCreator := &mockCommentCreator{}
statusUpdater := &multiMockStatusUpdater{
Expand Down Expand Up @@ -377,7 +369,7 @@ func TestCommentEventWorkerProxy_HandlePlanComment_NoCmds(t *testing.T) {
prSignaler := &mockPRSignaler{
expectedT: t,
}
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
bufReq := buildRequest(t)
cmd := &command.Comment{
Name: command.Plan,
Expand All @@ -387,7 +379,6 @@ func TestCommentEventWorkerProxy_HandlePlanComment_NoCmds(t *testing.T) {
assert.False(t, statusUpdater.AllCalled())
assert.False(t, commentCreator.isCalled)
assert.False(t, testSignaler.called)
assert.False(t, writer.isCalled)
}

func TestCommentEventWorkerProxy_HandleApplyComment_NoCmds(t *testing.T) {
Expand All @@ -413,7 +404,6 @@ func TestCommentEventWorkerProxy_HandleApplyComment_NoCmds(t *testing.T) {
},
InstallationToken: 123,
}
writer := &mockSnsWriter{}
scheduler := &sync.SynchronousScheduler{Logger: logger}
commentCreator := &mockCommentCreator{}
statusUpdater := &multiMockStatusUpdater{
Expand All @@ -432,7 +422,7 @@ func TestCommentEventWorkerProxy_HandleApplyComment_NoCmds(t *testing.T) {
prSignaler := &mockPRSignaler{
expectedT: t,
}
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
bufReq := buildRequest(t)
cmd := &command.Comment{
Name: command.Apply,
Expand All @@ -442,7 +432,6 @@ func TestCommentEventWorkerProxy_HandleApplyComment_NoCmds(t *testing.T) {
assert.False(t, statusUpdater.AllCalled())
assert.False(t, commentCreator.isCalled)
assert.False(t, testSignaler.called)
assert.False(t, writer.isCalled)
}

func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) {
Expand Down Expand Up @@ -490,7 +479,6 @@ func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) {
},
InstallationToken: 123,
}
writer := &mockSnsWriter{}
scheduler := &sync.SynchronousScheduler{Logger: logger}
commentCreator := &mockCommentCreator{}
statusUpdater := &mockStatusUpdater{}
Expand All @@ -500,7 +488,7 @@ func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) {
expectedRoots: roots,
expectedPRRequest: prRequest,
}
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, deploySignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, scheduler, prSignaler, deploySignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
bufReq := buildRequest(t)
cmd := &command.Comment{
Name: command.Plan,
Expand All @@ -511,7 +499,6 @@ func TestCommentEventWorkerProxy_HandlePlanComment(t *testing.T) {
assert.False(t, commentCreator.isCalled)
assert.False(t, deploySignaler.called)
assert.True(t, prSignaler.called)
assert.True(t, writer.isCalled)
}

func TestCommentEventWorkerProxy_HandlePlanCommentAllocatorEnabled(t *testing.T) {
Expand Down Expand Up @@ -559,7 +546,6 @@ func TestCommentEventWorkerProxy_HandlePlanCommentAllocatorEnabled(t *testing.T)
},
InstallationToken: 123,
}
writer := &mockSnsWriter{}
scheduler := &sync.SynchronousScheduler{Logger: logger}
commentCreator := &mockCommentCreator{}
statusUpdater := &mockStatusUpdater{}
Expand All @@ -569,7 +555,7 @@ func TestCommentEventWorkerProxy_HandlePlanCommentAllocatorEnabled(t *testing.T)
expectedRoots: roots,
expectedPRRequest: prRequest,
}
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, writer, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
commentEventWorkerProxy := event.NewCommentEventWorkerProxy(logger, scheduler, prSignaler, testSignaler, commentCreator, statusUpdater, cfg, rootConfigBuilder, noopErrorHandler{}, noopErrorHandler{}, &requirementsChecker{})
bufReq := buildRequest(t)
cmd := &command.Comment{
Name: command.Plan,
Expand All @@ -579,7 +565,6 @@ func TestCommentEventWorkerProxy_HandlePlanCommentAllocatorEnabled(t *testing.T)
assert.False(t, statusUpdater.isCalled)
assert.False(t, commentCreator.isCalled)
assert.False(t, testSignaler.called)
assert.True(t, writer.isCalled)
assert.True(t, prSignaler.called)
}

Expand Down
43 changes: 0 additions & 43 deletions server/neptune/gateway/event/legacy_comment_handler.go

This file was deleted.

Loading
Loading