Unicron implemented rest fallback when graphql fails prod

cla-backend-go/github/github_repository.go

@@ -1220,6 +1220,90 @@ func GetCommitAuthorsSignedStatuses(
 	return signed, unsigned
 }
 
+//nolint:gocyclo // complexity is acceptable for now
+func GetPullRequestCommitAuthorsREST(ctx context.Context, usersService users.Service, installationID int64, pullRequestID int, owner, repo string, withCoAuthors bool) ([]*UserCommitSummary, bool, error) {
+	f := logrus.Fields{
+		"functionName":  "github.github_repository.GetPullRequestCommitAuthorsREST",
+		"pullRequestID": pullRequestID,
+		"withCoAuthors": withCoAuthors,
+	}
+	var userCommitSummary []*UserCommitSummary
+	var mu sync.Mutex
+
+	client, err := NewGithubAppClient(installationID)
+	if err != nil {
+		log.WithFields(f).WithError(err).Warn("unable to create Github client")
+		return nil, false, err
+	}
+
+	anyMissing := false
+	opts := &github.ListOptions{PerPage: 100}
+	for {
+		commits, resp, comErr := client.PullRequests.ListCommits(ctx, owner, repo, pullRequestID, opts)
+		if comErr != nil {
+			log.WithFields(f).WithError(comErr).Warnf("problem listing commits for repo: %s/%s pull request: %d", owner, repo, pullRequestID)
+			return nil, false, comErr
+		}
+		if resp.StatusCode != http.StatusOK {
+			msg := fmt.Sprintf("unexpected status code: %d - expected: %d", resp.StatusCode, http.StatusOK)
+			log.WithFields(f).Warn(msg)
+			return nil, false, errors.New(msg)
+		}
+
+		log.WithFields(f).Debugf("found %d commits for pull request: %d", len(commits), pullRequestID)
+		for _, commit := range commits {
+			log.WithFields(f).Debugf("loaded commit: %+v", commit)
+			commitAuthor := ""
+			if commit != nil && commit.Commit != nil && commit.Commit.Author != nil && commit.Commit.Author.Login != nil {
+				log.WithFields(f).Debugf("commit.Commit.Author.Login: %s", utils.StringValue(commit.Commit.Author.Login))
+				commitAuthor = utils.StringValue(commit.Commit.Author.Login)
+			} else if commit != nil && commit.Author != nil && commit.Author.Login != nil {
+				log.WithFields(f).Debugf("commit.Author.Login: %s", utils.StringValue(commit.Author.Login))
+				commitAuthor = utils.StringValue(commit.Author.Login)
+			}
+			name, email := "", ""
+			if commit != nil && commit.Commit != nil && commit.Commit.Author != nil {
+				name = strings.TrimSpace(utils.StringValue(commit.Commit.Author.Name))
+				email = strings.TrimSpace(utils.StringValue(commit.Commit.Author.Email))
+				if (name != "" || email != "") && commit.Author == nil {
+					commit.Author = &github.User{}
+				}
+				if name != "" && commit.Author != nil {
+					if commit.Author.Name == nil || strings.TrimSpace(utils.StringValue(commit.Author.Name)) == "" {
+						n := name
+						commit.Author.Name = &n
+					}
+				}
+				if email != "" && commit.Author != nil {
+					if commit.Author.Email == nil || strings.TrimSpace(utils.StringValue(commit.Author.Email)) == "" {
+						e := email
+						commit.Author.Email = &e
+					}
+				}
+			}
+			log.WithFields(f).Debugf("commitAuthor: %s, name: %s, email: %s", commitAuthor, name, email)
+			userCommitSummary = append(userCommitSummary, &UserCommitSummary{
+				SHA:          utils.StringValue(commit.SHA),
+				CommitAuthor: commit.Author,
+				Affiliated:   false,
+				Authorized:   false,
+			})
+			if withCoAuthors {
+				missing := ExpandWithCoAuthors(ctx, client, usersService, commit, pullRequestID, installationID, &userCommitSummary, &mu)
+				if !anyMissing && missing {
+					anyMissing = true
+				}
+			}
+		}
+		if resp.NextPage == 0 {
+			break
+		}
+		opts.Page = resp.NextPage
+	}
+
+	return userCommitSummary, anyMissing, nil
+}
+
 func GetPullRequestCommitAuthors(
 	ctx context.Context,
 	usersService users.Service,
@@ -1302,7 +1386,7 @@ query($owner:String!, $name:String!, $number:Int!, $pageSize:Int!, $cursor:Strin
 		var page prCommitsPage
 		if _, err := doGraphQL(ctx, client, query, vars, &page); err != nil {
 			log.WithFields(f).WithError(err).Warnf("problem listing commits via GraphQL for %s/%s PR #%d", owner, repo, pullRequestID)
-			return nil, false, err
+			return GetPullRequestCommitAuthorsREST(ctx, usersService, installationID, pullRequestID, owner, repo, withCoAuthors)
 		}
 
 		c := page.Repository.PullRequest.Commits

cla-backend/cla/models/github_models.py

@@ -2210,7 +2210,7 @@ def iter_pr_commits_full(g, owner: str, repo_name: str, number: int, page_size:
         )
         if res is None:
             cla.log.error(f"Failed to fetch commits for {owner}/{repo_name} PR #{number}")
-            return
+            raise ValueError("failed to fetch commits using GraphQL")
         commits = res["repository"]["pullRequest"]["commits"]
         for n in commits["nodes"]:
             c = n["commit"]
@@ -2341,29 +2341,61 @@ def get_pull_request_commit_authors(client, org, pull_request, installation_id,
 
     pr_number = pull_request.number
     repo_name = pull_request.base.repo.name
+    gql_ok = True
+    pr_commits = None
+
     count = get_pr_commit_count_gql(client, org, repo_name, pr_number)
     if count is not None:
-        cla.log.debug(f"{fn} - PR: {pr_number}, number of commits: {count}")
+        cla.log.debug(f"{fn} - PR: {pr_number}, number of commits (GraphQL): {count}")
     else:
-        cla.log.debug(f"{fn} - PR: {pr_number}, number of commits: (unknown)")
+        cla.log.debug(f"{fn} - PR: {pr_number}, failed to get commits count using GraphQL, fallback to REST")
+        gql_ok = False
+        try:
+            pr_commits = pull_request.get_commits()
+            count = pr_commits.totalCount
+        except Exception as exc:
+            cla.log.warning(f"{fn} - PR: {pr_number}, get PR commits raised: {exc}")
+            raise
+        cla.log.debug(f"{fn} - PR: {pr_number}, number of commits (REST API): {count}")
+        if count == 250:
+            cla.log.warning(f"{fn} - PR: {pr_number}, commit count is 250, which is the max for REST API, there can be more commits")
 
     commit_authors = []
     any_missing = False
     max_workers = 16
     submit_chunk = 256
 
-    try:
-        with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor:
-            for chunk in iter_chunks(iter_pr_commits_full(client, org, repo_name, pr_number), submit_chunk):
-                futures = [executor.submit(get_author_summary_gql, c, pr_number, installation_id, with_co_authors)
-                           for c in chunk]
+    if gql_ok:
+        try:
+            with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor:
+                for chunk in iter_chunks(iter_pr_commits_full(client, org, repo_name, pr_number), submit_chunk):
+                    futures = [executor.submit(get_author_summary_gql, c, pr_number, installation_id, with_co_authors) for c in chunk]
+                    for fut in concurrent.futures.as_completed(futures):
+                        authors, missing = fut.result()
+                        any_missing = any_missing or missing
+                        commit_authors.extend(authors)
+        except Exception as exc:
+            cla.log.warning(f"{fn} - PR: {pr_number}, GraphQL processing failed: {exc}, falling back to REST")
+            gql_ok = False
+            commit_authors = []
+            any_missing = False
+    if not gql_ok:
+        if pr_commits is None:
+            try:
+                pr_commits = pull_request.get_commits()
+            except Exception as exc:
+                cla.log.warning(f"{fn} - PR: {pr_number}, fallback get PR commits raised: {exc}")
+                raise
+        try:
+            with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor:
+                futures = [executor.submit(get_author_summary, c, pr_number, installation_id, with_co_authors) for c in pr_commits]
                 for fut in concurrent.futures.as_completed(futures):
                     authors, missing = fut.result()
                     any_missing = any_missing or missing
                     commit_authors.extend(authors)
-    except Exception as exc:
-        cla.log.warning(f"{fn} - PR: {pr_number}, get_author_summary_gql raised: {exc}")
-        raise
+        except Exception as exc:
+            cla.log.warning(f"{fn} - PR: {pr_number}, REST processing failed: {exc}")
+            raise
 
     return (commit_authors, any_missing)