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

Add more logging to troubleshoot issue and fix the bot names #18

Merged
merged 3 commits into from
Jun 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
4 changes: 3 additions & 1 deletion cmd/notifyPendingCI.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"context"
"log"
"time"

Expand All @@ -24,10 +25,11 @@ var notifyPendingCICmd = &cobra.Command{

loop := viper.GetBool("loop")
loopDuration := viper.GetDuration("loop-time")
ctx := context.Background()
var listPendingPRs []string
for {
log.Println("Checking for community PRs that are waiting for CI to run")
listPendingPRs, err = github2.CheckForPendingCI(client, cfg)
listPendingPRs, err = github2.CheckForPendingCI(ctx, client, cfg)
if err != nil {
log.Printf("The following error occurred while obtaining a list of pending PRs: %s", err)
time.Sleep(loopDuration)
Expand Down
4 changes: 3 additions & 1 deletion cmd/notifyStale.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"context"
"log"
"time"

Expand All @@ -24,10 +25,11 @@ var notifyStaleCmd = &cobra.Command{

loop := viper.GetBool("loop")
loopDuration := viper.GetDuration("loop-time")
ctx := context.Background()
var listPendingPRs []string
for {
log.Println("Checking for community PRs that have no update in the last 7 days")
_, err = github2.CheckStalePRs(client, cfg)
_, err = github2.CheckStalePRs(ctx, client, cfg)
if err != nil {
log.Printf("The following error occurred while obtaining a list of stale PRs: %s", err)
time.Sleep(loopDuration)
Expand Down
33 changes: 19 additions & 14 deletions internal/github/checkPendingCI.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// CheckForPendingCI returns a list of PR URLs that are ready for CI to run but haven't started yet.
func CheckForPendingCI(githubClient *github.Client, cfg *config.Config) ([]string, error) {
func CheckForPendingCI(ctx context.Context, githubClient *github.Client, cfg *config.Config) ([]string, error) {
teamMembers, _ := GetTeamMemberList(githubClient, cfg.InternalTeam)
var pendingPRs []string

Expand All @@ -34,8 +34,10 @@ func CheckForPendingCI(githubClient *github.Client, cfg *config.Config) ([]strin
}

for _, pr := range communityPRs {
ctx, cancel := context.WithTimeout(ctx, 30*time.Second) // 30 seconds timeout for each request
defer cancel()
// Dynamic cutoff time based on the last commit to the PR
lastCommitTime, err := getLastCommitTime(githubClient, owner, repo, pr.GetNumber())
lastCommitTime, err := getLastCommitTime(ctx, 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 @@ -47,13 +49,13 @@ func CheckForPendingCI(githubClient *github.Client, cfg *config.Config) ([]strin
continue
}

hasCIRuns, err := checkCIStatus(githubClient, owner, repo, pr.GetNumber())
hasCIRuns, err := checkCIStatus(ctx, 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(githubClient, owner, repo, pr.GetNumber(), teamMembers, lastCommitTime)
teamMemberActivity, err := checkTeamMemberActivity(ctx, 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
Expand All @@ -67,51 +69,54 @@ func CheckForPendingCI(githubClient *github.Client, cfg *config.Config) ([]strin
return pendingPRs, nil
}

func getLastCommitTime(client *github.Client, owner, repo string, prNumber int) (time.Time, error) {
commits, _, err := client.PullRequests.ListCommits(context.Background(), owner, repo, prNumber, nil)
func getLastCommitTime(ctx context.Context, client *github.Client, owner, repo string, prNumber int) (time.Time, error) {
commits, _, err := client.PullRequests.ListCommits(ctx, owner, repo, prNumber, nil)
if err != nil {
return time.Time{}, err // Properly handle API errors
}
if len(commits) == 0 {
return time.Time{}, fmt.Errorf("no commits found for PR #%d", prNumber) // Handle case where no commits are found
return time.Time{}, fmt.Errorf("no commits found for PR #%d of repo %s", prNumber, repo) // Handle case where no commits are found
}
// Requesting a list of commits will return the json list in descending order
lastCommit := commits[len(commits)-1]
commitDate := lastCommit.GetCommit().GetAuthor().GetDate() // commitDate is of type Timestamp

// Since GetDate() returns a Timestamp (not *Timestamp), use the address to call GetTime()
commitTime := commitDate.GetTime() // Correctly accessing GetTime(), which returns *time.Time

if commitTime == nil {
return time.Time{}, fmt.Errorf("commit time is nil for PR #%d", prNumber)
return time.Time{}, fmt.Errorf("commit time is nil for PR #%d of repo %s", prNumber, repo)
}
log.Printf("The last commit time is %s for PR #%d of repo %s", commitTime.Format(time.RFC3339), prNumber, repo)

return *commitTime, nil // Safely dereference *time.Time to get time.Time
}

func checkCIStatus(client *github.Client, owner, repo string, prNumber int) (bool, error) {
checks, _, err := client.Checks.ListCheckRunsForRef(context.Background(), owner, repo, strconv.Itoa(prNumber), &github.ListCheckRunsOptions{})
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{})
if err != nil {
return false, err
}
return checks.GetTotal() > 0, nil
}

func checkTeamMemberActivity(client *github.Client, owner, repo string, prNumber int, teamMembers map[string]bool, lastCommitTime time.Time) (bool, error) {
comments, _, err := client.Issues.ListComments(context.Background(), owner, repo, prNumber, nil)
func checkTeamMemberActivity(ctx context.Context, client *github.Client, owner, repo string, prNumber int, teamMembers map[string]bool, lastCommitTime time.Time) (bool, error) {
comments, _, err := client.Issues.ListComments(ctx, owner, repo, prNumber, nil)
if err != nil {
return false, fmt.Errorf("failed to fetch comments: %w", err)
}

for _, comment := range comments {
log.Printf("Checking comment by %s at %s for PR #%d of repo %s", comment.User.GetLogin(), comment.CreatedAt.Format(time.RFC3339), prNumber, repo)
if _, ok := teamMembers[comment.User.GetLogin()]; ok && comment.CreatedAt.After(lastCommitTime) {
log.Printf("Found team member comment after last commit time: %s for PR #%d of repo %s", comment.CreatedAt.Format(time.RFC3339), prNumber, repo)
// Check if the comment is after the last commit
return true, nil // Active and relevant participation
}
}

reviews, _, err := client.PullRequests.ListReviews(context.Background(), owner, repo, prNumber, nil)
if err != nil {
return false, fmt.Errorf("failed to fetch reviews: %w", err)
return false, fmt.Errorf("failed to fetch reviews: %w for PR #%d of repo %s", err, prNumber, repo)
}

for _, review := range reviews {
Expand Down
16 changes: 7 additions & 9 deletions internal/github/checkStalePRs.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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(githubClient *github.Client, cfg *config.Config) ([]string, error) {
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
teamMembers, err := GetTeamMemberList(githubClient, cfg.InternalTeam)
Expand All @@ -25,8 +25,8 @@ func CheckStalePRs(githubClient *github.Client, cfg *config.Config) ([]string, e
}

for _, pr := range communityPRs {
repoName := pr.GetBase().GetRepo().GetFullName() // Get the full name of the repository
stale, err := isStale(githubClient, pr, teamMembers, cutoffDate) // Handle both returned values
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
Expand All @@ -39,23 +39,21 @@ func CheckStalePRs(githubClient *github.Client, cfg *config.Config) ([]string, e
}

// Checks if a PR is stale based on the last update from team members
func isStale(githubClient *github.Client, pr *github.PullRequest, teamMembers map[string]bool, cutoffDate time.Time) (bool, error) {
func isStale(ctx context.Context, githubClient *github.Client, pr *github.PullRequest, teamMembers map[string]bool, cutoffDate time.Time) (bool, error) {
listOptions := &github.ListOptions{PerPage: 100}
for {
// Create a context for each request
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) // 30 seconds timeout 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)
if err != nil {
cancel() // Explicitly cancel the context when an error occurs
return false, fmt.Errorf("failed to get timeline for PR #%d: %w", pr.GetNumber(), err)
return false, fmt.Errorf("failed to get timeline for PR #%d of repo %s: %w", pr.GetNumber(), pr.Base.Repo.GetName(), err)
}
for _, event := range events {
if event.Event == nil || event.Actor == nil || event.Actor.Login == nil {
continue
}
if (*event.Event == "commented" || *event.Event == "reviewed") && teamMembers[*event.Actor.Login] && event.CreatedAt.After(cutoffDate) {
cancel() // Clean up the context when returning within the loop
return false, nil
}
}
Expand Down
2 changes: 1 addition & 1 deletion k8s/notify-pending-prs.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ image:

deployment:
args:
- notify-stale
- notify-pendingci
- --loop

# Creates a secret with the following values, and mounts as a file into the main deployment container
Expand Down
2 changes: 1 addition & 1 deletion k8s/notify-stale-prs.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ image:

deployment:
args:
- notify-pendingci
- notify-stale
- --loop

# Creates a secret with the following values, and mounts as a file into the main deployment container
Expand Down