Skip to content

Unicron implemente rest fallback when graphql fails #4827

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

Merged
lukaszgryglicki merged 3 commits into from
Oct 16, 2025
Merged

Unicron implemente rest fallback when graphql fails #4827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9317c64
REST fallback in python code
lukaszgryglicki Oct 16, 2025
a6c6063
REST fallback in golang code
lukaszgryglicki Oct 16, 2025
ef8afdf
Address AI feedback
lukaszgryglicki Oct 16, 2025
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
86 changes: 85 additions & 1 deletion cla-backend-go/github/github_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
54 changes: 43 additions & 11 deletions cla-backend/cla/models/github_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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)

Expand Down
Loading