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

Fix time to NotifyPullRequestSynchronized #22650

Merged
merged 25 commits into from
Feb 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c25a16e
fix: PushToBaseRepo first
wolfogre Jan 29, 2023
fc94e9f
chore: debug CI
wolfogre Jan 30, 2023
9f2f3df
chore: debug CI
wolfogre Jan 30, 2023
a64347e
Revert "fix: PushToBaseRepo first"
wolfogre Jan 30, 2023
127ec2a
chore: add debug log
wolfogre Jan 30, 2023
faae1b2
fix: PushToBaseRepo first
wolfogre Jan 29, 2023
919d953
chore: debug CI
wolfogre Jan 30, 2023
9d09267
chore: debug CI
wolfogre Jan 31, 2023
36a60cb
chore: remove debug code
wolfogre Jan 31, 2023
e2ac98c
test: wait for async operations
wolfogre Jan 31, 2023
ba8f5f8
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Jan 31, 2023
6e7238d
test: wait longer
wolfogre Jan 31, 2023
da8854e
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Jan 31, 2023
6f84419
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Jan 31, 2023
c6775ce
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Jan 31, 2023
3967830
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Feb 1, 2023
37f7408
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Feb 1, 2023
f6751cf
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Feb 1, 2023
a84636e
Merge branch 'main' into bugfix/notify_pr_sync
lunny Feb 1, 2023
cdd8381
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Feb 3, 2023
bbbdc38
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Feb 3, 2023
259e55e
fix: return error if issues and prs may be not in sync
wolfogre Feb 5, 2023
40366dd
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Feb 5, 2023
d41195c
Merge remote-tracking branch 'origin/bugfix/notify_pr_sync' into bugf…
wolfogre Feb 5, 2023
9256a2c
Merge branch 'main' into bugfix/notify_pr_sync
wolfogre Feb 5, 2023
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
13 changes: 12 additions & 1 deletion models/issues/pull_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/util"

"xorm.io/xorm"
)
Expand Down Expand Up @@ -175,7 +176,17 @@ func (prs PullRequestList) loadAttributes(ctx context.Context) error {
}
for _, pr := range prs {
pr.Issue = set[pr.IssueID]
pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync
/*
Old code:
pr.Issue.PullRequest = pr // panic here means issueIDs and prs are not in sync

It's worth panic because it's almost impossible to happen under normal use.
But in integration testing, an asynchronous task could read a database that has been reset.
So returning an error would make more sense, let the caller has a choice to ignore it.
*/
if pr.Issue == nil {
return fmt.Errorf("issues and prs may be not in sync: cannot find issue %v for pr %v: %w", pr.IssueID, pr.ID, util.ErrNotExist)
}
}
return nil
}
Expand Down
36 changes: 18 additions & 18 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,24 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
return
}

for _, pr := range prs {
log.Trace("Updating PR[%d]: composing new test task", pr.ID)
if pr.Flow == issues_model.PullRequestFlowGithub {
if err := PushToBaseRepo(ctx, pr); err != nil {
log.Error("PushToBaseRepo: %v", err)
continue
}
} else {
continue
}

AddToTaskQueue(pr)
comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID)
if err == nil && comment != nil {
notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment)
}
}

if isSync {
requests := issues_model.PullRequestList(prs)
if err = requests.LoadAttributes(); err != nil {
Expand Down Expand Up @@ -303,24 +321,6 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string,
}
}

for _, pr := range prs {
log.Trace("Updating PR[%d]: composing new test task", pr.ID)
if pr.Flow == issues_model.PullRequestFlowGithub {
if err := PushToBaseRepo(ctx, pr); err != nil {
log.Error("PushToBaseRepo: %v", err)
continue
}
} else {
continue
}

AddToTaskQueue(pr)
comment, err := CreatePushPullComment(ctx, doer, pr, oldCommitID, newCommitID)
if err == nil && comment != nil {
notification.NotifyPullRequestPushCommits(ctx, doer, pr, comment)
}
}

log.Trace("AddTestPullRequestTask [base_repo_id: %d, base_branch: %s]: finding pull requests", repoID, branch)
prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(repoID, branch)
if err != nil {
Expand Down