Skip to content

Commit 1ca00f9

Browse files
authored
fallback strategy, remove sleep block (#2305)
* fallback strategy, remove sleep block * remove comment changes * restore TODO comment * limit scope of fallback * remove escape to default * remove unused getdefault * preserve returned commit sha * remove unused * remove comment
1 parent 869b24d commit 1ca00f9

File tree

3 files changed

+116
-34
lines changed

3 files changed

+116
-34
lines changed

backend/controllers/github_helpers.go

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,84 @@ func retrieveConfigFromCache(orgId uint, repoFullName string) (string, *digger_c
591591
return repoCache.DiggerYmlStr, &config, &projectsGraph, &taConfig, nil
592592
}
593593

594+
// GetDiggerConfigForBranchWithFallback attempts to load config from the specified branch,
595+
// and falls back to the target branch if the branch doesn't exist and shouldFallback is true
596+
func GetDiggerConfigForBranchWithFallback(gh utils.GithubClientProvider, installationId int64, repoFullName string, repoOwner string, repoName string, cloneUrl string, branch string, targetBranch string, shouldFallback bool, changedFiles []string, taConfig *tac.AtlantisConfig) (string, *github2.GithubService, *digger_config.DiggerConfig, graph.Graph[string, digger_config.Project], error) {
597+
slog.Info("Attempting to get Digger config for branch with fallback",
598+
"repoFullName", repoFullName,
599+
"primaryBranch", branch,
600+
"targetBranch", targetBranch,
601+
"shouldFallback", shouldFallback,
602+
)
603+
604+
ghService, _, err := utils.GetGithubService(gh, installationId, repoFullName, repoOwner, repoName)
605+
if err != nil {
606+
return "", nil, nil, nil,fmt.Errorf("error getting github service")
607+
}
608+
609+
// Try the primary branch first
610+
diggerYmlStr, ghService, config, dependencyGraph, err := GetDiggerConfigForBranch(
611+
gh, installationId, repoFullName, repoOwner, repoName, cloneUrl, branch, changedFiles, taConfig,
612+
)
613+
614+
actualBranchUsed := branch
615+
616+
if err != nil {
617+
// Check if it's a "branch not found" error
618+
errMsg := err.Error()
619+
isBranchNotFound := strings.Contains(errMsg, "Remote branch") && strings.Contains(errMsg, "not found") ||
620+
strings.Contains(errMsg, "couldn't find remote ref") ||
621+
strings.Contains(errMsg, "exit status 128")
622+
623+
if isBranchNotFound && branch != targetBranch && shouldFallback {
624+
slog.Warn("Branch not found and fallback enabled, falling back to target branch",
625+
"missingBranch", branch,
626+
"targetBranch", targetBranch,
627+
"repoFullName", repoFullName,
628+
"originalError", err,
629+
)
630+
631+
// Try the target branch as fallback
632+
var fallbackErr error
633+
diggerYmlStr, ghService, config, dependencyGraph, fallbackErr = GetDiggerConfigForBranch(
634+
gh, installationId, repoFullName, repoOwner, repoName, cloneUrl, targetBranch, changedFiles, taConfig,
635+
)
636+
637+
if fallbackErr != nil {
638+
slog.Error("Fallback to target branch also failed",
639+
"targetBranch", targetBranch,
640+
"repoFullName", repoFullName,
641+
"fallbackError", fallbackErr,
642+
)
643+
return "", nil, nil, nil, fmt.Errorf("failed to load config from branch '%s' (branch not found) and fallback to target branch '%s' also failed: %v", branch, targetBranch, fallbackErr)
644+
}
645+
646+
slog.Info("Successfully loaded config from target branch",
647+
"targetBranch", targetBranch,
648+
"repoFullName", repoFullName,
649+
)
650+
651+
actualBranchUsed = targetBranch
652+
} else {
653+
// Either not a branch-not-found error, or fallback is disabled
654+
if isBranchNotFound && !shouldFallback {
655+
slog.Info("Branch not found but fallback is disabled",
656+
"missingBranch", branch,
657+
"repoFullName", repoFullName,
658+
)
659+
}
660+
return "", nil, nil, nil, err
661+
}
662+
}
663+
664+
slog.Info("Config loaded successfully",
665+
"repoFullName", repoFullName,
666+
"branchUsed", actualBranchUsed,
667+
)
668+
669+
return diggerYmlStr, ghService, config, dependencyGraph, nil
670+
}
671+
594672
// TODO: Refactor this func to receive ghService as input
595673
func getDiggerConfigForPR(gh utils.GithubClientProvider, orgId uint, prLabels []string, installationId int64, repoFullName string, repoOwner string, repoName string, cloneUrl string, prNumber int) (string, *github2.GithubService, *digger_config.DiggerConfig, graph.Graph[string, digger_config.Project], *string, *string, []string, error) {
596674
slog.Info("Getting Digger config for PR",
@@ -616,7 +694,8 @@ func getDiggerConfigForPR(gh utils.GithubClientProvider, orgId uint, prLabels []
616694
}
617695

618696
var prBranch string
619-
prBranch, prCommitSha, _, _, err := ghService.GetBranchName(prNumber)
697+
var targetBranch string
698+
prBranch, prCommitSha, targetBranch, _, err := ghService.GetBranchName(prNumber)
620699
if err != nil {
621700
slog.Error("Error getting branch name for PR",
622701
"prNumber", prNumber,
@@ -626,10 +705,20 @@ func getDiggerConfigForPR(gh utils.GithubClientProvider, orgId uint, prLabels []
626705
return "", nil, nil, nil, nil, nil, nil, fmt.Errorf("error getting branch name")
627706
}
628707

708+
// Target branch must be available - no fallback to repo default
709+
if targetBranch == "" {
710+
slog.Error("PR target branch is empty",
711+
"prNumber", prNumber,
712+
"repoFullName", repoFullName,
713+
)
714+
return "", nil, nil, nil, nil, nil, nil, fmt.Errorf("PR target branch is empty for PR #%d", prNumber)
715+
}
716+
629717
slog.Debug("Retrieved PR details",
630718
"prNumber", prNumber,
631719
"branch", prBranch,
632720
"commitSha", prCommitSha,
721+
"targetBranch", targetBranch,
633722
)
634723

635724
changedFiles, err := ghService.GetChangedFiles(prNumber)
@@ -684,15 +773,39 @@ func getDiggerConfigForPR(gh utils.GithubClientProvider, orgId uint, prLabels []
684773
slog.Info("Loading config from repository",
685774
"repoFullName", repoFullName,
686775
"branch", prBranch,
776+
"targetBranch", targetBranch,
687777
"prNumber", prNumber,
688778
)
689779

690-
diggerYmlStr, ghService, config, dependencyGraph, err := GetDiggerConfigForBranch(gh, installationId, repoFullName, repoOwner, repoName, cloneUrl, prBranch, changedFiles, taConfig)
780+
// Check if PR is merged to determine if we should fallback
781+
shouldFallback := false
782+
isMerged, err := ghService.IsMerged(prNumber)
783+
if err != nil {
784+
slog.Warn("Could not check PR merge status, will not enable fallback",
785+
"prNumber", prNumber,
786+
"repoFullName", repoFullName,
787+
"error", err,
788+
)
789+
} else {
790+
shouldFallback = isMerged
791+
slog.Debug("PR merge status checked",
792+
"prNumber", prNumber,
793+
"isMerged", isMerged,
794+
"shouldFallback", shouldFallback,
795+
)
796+
}
797+
798+
// Use the fallback method with merge-based fallback logic
799+
diggerYmlStr, ghService, config, dependencyGraph, err := GetDiggerConfigForBranchWithFallback(
800+
gh, installationId, repoFullName, repoOwner, repoName, cloneUrl,
801+
prBranch, targetBranch, shouldFallback, changedFiles, taConfig,
802+
)
691803
if err != nil {
692804
slog.Error("Error loading Digger config from repository",
693805
"prNumber", prNumber,
694806
"repoFullName", repoFullName,
695807
"branch", prBranch,
808+
"targetBranch", targetBranch,
696809
"error", err,
697810
)
698811
return "", nil, nil, nil, nil, nil, nil, fmt.Errorf("error loading digger.yml: %v", err)

backend/controllers/github_pull_request.go

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"runtime/debug"
99
"slices"
1010
"strconv"
11-
"time"
1211

1312
"github.com/diggerhq/digger/backend/ci_backends"
1413
config2 "github.com/diggerhq/digger/backend/config"
@@ -101,36 +100,6 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR
101100
return fmt.Errorf("error getting ghService to post error comment")
102101
}
103102

104-
// here we check if pr was closed and automatic deletion is enabled, to avoid errors when
105-
// pr is merged and the branch does not exist we handle that gracefully
106-
if action == "closed" {
107-
slog.Debug("Handling closed PR action", "prNumber", prNumber)
108-
// we sleep for 1 second to give github time to delete the branch
109-
time.Sleep(3 * time.Second)
110-
111-
branchName, _, _, _, err := ghService.GetBranchName(prNumber)
112-
if err != nil {
113-
slog.Error("Could not retrieve PR details", "prNumber", prNumber, "error", err)
114-
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: Could not retrieve PR details, error: %v", err))
115-
return fmt.Errorf("Could not retrieve PR details: %v", err)
116-
}
117-
118-
branchExists, err := ghService.CheckBranchExists(branchName)
119-
if err != nil {
120-
slog.Error("Could not check if branch exists", "prNumber", prNumber, "branchName", branchName, "error", err)
121-
utils.InitCommentReporter(ghService, prNumber, fmt.Sprintf(":x: Could not check if branch exists, error: %v", err))
122-
return fmt.Errorf("Could not check if branch exists: %v", err)
123-
}
124-
125-
if !branchExists {
126-
slog.Info("Branch no longer exists, ignoring PR closed event",
127-
"prNumber", prNumber,
128-
"branchName", branchName,
129-
)
130-
return nil
131-
}
132-
}
133-
134103
if !slices.Contains([]string{"closed", "opened", "reopened", "synchronize", "converted_to_draft"}, action) {
135104
slog.Info("Ignoring event with action not requiring processing", "action", action, "prNumber", prNumber)
136105
return nil

libs/ci/github/github.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -859,4 +859,4 @@ func CheckIfShowProjectsComment(event interface{}) bool {
859859
slog.Debug("show-projects comment detected")
860860
}
861861
return result
862-
}
862+
}

0 commit comments

Comments
 (0)