Skip to content

Commit 0966349

Browse files
authored
Make the github migration less rate limit waiting to get comment per page from repository but not per issue (#16070)
* Make the github migration less rate limit waiting to get comment per page from repository but not per issue * Fix lint * adjust Downloader interface * Fix missed reviews * Fix test * Remove unused struct
1 parent e8c6cea commit 0966349

13 files changed

+191
-53
lines changed

modules/migrations/base/downloader.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ import (
1111
"code.gitea.io/gitea/modules/structs"
1212
)
1313

14+
// GetCommentOptions represents an options for get comment
15+
type GetCommentOptions struct {
16+
IssueNumber int64
17+
Page int
18+
PageSize int
19+
}
20+
1421
// Downloader downloads the site repo informations
1522
type Downloader interface {
1623
SetContext(context.Context)
@@ -20,7 +27,8 @@ type Downloader interface {
2027
GetReleases() ([]*Release, error)
2128
GetLabels() ([]*Label, error)
2229
GetIssues(page, perPage int) ([]*Issue, bool, error)
23-
GetComments(issueNumber int64) ([]*Comment, error)
30+
GetComments(opts GetCommentOptions) ([]*Comment, bool, error)
31+
SupportGetRepoComments() bool
2432
GetPullRequests(page, perPage int) ([]*PullRequest, bool, error)
2533
GetReviews(pullRequestNumber int64) ([]*Review, error)
2634
FormatCloneURL(opts MigrateOptions, remoteAddr string) (string, error)

modules/migrations/base/null_downloader.go

+7-2
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ func (n NullDownloader) GetIssues(page, perPage int) ([]*Issue, bool, error) {
5151
}
5252

5353
// GetComments returns comments according issueNumber
54-
func (n NullDownloader) GetComments(issueNumber int64) ([]*Comment, error) {
55-
return nil, &ErrNotSupported{Entity: "Comments"}
54+
func (n NullDownloader) GetComments(GetCommentOptions) ([]*Comment, bool, error) {
55+
return nil, false, &ErrNotSupported{Entity: "Comments"}
5656
}
5757

5858
// GetPullRequests returns pull requests according page and perPage
@@ -80,3 +80,8 @@ func (n NullDownloader) FormatCloneURL(opts MigrateOptions, remoteAddr string) (
8080
}
8181
return remoteAddr, nil
8282
}
83+
84+
// SupportGetRepoComments return true if it supports get repo comments
85+
func (n NullDownloader) SupportGetRepoComments() bool {
86+
return false
87+
}

modules/migrations/base/retry_downloader.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -150,18 +150,19 @@ func (d *RetryDownloader) GetIssues(page, perPage int) ([]*Issue, bool, error) {
150150
}
151151

152152
// GetComments returns a repository's comments with retry
153-
func (d *RetryDownloader) GetComments(issueNumber int64) ([]*Comment, error) {
153+
func (d *RetryDownloader) GetComments(opts GetCommentOptions) ([]*Comment, bool, error) {
154154
var (
155155
comments []*Comment
156+
isEnd bool
156157
err error
157158
)
158159

159160
err = d.retry(func() error {
160-
comments, err = d.Downloader.GetComments(issueNumber)
161+
comments, isEnd, err = d.Downloader.GetComments(opts)
161162
return err
162163
})
163164

164-
return comments, err
165+
return comments, isEnd, err
165166
}
166167

167168
// GetPullRequests returns a repository's pull requests with retry

modules/migrations/gitea_downloader.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -435,37 +435,37 @@ func (g *GiteaDownloader) GetIssues(page, perPage int) ([]*base.Issue, bool, err
435435
}
436436

437437
// GetComments returns comments according issueNumber
438-
func (g *GiteaDownloader) GetComments(index int64) ([]*base.Comment, error) {
438+
func (g *GiteaDownloader) GetComments(opts base.GetCommentOptions) ([]*base.Comment, bool, error) {
439439
var allComments = make([]*base.Comment, 0, g.maxPerPage)
440440

441441
// for i := 1; ; i++ {
442442
// make sure gitea can shutdown gracefully
443443
select {
444444
case <-g.ctx.Done():
445-
return nil, nil
445+
return nil, false, nil
446446
default:
447447
}
448448

449-
comments, _, err := g.client.ListIssueComments(g.repoOwner, g.repoName, index, gitea_sdk.ListIssueCommentOptions{ListOptions: gitea_sdk.ListOptions{
449+
comments, _, err := g.client.ListIssueComments(g.repoOwner, g.repoName, opts.IssueNumber, gitea_sdk.ListIssueCommentOptions{ListOptions: gitea_sdk.ListOptions{
450450
// PageSize: g.maxPerPage,
451451
// Page: i,
452452
}})
453453
if err != nil {
454-
return nil, fmt.Errorf("error while listing comments for issue #%d. Error: %v", index, err)
454+
return nil, false, fmt.Errorf("error while listing comments for issue #%d. Error: %v", opts.IssueNumber, err)
455455
}
456456

457457
for _, comment := range comments {
458458
reactions, err := g.getCommentReactions(comment.ID)
459459
if err != nil {
460-
log.Warn("Unable to load comment reactions during migrating issue #%d for comment %d to %s/%s. Error: %v", index, comment.ID, g.repoOwner, g.repoName, err)
460+
log.Warn("Unable to load comment reactions during migrating issue #%d for comment %d to %s/%s. Error: %v", opts.IssueNumber, comment.ID, g.repoOwner, g.repoName, err)
461461
if err2 := models.CreateRepositoryNotice(
462-
fmt.Sprintf("Unable to load reactions during migrating issue #%d for comment %d to %s/%s. Error: %v", index, comment.ID, g.repoOwner, g.repoName, err)); err2 != nil {
462+
fmt.Sprintf("Unable to load reactions during migrating issue #%d for comment %d to %s/%s. Error: %v", opts.IssueNumber, comment.ID, g.repoOwner, g.repoName, err)); err2 != nil {
463463
log.Error("create repository notice failed: ", err2)
464464
}
465465
}
466466

467467
allComments = append(allComments, &base.Comment{
468-
IssueIndex: index,
468+
IssueIndex: opts.IssueNumber,
469469
PosterID: comment.Poster.ID,
470470
PosterName: comment.Poster.UserName,
471471
PosterEmail: comment.Poster.Email,
@@ -481,7 +481,7 @@ func (g *GiteaDownloader) GetComments(index int64) ([]*base.Comment, error) {
481481
// break
482482
// }
483483
//}
484-
return allComments, nil
484+
return allComments, true, nil
485485
}
486486

487487
// GetPullRequests returns pull requests according page and perPage

modules/migrations/gitea_downloader_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,9 @@ func TestGiteaDownloadRepo(t *testing.T) {
224224
Closed: &closed2,
225225
}, issues[1])
226226

227-
comments, err := downloader.GetComments(4)
227+
comments, _, err := downloader.GetComments(base.GetCommentOptions{
228+
IssueNumber: 4,
229+
})
228230
assert.NoError(t, err)
229231
assert.Len(t, comments, 2)
230232
assert.EqualValues(t, 1598975370, comments[0].Created.Unix())

modules/migrations/github.go

+86-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"io"
1212
"net/http"
1313
"net/url"
14+
"strconv"
1415
"strings"
1516
"time"
1617

@@ -450,8 +451,22 @@ func (g *GithubDownloaderV3) GetIssues(page, perPage int) ([]*base.Issue, bool,
450451
return allIssues, len(issues) < perPage, nil
451452
}
452453

454+
// SupportGetRepoComments return true if it supports get repo comments
455+
func (g *GithubDownloaderV3) SupportGetRepoComments() bool {
456+
return true
457+
}
458+
453459
// GetComments returns comments according issueNumber
454-
func (g *GithubDownloaderV3) GetComments(issueNumber int64) ([]*base.Comment, error) {
460+
func (g *GithubDownloaderV3) GetComments(opts base.GetCommentOptions) ([]*base.Comment, bool, error) {
461+
if opts.IssueNumber > 0 {
462+
comments, err := g.getComments(opts.IssueNumber)
463+
return comments, false, err
464+
}
465+
466+
return g.GetAllComments(opts.Page, opts.PageSize)
467+
}
468+
469+
func (g *GithubDownloaderV3) getComments(issueNumber int64) ([]*base.Comment, error) {
455470
var (
456471
allComments = make([]*base.Comment, 0, g.maxPerPage)
457472
created = "created"
@@ -519,6 +534,75 @@ func (g *GithubDownloaderV3) GetComments(issueNumber int64) ([]*base.Comment, er
519534
return allComments, nil
520535
}
521536

537+
// GetAllComments returns repository comments according page and perPageSize
538+
func (g *GithubDownloaderV3) GetAllComments(page, perPage int) ([]*base.Comment, bool, error) {
539+
var (
540+
allComments = make([]*base.Comment, 0, perPage)
541+
created = "created"
542+
asc = "asc"
543+
)
544+
opt := &github.IssueListCommentsOptions{
545+
Sort: &created,
546+
Direction: &asc,
547+
ListOptions: github.ListOptions{
548+
Page: page,
549+
PerPage: perPage,
550+
},
551+
}
552+
553+
g.sleep()
554+
comments, resp, err := g.client.Issues.ListComments(g.ctx, g.repoOwner, g.repoName, 0, opt)
555+
if err != nil {
556+
return nil, false, fmt.Errorf("error while listing repos: %v", err)
557+
}
558+
log.Trace("Request get comments %d/%d, but in fact get %d", perPage, page, len(comments))
559+
g.rate = &resp.Rate
560+
for _, comment := range comments {
561+
var email string
562+
if comment.User.Email != nil {
563+
email = *comment.User.Email
564+
}
565+
566+
// get reactions
567+
var reactions []*base.Reaction
568+
for i := 1; ; i++ {
569+
g.sleep()
570+
res, resp, err := g.client.Reactions.ListIssueCommentReactions(g.ctx, g.repoOwner, g.repoName, comment.GetID(), &github.ListOptions{
571+
Page: i,
572+
PerPage: g.maxPerPage,
573+
})
574+
if err != nil {
575+
return nil, false, err
576+
}
577+
g.rate = &resp.Rate
578+
if len(res) == 0 {
579+
break
580+
}
581+
for _, reaction := range res {
582+
reactions = append(reactions, &base.Reaction{
583+
UserID: reaction.User.GetID(),
584+
UserName: reaction.User.GetLogin(),
585+
Content: reaction.GetContent(),
586+
})
587+
}
588+
}
589+
idx := strings.LastIndex(*comment.IssueURL, "/")
590+
issueIndex, _ := strconv.ParseInt((*comment.IssueURL)[idx+1:], 10, 64)
591+
allComments = append(allComments, &base.Comment{
592+
IssueIndex: issueIndex,
593+
PosterID: *comment.User.ID,
594+
PosterName: *comment.User.Login,
595+
PosterEmail: email,
596+
Content: *comment.Body,
597+
Created: *comment.CreatedAt,
598+
Updated: *comment.UpdatedAt,
599+
Reactions: reactions,
600+
})
601+
}
602+
603+
return allComments, len(allComments) < perPage, nil
604+
}
605+
522606
// GetPullRequests returns pull requests according page and perPage
523607
func (g *GithubDownloaderV3) GetPullRequests(page, perPage int) ([]*base.PullRequest, bool, error) {
524608
if perPage > g.maxPerPage {
@@ -539,6 +623,7 @@ func (g *GithubDownloaderV3) GetPullRequests(page, perPage int) ([]*base.PullReq
539623
if err != nil {
540624
return nil, false, fmt.Errorf("error while listing repos: %v", err)
541625
}
626+
log.Trace("Request get pull requests %d/%d, but in fact get %d", perPage, page, len(prs))
542627
g.rate = &resp.Rate
543628
for _, pr := range prs {
544629
var body string

modules/migrations/github_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,9 @@ func TestGitHubDownloadRepo(t *testing.T) {
240240
}, issues)
241241

242242
// downloader.GetComments()
243-
comments, err := downloader.GetComments(2)
243+
comments, _, err := downloader.GetComments(base.GetCommentOptions{
244+
IssueNumber: 2,
245+
})
244246
assert.NoError(t, err)
245247
assert.Len(t, comments, 2)
246248
assert.EqualValues(t, []*base.Comment{

modules/migrations/gitlab.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,8 @@ func (g *GitlabDownloader) GetIssues(page, perPage int) ([]*base.Issue, bool, er
430430

431431
// GetComments returns comments according issueNumber
432432
// TODO: figure out how to transfer comment reactions
433-
func (g *GitlabDownloader) GetComments(issueNumber int64) ([]*base.Comment, error) {
433+
func (g *GitlabDownloader) GetComments(opts base.GetCommentOptions) ([]*base.Comment, bool, error) {
434+
var issueNumber = opts.IssueNumber
434435
var allComments = make([]*base.Comment, 0, g.maxPerPage)
435436

436437
var page = 1
@@ -457,7 +458,7 @@ func (g *GitlabDownloader) GetComments(issueNumber int64) ([]*base.Comment, erro
457458
}
458459

459460
if err != nil {
460-
return nil, fmt.Errorf("error while listing comments: %v %v", g.repoID, err)
461+
return nil, false, fmt.Errorf("error while listing comments: %v %v", g.repoID, err)
461462
}
462463
for _, comment := range comments {
463464
// Flatten comment threads
@@ -490,7 +491,7 @@ func (g *GitlabDownloader) GetComments(issueNumber int64) ([]*base.Comment, erro
490491
}
491492
page = resp.NextPage
492493
}
493-
return allComments, nil
494+
return allComments, true, nil
494495
}
495496

496497
// GetPullRequests returns pull requests according page and perPage

modules/migrations/gitlab_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,9 @@ func TestGitlabDownloadRepo(t *testing.T) {
204204
},
205205
}, issues)
206206

207-
comments, err := downloader.GetComments(2)
207+
comments, _, err := downloader.GetComments(base.GetCommentOptions{
208+
IssueNumber: 2,
209+
})
208210
assert.NoError(t, err)
209211
assert.Len(t, comments, 4)
210212
assert.EqualValues(t, []*base.Comment{

modules/migrations/gogs.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,13 @@ func (g *GogsDownloader) getIssues(page int, state string) ([]*base.Issue, bool,
227227
}
228228

229229
// GetComments returns comments according issueNumber
230-
func (g *GogsDownloader) GetComments(issueNumber int64) ([]*base.Comment, error) {
230+
func (g *GogsDownloader) GetComments(opts base.GetCommentOptions) ([]*base.Comment, bool, error) {
231+
var issueNumber = opts.IssueNumber
231232
var allComments = make([]*base.Comment, 0, 100)
232233

233234
comments, err := g.client.ListIssueComments(g.repoOwner, g.repoName, issueNumber)
234235
if err != nil {
235-
return nil, fmt.Errorf("error while listing repos: %v", err)
236+
return nil, false, fmt.Errorf("error while listing repos: %v", err)
236237
}
237238
for _, comment := range comments {
238239
if len(comment.Body) == 0 || comment.Poster == nil {
@@ -249,7 +250,7 @@ func (g *GogsDownloader) GetComments(issueNumber int64) ([]*base.Comment, error)
249250
})
250251
}
251252

252-
return allComments, nil
253+
return allComments, true, nil
253254
}
254255

255256
// GetTopics return repository topics

modules/migrations/gogs_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ func TestGogsDownloadRepo(t *testing.T) {
103103
}, issues)
104104

105105
// downloader.GetComments()
106-
comments, err := downloader.GetComments(1)
106+
comments, _, err := downloader.GetComments(base.GetCommentOptions{
107+
IssueNumber: 1,
108+
})
107109
assert.NoError(t, err)
108110
assert.Len(t, comments, 1)
109111
assert.EqualValues(t, []*base.Comment{

0 commit comments

Comments
 (0)