retry change detection #598
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| make/repo.Makefile |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,14 +4,15 @@ import ( | |
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "github.com/avast/retry-go" | ||
| "github.com/google/go-github/v41/github" | ||
| "github.com/synapsecns/sanguine/contrib/git-changes-action/detector/actionscore" | ||
| "github.com/synapsecns/sanguine/contrib/git-changes-action/detector/tree" | ||
| "golang.org/x/exp/slices" | ||
| "golang.org/x/oauth2" | ||
| "os" | ||
| "strings" | ||
| "time" | ||
| ) | ||
|
|
||
| // GetChangeTree returns the ref for the given event name. | ||
|
|
@@ -64,15 +65,25 @@ func getChangedFilesFromAPI(ctx context.Context, ghContext *actionscore.Context, | |
| ct = tree.NewTree() | ||
|
|
||
| page := 1 | ||
| const retryCount = 10 | ||
| for { | ||
| var files []*github.CommitFile | ||
| var res *github.Response | ||
| err = retry.Do(func() error { | ||
| reqCtx, cancel := context.WithTimeout(ctx, 15*time.Second) | ||
| defer cancel() | ||
|
|
||
| files, res, err = client.PullRequests.ListFiles(reqCtx, repoOwner, repoName, prNumber, &github.ListOptions{ | ||
| Page: page, | ||
| PerPage: 100, | ||
|
There was a problem hiding this comment. In master, I now changed this to PerPage: 20 and it worked after not working for a long time. Not sure if I just ended up getting lucky with that, but maybe just use 20 since that worked for me? There was a problem hiding this comment. so max is 100, 20 will cause rate limit hits during high commit times There was a problem hiding this comment. Yeah, just slacked you before seeing this. |
||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("could not get files: %w", err) | ||
| } | ||
| return nil | ||
| }, retry.Context(ctx), retry.Attempts(retryCount)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("could not get files after %d retries: %w", retryCount, err) | ||
|
There was a problem hiding this comment. Maybe include the information I had in my log about the page number and repoName and prNumber and repoOwner? There was a problem hiding this comment. done and wrapped above |
||
| } | ||
|
|
||
| for _, file := range files { | ||
|
|
@@ -84,7 +95,7 @@ func getChangedFilesFromAPI(ctx context.Context, ghContext *actionscore.Context, | |
| } | ||
| } | ||
|
|
||
| if res.NextPage == 0 { | ||
|
There was a problem hiding this comment. I am not sure this will work? Keeping it res.LastPage worked for me in the messaging branch that had multiple pages. There was a problem hiding this comment.
I ran it w/ a fmt.Println and you can see the last page detection not working here https://github.com/synapsecns/sanguine/actions/runs/4309209034/jobs/7516328981 It'd get to 12 and then reset to 1. res.LastPage can be 0 when the cursor is lost |
||
| break | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent so much time on this :-)