Skip to content

Commit

Permalink
Merge pull request #20 from Chia-Network/fix-looping-contexts
Browse files Browse the repository at this point in the history 
Fix issues where functions were using the same PR numbers for differe…
  • Loading branch information
@pmaslana
pmaslana authored Jun 13, 2024
2 parents f2392f7 + 2f91f1b commit fd2a6d1
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 85 deletions.
33 changes: 23 additions & 10 deletions internal/github/checkPendingCI.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"log"
"strconv"
"strings"
"time"

Expand All @@ -28,16 +27,16 @@ func CheckForPendingCI(ctx context.Context, githubClient *github.Client, cfg *co
owner, repo := parts[0], parts[1]

// Fetch community PRs using the FindCommunityPRs function
communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient)
communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient, owner, repo, fullRepo.MinimumNumber)
if err != nil {
return nil, err
}

for _, pr := range communityPRs {
ctx, cancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer cancel()
prctx, prcancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer prcancel()
// Dynamic cutoff time based on the last commit to the PR
lastCommitTime, err := getLastCommitTime(ctx, githubClient, owner, repo, pr.GetNumber())
lastCommitTime, err := getLastCommitTime(prctx, githubClient, owner, repo, pr.GetNumber())
if err != nil {
log.Printf("Error retrieving last commit time for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
continue
Expand All @@ -49,18 +48,18 @@ func CheckForPendingCI(ctx context.Context, githubClient *github.Client, cfg *co
continue
}

hasCIRuns, err := checkCIStatus(ctx, githubClient, owner, repo, pr.GetNumber())
hasCIRuns, err := checkCIStatus(prctx, githubClient, owner, repo, pr.GetNumber())
if err != nil {
log.Printf("Error checking CI status for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
continue
}

teamMemberActivity, err := checkTeamMemberActivity(ctx, githubClient, owner, repo, pr.GetNumber(), teamMembers, lastCommitTime)
teamMemberActivity, err := checkTeamMemberActivity(prctx, githubClient, owner, repo, pr.GetNumber(), teamMembers, lastCommitTime)
if err != nil {
log.Printf("Error checking team member activity for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
continue // or handle the error as needed
}
if !hasCIRuns || !teamMemberActivity {
if !hasCIRuns && !teamMemberActivity {
log.Printf("PR #%d in %s/%s by %s is ready for CI since %v but no CI actions have started yet, or it requires re-approval.", pr.GetNumber(), owner, repo, pr.User.GetLogin(), pr.CreatedAt)
pendingPRs = append(pendingPRs, pr.GetHTMLURL())
}
Expand Down Expand Up @@ -92,10 +91,24 @@ func getLastCommitTime(ctx context.Context, client *github.Client, owner, repo s
}

func checkCIStatus(ctx context.Context, client *github.Client, owner, repo string, prNumber int) (bool, error) {
checks, _, err := client.Checks.ListCheckRunsForRef(ctx, owner, repo, strconv.Itoa(prNumber), &github.ListCheckRunsOptions{})
pr, _, err := client.PullRequests.Get(ctx, owner, repo, prNumber)
if err != nil {
return false, err
return false, fmt.Errorf("failed to fetch pull request #%d: %w", prNumber, err)
}
// Obtaining CI runs for the most recent commit
commitSHA := pr.GetHead().GetSHA()

checks, _, err := client.Checks.ListCheckRunsForRef(ctx, owner, repo, commitSHA, &github.ListCheckRunsOptions{})
if err != nil {
return false, fmt.Errorf("failed to fetch check runs for ref %s: %w", commitSHA, err)
}
// Potentially add logic later for checking for passing CI
/*
for _, checkRun := range checks.CheckRuns {
if checkRun.GetStatus() != "completed" || checkRun.GetConclusion() != "success" {
return false, nil // There are check runs that are not completed or not successful
}
*/
return checks.GetTotal() > 0, nil
}

Expand Down
43 changes: 27 additions & 16 deletions internal/github/checkStalePRs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log"
"strings"
"time"

"github.com/google/go-github/v60/github"
Expand All @@ -14,25 +15,35 @@ import (
// CheckStalePRs will return a list of PR URLs that have not been updated in the last 7 days by internal team members.
func CheckStalePRs(ctx context.Context, githubClient *github.Client, cfg *config.Config) ([]string, error) {
var stalePRUrls []string
cutoffDate := time.Now().Add(7 * 24 * time.Hour) // 7 days ago
cutoffDate := time.Now().Add(-7 * 24 * time.Hour) // 7 days ago
teamMembers, err := GetTeamMemberList(githubClient, cfg.InternalTeam)
if err != nil {
return nil, err
}
communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient)
if err != nil {
return nil, err
}

for _, pr := range communityPRs {
repoName := pr.GetBase().GetRepo().GetFullName() // Get the full name of the repository
stale, err := isStale(ctx, githubClient, pr, teamMembers, cutoffDate) // Handle both returned values
for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
return nil, fmt.Errorf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
}
owner, repo := parts[0], parts[1]

communityPRs, err := FindCommunityPRs(cfg, teamMembers, githubClient, owner, repo, fullRepo.MinimumNumber)
if err != nil {
log.Printf("Error checking if PR in repo %s is stale: %v", repoName, err)
continue // Skip this PR or handle the error appropriately
return nil, err
}
if stale {
stalePRUrls = append(stalePRUrls, pr.GetHTMLURL()) // Append if PR is confirmed stale

for _, pr := range communityPRs {
repoName := pr.GetBase().GetRepo().GetFullName() // Get the full name of the repository
stale, err := isStale(ctx, githubClient, pr, teamMembers, cutoffDate) // Handle both returned values
if err != nil {
log.Printf("Error checking if PR in repo %s is stale: %v", repoName, err)
continue // Skip this PR or handle the error appropriately
}
if stale {
stalePRUrls = append(stalePRUrls, pr.GetHTMLURL()) // Append if PR is confirmed stale
}
}
}
return stalePRUrls, nil
Expand All @@ -43,9 +54,9 @@ func isStale(ctx context.Context, githubClient *github.Client, pr *github.PullRe
listOptions := &github.ListOptions{PerPage: 100}
for {
// Create a context for each request
ctx, cancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer cancel()
events, resp, err := githubClient.Issues.ListIssueTimeline(ctx, pr.Base.Repo.Owner.GetLogin(), pr.Base.Repo.GetName(), pr.GetNumber(), listOptions)
staleCtx, staleCancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer staleCancel()
events, resp, err := githubClient.Issues.ListIssueTimeline(staleCtx, pr.Base.Repo.Owner.GetLogin(), pr.Base.Repo.GetName(), pr.GetNumber(), listOptions)
if err != nil {
return false, fmt.Errorf("failed to get timeline for PR #%d of repo %s: %w", pr.GetNumber(), pr.Base.Repo.GetName(), err)
}
Expand All @@ -57,7 +68,7 @@ func isStale(ctx context.Context, githubClient *github.Client, pr *github.PullRe
return false, nil
}
}
cancel() // Clean up the context at the end of the loop iteration
staleCancel() // Clean up the context at the end of the loop iteration
if resp.NextPage == 0 {
break
}
Expand Down
47 changes: 18 additions & 29 deletions internal/github/communityPRs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ package github
import (
"context"
"fmt"
"log"
"strings"

"github.com/google/go-github/v60/github"

"github.com/chia-network/github-bot/internal/config"
)

// FindCommunityPRs obtains PRs based on provided filters
func FindCommunityPRs(cfg *config.Config, teamMembers map[string]bool, githubClient *github.Client) ([]*github.PullRequest, error) {
func FindCommunityPRs(cfg *config.Config, teamMembers map[string]bool, githubClient *github.Client, owner string, repo string, minimumNumber int) ([]*github.PullRequest, error) {
var finalPRs []*github.PullRequest
opts := &github.PullRequestListOptions{
State: "open",
Expand All @@ -24,38 +22,29 @@ func FindCommunityPRs(cfg *config.Config, teamMembers map[string]bool, githubCli
},
}

for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
return nil, fmt.Errorf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
for {
pullRequests, resp, err := githubClient.PullRequests.List(context.Background(), owner, repo, opts)
if err != nil {
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", owner, repo, err)
}
owner, repo := parts[0], parts[1]

for {
pullRequests, resp, err := githubClient.PullRequests.List(context.TODO(), owner, repo, opts)
if err != nil {
return nil, fmt.Errorf("error listing pull requests for %s/%s: %w", owner, repo, err)
for _, pullRequest := range pullRequests {
if *pullRequest.Number < minimumNumber {
break
}

for _, pullRequest := range pullRequests {
if *pullRequest.Number < fullRepo.MinimumNumber {
break
}
if *pullRequest.Draft {
continue
}
user := *pullRequest.User.Login
if !teamMembers[user] && !cfg.SkipUsersMap[user] {
finalPRs = append(finalPRs, pullRequest)
}
if *pullRequest.Draft {
continue
}

if resp.NextPage == 0 {
break // Exit the loop if there are no more pages
user := *pullRequest.User.Login
if !teamMembers[user] && !cfg.SkipUsersMap[user] {
finalPRs = append(finalPRs, pullRequest)
}
opts.Page = resp.NextPage // Set next page number
}

if resp.NextPage == 0 {
break // Exit the loop if there are no more pages
}
opts.Page = resp.NextPage // Set next page number
}
return finalPRs, nil
}
70 changes: 40 additions & 30 deletions internal/label/pullrequests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log"
"strings"

"github.com/google/go-github/v60/github"

Expand All @@ -18,43 +19,52 @@ func PullRequests(githubClient *github.Client, cfg *config.Config) error {
return fmt.Errorf("error getting team members: %w", err) // Properly handle and return error if team member list fetch fails
}

pullRequests, err := github2.FindCommunityPRs(cfg, teamMembers, githubClient)
if err != nil {
return fmt.Errorf("error finding community PRs: %w", err) // Handle error from finding community PRs
}

for _, pullRequest := range pullRequests {
user := *pullRequest.User.Login
for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
log.Printf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
continue
}
owner, repo := parts[0], parts[1]

var label string
if teamMembers[user] {
label = cfg.LabelInternal
} else {
label = cfg.LabelExternal
pullRequests, err := github2.FindCommunityPRs(cfg, teamMembers, githubClient, owner, repo, fullRepo.MinimumNumber)
if err != nil {
return fmt.Errorf("error finding community PRs: %w", err) // Handle error from finding community PRs
}
if label != "" {
log.Printf("Pull Request %d by %s will be labeled %s\n", *pullRequest.Number, user, label)
hasLabel := false
for _, existingLabel := range pullRequest.Labels {
if *existingLabel.Name == label {
log.Println("Already labeled, skipping...")
hasLabel = true
break
}
}

if !hasLabel {
allLabels := []string{label}
for _, labelP := range pullRequest.Labels {
allLabels = append(allLabels, *labelP.Name)
for _, pullRequest := range pullRequests {
user := *pullRequest.User.Login

var label string
if teamMembers[user] {
label = cfg.LabelInternal
} else {
label = cfg.LabelExternal
}
if label != "" {
log.Printf("Pull Request %d by %s will be labeled %s\n", *pullRequest.Number, user, label)
hasLabel := false
for _, existingLabel := range pullRequest.Labels {
if *existingLabel.Name == label {
log.Println("Already labeled, skipping...")
hasLabel = true
break
}
}
_, _, err := githubClient.Issues.AddLabelsToIssue(context.TODO(), *pullRequest.Base.Repo.Owner.Login, *pullRequest.Base.Repo.Name, *pullRequest.Number, allLabels)
if err != nil {
return fmt.Errorf("error adding labels to pull request %d: %w", *pullRequest.Number, err) // Ensure error from label adding is handled

if !hasLabel {
allLabels := []string{label}
for _, labelP := range pullRequest.Labels {
allLabels = append(allLabels, *labelP.Name)
}
_, _, err := githubClient.Issues.AddLabelsToIssue(context.TODO(), *pullRequest.Base.Repo.Owner.Login, *pullRequest.Base.Repo.Name, *pullRequest.Number, allLabels)
if err != nil {
return fmt.Errorf("error adding labels to pull request %d: %w", *pullRequest.Number, err) // Ensure error from label adding is handled
}
}
}
}
}

return nil
}

0 comments on commit fd2a6d1

Please sign in to comment.