From 01c342aa35a765f44cb9304843a003e4636138e9 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 14:03:03 +0200 Subject: [PATCH 01/18] allow repos to be incrementally fetched and updated --- dev/scaletesting/bulkreposettings/main.go | 243 +++++++++++++++++----- 1 file changed, 196 insertions(+), 47 deletions(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index a23915fd486d..327f04c0a32b 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -13,7 +13,9 @@ import ( "golang.org/x/oauth2" "github.com/sourcegraph/log" + "github.com/sourcegraph/sourcegraph/dev/scaletesting/internal/store" + "github.com/sourcegraph/sourcegraph/lib/errors" "github.com/sourcegraph/sourcegraph/lib/group" "github.com/sourcegraph/sourcegraph/lib/output" ) @@ -86,56 +88,95 @@ var app = &cli.App{ return err } + var repoIter Iter[[]*store.Repo] + var total int64 if len(repos) == 0 { - logger.Info("No existing state found, creating ...") - repos, err = fetchRepos(cmd.Context, org, gh) + logger.Info("Using GithubRepoFetcher") + repoIter = &GithubRepoFetcher{ + client: gh, + repoType: "public", // we're only interested in public repos to change visibility + org: org, + pageStart: 0, + done: false, + err: nil, + } + + t, err := getTotalPublicRepos(ctx, gh, org) + logger.Info("Estimated public repos from API", log.Int("total", t)) if err != nil { - logger.Error("failed to fetch repositories from org", log.Error(err), log.String("github.org", org)) - return err + logger.Fatal("failed to get total public repos size for org", log.String("org", org), log.Error(err)) } - if err := s.Insert(repos); err != nil { - logger.Error("failed to insert repositories from org", log.Error(err), log.String("github.org", org)) - return err + total = int64(t) + } else { + logger.Info("Using StaticRepoFecther") + repoIter = &StaticRepoFetcher{ + repos: repos, + iterSize: 10, + start: 0, } + total = int64(len(repos)) } out := output.NewOutput(os.Stdout, output.OutputOpts{}) bars := []output.ProgressBar{ - {Label: "Updating repos", Max: float64(len(repos))}, + {Label: "Updating repos", Max: float64(total)}, } progress := out.Progress(bars, nil) defer progress.Destroy() var done int64 - total := len(repos) g := group.NewWithResults[error]().WithMaxConcurrency(20) - for _, r := range repos { - r := r - g.Go(func() error { - if r.Pushed { - return nil - } - var err error - settings := &github.Repository{Private: github.Bool(true)} - for i := 0; i < cmd.Int("retry"); i++ { - _, _, err = gh.Repositories.Edit(cmd.Context, org, r.Name, settings) - if err != nil { - r.Failed = err.Error() - } else { - r.Failed = "" - r.Pushed = true - break - } - } + for !repoIter.Done() && repoIter.Err() == nil { + + for _, r := range repoIter.Next(ctx) { + r := r if err := s.SaveRepo(r); err != nil { logger.Fatal("could not save repo", log.Error(err), log.String("repo", r.Name)) } - atomic.AddInt64(&done, 1) - progress.SetValue(0, float64(done)) - progress.SetLabel(0, fmt.Sprintf("Updating repos (%d/%d)", done, total)) - return err - }) + + g.Go(func() error { + if r.Pushed { + return nil + } + var err error + settings := &github.Repository{Private: github.Bool(true)} + for i := 0; i < cmd.Int("retry"); i++ { + _, _, err = gh.Repositories.Edit(cmd.Context, org, r.Name, settings) + if err != nil { + r.Failed = err.Error() + } else { + r.Failed = "" + r.Pushed = true + break + } + } + if err := s.SaveRepo(r); err != nil { + logger.Fatal("could not save repo", log.Error(err), log.String("repo", r.Name)) + } + atomic.AddInt64(&done, 1) + progress.SetValue(0, float64(done)) + progress.SetLabel(0, fmt.Sprintf("%d/+-%d ", done, total)) + return err + }) + } + // The total we get from Github is not correct (ie. 50k when we know the org as 200k) + // So when done reaches the total, we attempt to get the total again and double the Max + // of the bar + if atomic.LoadInt64(&done) == int64(total) { + t, err := getTotalPublicRepos(ctx, gh, org) + if err != nil { + logger.Fatal("failed to get updated public repos count", log.Error(err)) + } + atomic.AddInt64(&total, int64(t)) + bars[0].Max = bars[0].Max + float64(total) + + } + } + + errs := g.Wait() + if len(errs) > 0 { + return errs[0] } return nil }, @@ -145,22 +186,117 @@ var app = &cli.App{ }, } -func fetchRepos(ctx context.Context, org string, gh *github.Client) ([]*store.Repo, error) { +type Iter[T any] interface { + Err() error + Next(ctx context.Context) T + Done() bool +} + +var _ Iter[[]*store.Repo] = (*GithubRepoFetcher)(nil) + +// StaticRepoFetcher satisfies the Iter interface allowing one to iterate over a static array of repos. To change +// how many repos are returned per invocation of next, set iterSize (default 10). To start iterating at a different +// index, set start to a different value. +// +// The iteration is considered done when start >= len(repos) +type StaticRepoFetcher struct { + repos []*store.Repo + iterSize int + start int +} + +// Err returns the last error (if any) encountered by Iter. For StaticRepoFetcher, this retuns nil always +func (s *StaticRepoFetcher) Err() error { + return nil +} + +// Done determines whether this Iter can produce more items. When start >= length of repos, then this will return true +func (s *StaticRepoFetcher) Done() bool { + return s.start >= len(s.repos) +} + +// Next returns the next set of Repos. The amount of repos returned is determined by iterSize. When Done() is true, +// nil is returned. +func (s *StaticRepoFetcher) Next(_ context.Context) []*store.Repo { + if s.iterSize == 0 { + s.iterSize = 10 + } + if s.Done() { + return nil + } + if s.start+s.iterSize > len(s.repos) { + + s.start = len(s.repos) + return s.repos[s.start:] + } + + results := s.repos[s.start : s.start+s.iterSize] + // advance the start index + s.start += s.iterSize + return results + +} + +type GithubRepoFetcher struct { + client *github.Client + repoType string + org string + pageStart int + pageSize int + done bool + err error +} + +// Done determines whether more repos can be retrieved from Github +func (f *GithubRepoFetcher) Done() bool { + return f.done +} + +// Err returns the last error encountered by Iter +func (f *GithubRepoFetcher) Err() error { + return f.err +} + +// Next retrieves the next set of repos by contact Github. The amount of repos fetched is determined by pageSize. +// The next page start is automatically advanced based on the response received from Github. When the next page response +// from Github is 0, it means there are no more repos to fetch and this Iter is done, thus done is then set to true and +// Done() will also return true. +// +// If any error is encountered during retrieval of Repos the err value will be set and can be retrieved with Err() +func (f *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { + if f.done { + return nil + } + + results, next, err := f.listRepos(ctx, f.org, f.pageStart, f.pageSize) + if err != nil { + f.err = err + return nil + } + + hasMore := next != 0 + if !hasMore { + f.done = true + } + + f.pageStart = next + + return results +} + +func (f *GithubRepoFetcher) listRepos(ctx context.Context, org string, start int, size int) ([]*store.Repo, int, error) { opts := github.RepositoryListByOrgOptions{ - ListOptions: github.ListOptions{}, + Type: f.repoType, + ListOptions: github.ListOptions{Page: start, PerPage: size}, + } + + repos, resp, err := f.client.Repositories.ListByOrg(ctx, org, &opts) + if err != nil { + return nil, 0, err } - var repos []*github.Repository - for { - rs, resp, err := gh.Repositories.ListByOrg(ctx, org, &opts) - if err != nil { - return nil, err - } - repos = append(repos, rs...) - - if resp.NextPage == 0 { - break - } - opts.ListOptions.Page = resp.NextPage + + if resp.StatusCode >= 300 { + return nil, 0, errors.Newf("failed to list repos for org %s. Got status %d code", org, resp.StatusCode) } res := make([]*store.Repo, 0, len(repos)) @@ -171,7 +307,20 @@ func fetchRepos(ctx context.Context, org string, gh *github.Client) ([]*store.Re }) } - return res, nil + return res, resp.NextPage, nil +} + +func getTotalPublicRepos(ctx context.Context, client *github.Client, org string) (int, error) { + orgRes, resp, err := client.Organizations.Get(ctx, org) + if err != nil { + return 0, err + } + + if resp.StatusCode >= 300 { + return 0, errors.Newf("failed to get org %s. Got status %d code", org, resp.StatusCode) + } + + return *orgRes.PublicRepos, nil } func main() { From e1afc5e602221128d83c79710239ece99b128a58 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 14:08:59 +0200 Subject: [PATCH 02/18] Update dev/scaletesting/bulkreposettings/main.go --- dev/scaletesting/bulkreposettings/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 327f04c0a32b..2137ed6db1f7 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -274,6 +274,7 @@ func (f *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { return nil } + // when next is 0, it means the Github api returned the nextPage as 0, which indicates that there are not more pages to fetch hasMore := next != 0 if !hasMore { f.done = true From 7a820863d3bf28387a08fcd027380c1beea7ec66 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 14:09:45 +0200 Subject: [PATCH 03/18] Update dev/scaletesting/bulkreposettings/main.go --- dev/scaletesting/bulkreposettings/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 2137ed6db1f7..1d17191e13db 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -279,7 +279,7 @@ func (f *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { if !hasMore { f.done = true } - +// Ensure that the next request starts at the next page f.pageStart = next return results From 3f3a247c4e2591f5ab2e6464e23aef05fa9c3004 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 15:41:52 +0200 Subject: [PATCH 04/18] use pending context to convey repo update status --- dev/scaletesting/bulkreposettings/main.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 1d17191e13db..28556c21a99e 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -118,11 +118,8 @@ var app = &cli.App{ } out := output.NewOutput(os.Stdout, output.OutputOpts{}) - bars := []output.ProgressBar{ - {Label: "Updating repos", Max: float64(total)}, - } - progress := out.Progress(bars, nil) - defer progress.Destroy() + pending := out.Pending(output.Line(output.EmojiHourglass, output.StylePending, "Updating repos")) + defer pending.Destroy() var done int64 @@ -151,12 +148,12 @@ var app = &cli.App{ break } } + if err := s.SaveRepo(r); err != nil { logger.Fatal("could not save repo", log.Error(err), log.String("repo", r.Name)) } atomic.AddInt64(&done, 1) - progress.SetValue(0, float64(done)) - progress.SetLabel(0, fmt.Sprintf("%d/+-%d ", done, total)) + pending.Update(fmt.Sprintf("%d repos updated (estimated total: %d)", done, total)) return err }) } @@ -169,12 +166,11 @@ var app = &cli.App{ logger.Fatal("failed to get updated public repos count", log.Error(err)) } atomic.AddInt64(&total, int64(t)) - bars[0].Max = bars[0].Max + float64(total) - + pending.Update(fmt.Sprintf("%d repos updated (estimated total: %d)", done, total)) } } - errs := g.Wait() + pending.Complete(output.Line(output.EmojiOk, output.StyleBold, fmt.Sprintf("%d repos updated", done))) if len(errs) > 0 { return errs[0] } From 8b807694ae0d59f3e850024497b7a56e8af2e5a5 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 16:11:18 +0200 Subject: [PATCH 05/18] check errors and print first 5 --- dev/scaletesting/bulkreposettings/main.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 28556c21a99e..54ce6cdca0e8 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -169,11 +169,17 @@ var app = &cli.App{ pending.Update(fmt.Sprintf("%d repos updated (estimated total: %d)", done, total)) } } + errs := g.Wait() - pending.Complete(output.Line(output.EmojiOk, output.StyleBold, fmt.Sprintf("%d repos updated", done))) if len(errs) > 0 { + pending.Complete(output.Line(output.EmojiFailure, output.StyleBold, fmt.Sprintf("%d errors occured while updating repos", len(errs)))) + out.Writef("Printing first 5 errros") + for i := 0; i < len(errs) && i < 5; i++ { + logger.Error("Error updating repo", log.Error(errs[i])) + } return errs[0] } + pending.Complete(output.Line(output.EmojiOk, output.StyleBold, fmt.Sprintf("%d repos updated", done))) return nil }, }, @@ -270,12 +276,12 @@ func (f *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { return nil } - // when next is 0, it means the Github api returned the nextPage as 0, which indicates that there are not more pages to fetch + // when next is 0, it means the Github api returned the nextPage as 0, which indicates that there are not more pages to fetch hasMore := next != 0 if !hasMore { f.done = true } -// Ensure that the next request starts at the next page + // Ensure that the next request starts at the next page f.pageStart = next return results From 416fc8578f892b5624fa483a12d15f9965674423 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 16:24:54 +0200 Subject: [PATCH 06/18] move log line --- dev/scaletesting/bulkreposettings/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 54ce6cdca0e8..ba48766de1ce 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -102,10 +102,10 @@ var app = &cli.App{ } t, err := getTotalPublicRepos(ctx, gh, org) - logger.Info("Estimated public repos from API", log.Int("total", t)) if err != nil { logger.Fatal("failed to get total public repos size for org", log.String("org", org), log.Error(err)) } + logger.Info("Estimated public repos from API", log.Int("total", t)) total = int64(t) } else { logger.Info("Using StaticRepoFecther") From 83e6ffb1241d69f774cb491878c6edacd1415b87 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 21:30:27 +0200 Subject: [PATCH 07/18] check result errrors --- dev/scaletesting/bulkreposettings/main.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index ba48766de1ce..e1177b76ccb2 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -170,7 +170,16 @@ var app = &cli.App{ } } - errs := g.Wait() + results := g.Wait() + + // Check that we actually got errors + errs := make([]error, 0) + for _, r := range results { + if r != nil { + errs = append(errs, r) + } + } + if len(errs) > 0 { pending.Complete(output.Line(output.EmojiFailure, output.StyleBold, fmt.Sprintf("%d errors occured while updating repos", len(errs)))) out.Writef("Printing first 5 errros") From b23ccb9b2590e26fae498ba599515d178d981e48 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 21:46:30 +0200 Subject: [PATCH 08/18] move around next page logic --- dev/scaletesting/bulkreposettings/main.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index e1177b76ccb2..fafb07bd8c66 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -286,12 +286,13 @@ func (f *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { } // when next is 0, it means the Github api returned the nextPage as 0, which indicates that there are not more pages to fetch - hasMore := next != 0 - if !hasMore { + hasMore := next > 0 + if hasMore { + // Ensure that the next request starts at the next page + f.pageStart = next + } else { f.done = true } - // Ensure that the next request starts at the next page - f.pageStart = next return results } From 8c7a2e8f1cc9ab33104b7da781fd7e307fae1e9b Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 21:51:21 +0200 Subject: [PATCH 09/18] add debug to github iter --- dev/scaletesting/bulkreposettings/main.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index fafb07bd8c66..afd9f32ae46f 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -320,6 +320,12 @@ func (f *GithubRepoFetcher) listRepos(ctx context.Context, org string, start int }) } + if resp.NextPage == 0 { + println("-------") + println(resp.LastPage) + println(resp.NextPage) + println("-------") + } return res, resp.NextPage, nil } From 3ca01f885b2951820c7db16b66948b5c55278d44 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 21:53:59 +0200 Subject: [PATCH 10/18] print previous page num too --- dev/scaletesting/bulkreposettings/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index afd9f32ae46f..5828202cde77 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -324,6 +324,7 @@ func (f *GithubRepoFetcher) listRepos(ctx context.Context, org string, start int println("-------") println(resp.LastPage) println(resp.NextPage) + println(resp.PrevPage) println("-------") } return res, resp.NextPage, nil From 057c6f60d8727afe46b774bb52ce888a77de9c81 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 21:58:32 +0200 Subject: [PATCH 11/18] print repo iter error --- dev/scaletesting/bulkreposettings/main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 5828202cde77..8ed5bae5231b 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -170,6 +170,10 @@ var app = &cli.App{ } } + if err := repoIter.Err(); err != nil { + logger.Error("repo iterator encountered an error", log.Error(err)) + } + results := g.Wait() // Check that we actually got errors From 542d23ee620071195ab3fff00b64d1f5aef306c7 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 10 Nov 2022 22:33:32 +0200 Subject: [PATCH 12/18] attempt bug fix: only completes half of the repos --- dev/scaletesting/bulkreposettings/main.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 8ed5bae5231b..9a38f1811c7b 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -240,9 +240,9 @@ func (s *StaticRepoFetcher) Next(_ context.Context) []*store.Repo { return nil } if s.start+s.iterSize > len(s.repos) { - + results := s.repos[s.start:] s.start = len(s.repos) - return s.repos[s.start:] + return results } results := s.repos[s.start : s.start+s.iterSize] @@ -290,8 +290,7 @@ func (f *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { } // when next is 0, it means the Github api returned the nextPage as 0, which indicates that there are not more pages to fetch - hasMore := next > 0 - if hasMore { + if next > 0 { // Ensure that the next request starts at the next page f.pageStart = next } else { @@ -324,13 +323,13 @@ func (f *GithubRepoFetcher) listRepos(ctx context.Context, org string, start int }) } - if resp.NextPage == 0 { - println("-------") - println(resp.LastPage) - println(resp.NextPage) - println(resp.PrevPage) - println("-------") + next := resp.NextPage + if next == 0 { + if f.pageStart != resp.LastPage { + next = resp.LastPage + } } + return res, resp.NextPage, nil } From 48b3c886d347b75ed377c375d370794fc05984e0 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Tue, 15 Nov 2022 08:55:56 +0000 Subject: [PATCH 13/18] Update dev/scaletesting/bulkreposettings/main.go Co-authored-by: Jean-Hadrien Chabran --- dev/scaletesting/bulkreposettings/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 9a38f1811c7b..ffa9b6d24bd7 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -125,7 +125,6 @@ var app = &cli.App{ g := group.NewWithResults[error]().WithMaxConcurrency(20) for !repoIter.Done() && repoIter.Err() == nil { - for _, r := range repoIter.Next(ctx) { r := r if err := s.SaveRepo(r); err != nil { From 92b4552479c4566b553d05541f2ff69674df065f Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Tue, 15 Nov 2022 08:56:02 +0000 Subject: [PATCH 14/18] Update dev/scaletesting/bulkreposettings/main.go Co-authored-by: Jean-Hadrien Chabran --- dev/scaletesting/bulkreposettings/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index ffa9b6d24bd7..2772d5ecdaa8 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -261,7 +261,7 @@ type GithubRepoFetcher struct { err error } -// Done determines whether more repos can be retrieved from Github +// Done determines whether more repos can be retrieved from Github. func (f *GithubRepoFetcher) Done() bool { return f.done } From 3648d5f3f993de09254b90c245c40cb162d2598a Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Tue, 15 Nov 2022 08:56:18 +0000 Subject: [PATCH 15/18] Update dev/scaletesting/bulkreposettings/main.go Co-authored-by: Jean-Hadrien Chabran --- dev/scaletesting/bulkreposettings/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 2772d5ecdaa8..3cd165cce0ec 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -255,8 +255,8 @@ type GithubRepoFetcher struct { client *github.Client repoType string org string - pageStart int - pageSize int + page int + perPage int done bool err error } From 0ff7293f68452ceecd22312d763c15d437fc6671 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Tue, 15 Nov 2022 08:56:40 +0000 Subject: [PATCH 16/18] Update dev/scaletesting/bulkreposettings/main.go Co-authored-by: Jean-Hadrien Chabran --- dev/scaletesting/bulkreposettings/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 3cd165cce0ec..17f7b1cdef72 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -213,7 +213,7 @@ var _ Iter[[]*store.Repo] = (*GithubRepoFetcher)(nil) // index, set start to a different value. // // The iteration is considered done when start >= len(repos) -type StaticRepoFetcher struct { +type MockRepoFetcher struct { repos []*store.Repo iterSize int start int From faeff398812025324f2fc1551bb921612d287d30 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 17 Nov 2022 13:43:51 +0200 Subject: [PATCH 17/18] Update dev/scaletesting/bulkreposettings/main.go Co-authored-by: Jean-Hadrien Chabran --- dev/scaletesting/bulkreposettings/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 17f7b1cdef72..3d824601a5e1 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -176,7 +176,7 @@ var app = &cli.App{ results := g.Wait() // Check that we actually got errors - errs := make([]error, 0) + errs := []error{} for _, r := range results { if r != nil { errs = append(errs, r) From 15c1c87416d5608f09fa7d4c44c61a1b7220cc3f Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Thu, 17 Nov 2022 13:58:24 +0200 Subject: [PATCH 18/18] review comments --- dev/scaletesting/bulkreposettings/main.go | 87 +++++++++++------------ 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/dev/scaletesting/bulkreposettings/main.go b/dev/scaletesting/bulkreposettings/main.go index 3d824601a5e1..fb08aacd5c18 100644 --- a/dev/scaletesting/bulkreposettings/main.go +++ b/dev/scaletesting/bulkreposettings/main.go @@ -93,12 +93,12 @@ var app = &cli.App{ if len(repos) == 0 { logger.Info("Using GithubRepoFetcher") repoIter = &GithubRepoFetcher{ - client: gh, - repoType: "public", // we're only interested in public repos to change visibility - org: org, - pageStart: 0, - done: false, - err: nil, + client: gh, + repoType: "public", // we're only interested in public repos to change visibility + org: org, + page: 0, + done: false, + err: nil, } t, err := getTotalPublicRepos(ctx, gh, org) @@ -109,7 +109,7 @@ var app = &cli.App{ total = int64(t) } else { logger.Info("Using StaticRepoFecther") - repoIter = &StaticRepoFetcher{ + repoIter = &MockRepoFetcher{ repos: repos, iterSize: 10, start: 0, @@ -219,56 +219,56 @@ type MockRepoFetcher struct { start int } -// Err returns the last error (if any) encountered by Iter. For StaticRepoFetcher, this retuns nil always -func (s *StaticRepoFetcher) Err() error { +// Err returns the last error (if any) encountered by Iter. For MockRepoFetcher, this retuns nil always +func (m *MockRepoFetcher) Err() error { return nil } // Done determines whether this Iter can produce more items. When start >= length of repos, then this will return true -func (s *StaticRepoFetcher) Done() bool { - return s.start >= len(s.repos) +func (m *MockRepoFetcher) Done() bool { + return m.start >= len(m.repos) } // Next returns the next set of Repos. The amount of repos returned is determined by iterSize. When Done() is true, // nil is returned. -func (s *StaticRepoFetcher) Next(_ context.Context) []*store.Repo { - if s.iterSize == 0 { - s.iterSize = 10 +func (m *MockRepoFetcher) Next(_ context.Context) []*store.Repo { + if m.iterSize == 0 { + m.iterSize = 10 } - if s.Done() { + if m.Done() { return nil } - if s.start+s.iterSize > len(s.repos) { - results := s.repos[s.start:] - s.start = len(s.repos) + if m.start+m.iterSize > len(m.repos) { + results := m.repos[m.start:] + m.start = len(m.repos) return results } - results := s.repos[s.start : s.start+s.iterSize] + results := m.repos[m.start : m.start+m.iterSize] // advance the start index - s.start += s.iterSize + m.start += m.iterSize return results } type GithubRepoFetcher struct { - client *github.Client - repoType string - org string - page int + client *github.Client + repoType string + org string + page int perPage int - done bool - err error + done bool + err error } // Done determines whether more repos can be retrieved from Github. -func (f *GithubRepoFetcher) Done() bool { - return f.done +func (g *GithubRepoFetcher) Done() bool { + return g.done } // Err returns the last error encountered by Iter -func (f *GithubRepoFetcher) Err() error { - return f.err +func (g *GithubRepoFetcher) Err() error { + return g.err } // Next retrieves the next set of repos by contact Github. The amount of repos fetched is determined by pageSize. @@ -277,35 +277,35 @@ func (f *GithubRepoFetcher) Err() error { // Done() will also return true. // // If any error is encountered during retrieval of Repos the err value will be set and can be retrieved with Err() -func (f *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { - if f.done { +func (g *GithubRepoFetcher) Next(ctx context.Context) []*store.Repo { + if g.done { return nil } - results, next, err := f.listRepos(ctx, f.org, f.pageStart, f.pageSize) + results, next, err := g.listRepos(ctx, g.org, g.page, g.perPage) if err != nil { - f.err = err + g.err = err return nil } // when next is 0, it means the Github api returned the nextPage as 0, which indicates that there are not more pages to fetch if next > 0 { // Ensure that the next request starts at the next page - f.pageStart = next + g.page = next } else { - f.done = true + g.done = true } return results } -func (f *GithubRepoFetcher) listRepos(ctx context.Context, org string, start int, size int) ([]*store.Repo, int, error) { +func (g *GithubRepoFetcher) listRepos(ctx context.Context, org string, start int, size int) ([]*store.Repo, int, error) { opts := github.RepositoryListByOrgOptions{ - Type: f.repoType, + Type: g.repoType, ListOptions: github.ListOptions{Page: start, PerPage: size}, } - repos, resp, err := f.client.Repositories.ListByOrg(ctx, org, &opts) + repos, resp, err := g.client.Repositories.ListByOrg(ctx, org, &opts) if err != nil { return nil, 0, err } @@ -323,13 +323,12 @@ func (f *GithubRepoFetcher) listRepos(ctx context.Context, org string, start int } next := resp.NextPage - if next == 0 { - if f.pageStart != resp.LastPage { - next = resp.LastPage - } + // If next page is 0 we're at the last page, so set the last page + if next == 0 && g.page != resp.LastPage { + next = resp.LastPage } - return res, resp.NextPage, nil + return res, next, nil } func getTotalPublicRepos(ctx context.Context, client *github.Client, org string) (int, error) {