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

Shipit appears to miss some check run statuses for a commit (missing pagination?) #1323

Open
sjagoe opened this issue Jan 26, 2024 · 2 comments

Comments

@sjagoe
Copy link
Contributor

sjagoe commented Jan 26, 2024

We have been encountering a rare issue where check run statuses for a commit don't always update for the final check run status (thus blocking automatic deployment). When this does happen, the Refresh statuses & commits button does not appear to solve the issue.

This has only occurred on repositories with a large number of check runs (e.g. larger matrix test runs). In the past we've brute-forced our way past this by configuring the stack to ignore most check runs.

Checking one of the commits, it looks like the Commit.refresh_check_runs! method will only pull the first page of 30 results.

irb(main):041:0> response = stack.github_api.check_runs(stack.github_repo_name, commit.sha)
=>
{:total_count=>44,
...
irb(main):045:0> response.total_count
=> 44
irb(main):046:0> response.check_runs.length
=> 30

Simply increasing the page size (as seems to have been done for commit statuses) will probably make this much more unlikely to hit, but maybe pagination for both of check_runs and statuses would be useful.

@sjagoe
Copy link
Contributor Author

sjagoe commented Jan 26, 2024

I've validated a patch in our own shipit deployment, but being completely new to rails, I an unable to craft a unit test to validate this (specifically how to mock multiple different calls to github_api.last_response with different return values). Here's the patch I have applied locally if somebody would like to pick it up:

diff --git a/app/models/shipit/commit.rb b/app/models/shipit/commit.rb
index a1417cc5b..c86133753 100644
--- a/app/models/shipit/commit.rb
+++ b/app/models/shipit/commit.rb
@@ -168,10 +168,26 @@ module Shipit
       end
     end

-    def refresh_check_runs!
+    def paginated_check_runs
       response = stack.handle_github_redirections do
-        stack.github_api.check_runs(github_repo_name, sha)
+        stack.github_api.check_runs(github_repo_name, sha, per_page: 100)
+      end
+
+      return response if stack.github_api.last_response.rels[:next].nil?
+
+      loop do
+        page = stack.handle_github_redirections do
+          stack.github_api.get(stack.github_api.last_response.rels[:next].href)
+        end
+        response.check_runs.concat(page.check_runs)
+        break if stack.github_api.last_response.rels[:next].nil?
       end
+
+      response
+    end
+
+    def refresh_check_runs!
+      response = paginated_check_runs
       response.check_runs.each do |check_run|
         create_or_update_check_run_from_github!(check_run)
       end

@casperisfine
Copy link
Contributor

if somebody would like to pick it up:

Please just submit a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants