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.

Sign up for GitHub

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

fix: reverting change to command status logic #2173

Merged
merged 1 commit into from
Mar 31, 2022
Merged

fix: reverting change to command status logic #2173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions server/events/apply_command_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
return
}

if err = a.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil {
ctx.Log.Warn("unable to update commit status: %s", err)
}

// Get the mergeable status before we set any build statuses of our own.
// We do this here because when we set a "Pending" status, if users have
// required the Atlantis status checks to pass, then we've now changed
Expand All @@ -108,10 +112,6 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) {
ctx.Log.Warn("unable to get pull request status: %s. Continuing with mergeable and approved assumed false", err)
}

if err = a.commitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil {
ctx.Log.Warn("unable to update commit status: %s", err)
}

var projectCmds []command.ProjectContext
projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd)

Expand Down
11 changes: 0 additions & 11 deletions server/events/plan_command_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,6 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) {
if err := p.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil {
ctx.Log.Warn("unable to update plan commit status: %s", err)
}
if err := p.commitStatusUpdater.UpdateCombinedCount(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply, 0, len(projectCmds)); err != nil {
ctx.Log.Warn("unable to update apply commit status: %s", err)
}

// Only run commands in parallel if enabled
var result command.Result
Expand Down Expand Up @@ -183,14 +180,6 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) {

projectCmds, policyCheckCmds := p.partitionProjectCmds(ctx, projectCmds)

// At this point we are sure Atlantis has work to do, so set commit status to pending
if err := p.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil {
ctx.Log.Warn("unable to update plan commit status: %s", err)
}
if err := p.commitStatusUpdater.UpdateCombinedCount(ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Apply, 0, len(projectCmds)); err != nil {
ctx.Log.Warn("unable to update apply commit status: %s", err)
}

// Only run commands in parallel if enabled
var result command.Result
if p.isParallelEnabled(projectCmds) {
Expand Down
77 changes: 0 additions & 77 deletions server/events/vcs/fixtures/github-commit-status-empty.json
View file
Open in desktop

This file was deleted.

148 changes: 0 additions & 148 deletions server/events/vcs/fixtures/github-commit-status-full.json
View file
Open in desktop

This file was deleted.

28 changes: 0 additions & 28 deletions server/events/vcs/github_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,6 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
return false, errors.Wrap(err, "getting pull request")
}
state := githubPR.GetMergeableState()
status, _ := g.GetCombinedStatus(repo, githubPR.GetHead().GetSHA())

// We map our mergeable check to when the GitHub merge button is clickable.
// This corresponds to the following states:
// clean: No conflicts, all requirements satisfied.
Expand All @@ -314,27 +312,9 @@ func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
// has_hooks: GitHub Enterprise only, if a repo has custom pre-receive
// hooks. Merging is allowed (green box).
// See: https://github.com/octokit/octokit.net/issues/1763
//
// We should not dismiss the PR if the only our "atlantis/apply" status is pending/failing
if state == "blocked" {
applyStatus := false
for _, s := range status.Statuses {
if strings.Contains(s.GetContext(), "atlantis/apply") {
applyStatus = true
continue
}
if s.GetContext() != "atlantis/apply" && s.GetState() != "success" {
// If any other status is pending/failing mark as non-mergeable
return false, nil
}
}
return applyStatus, nil
}

if state != "clean" && state != "unstable" && state != "has_hooks" {
return false, nil
}

return true, nil
}

Expand Down Expand Up @@ -365,14 +345,6 @@ func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRe
return pull, err
}

func (g *GithubClient) GetCombinedStatus(repo models.Repo, ref string) (*github.jparrowsec.cnbinedStatus, error) {
opts := github.ListOptions{
PerPage: 300,
}
status, _, err := g.client.Repositories.GetCombinedStatus(g.ctx, repo.Owner, repo.Name, ref, &opts)
return status, err
}

// UpdateStatus updates the status badge on the pull request.
// See https://github.com/blog/1227-commit-status-api.
func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error {
Expand Down
Loading