perf(github): cache check-run lookups with retry#2669
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2669 +/- ##
==========================================
+ Coverage 59.01% 59.10% +0.09%
==========================================
Files 208 208
Lines 20452 20513 +61
==========================================
+ Hits 12069 12125 +56
- Misses 7609 7614 +5
Partials 774 774 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a caching mechanism for GitHub check runs to reduce the number of API calls made by the provider. It adds a checkRunsCache struct and utilizes sync.Once within getExistingCheckRunID to fetch and store all check run pages. Feedback on the implementation highlights a potential issue where sync.Once will cache transient errors from the initial API call, preventing any future successful attempts for that provider instance. It is recommended to use a sync.Mutex or a similar pattern to avoid caching failed operations.
|
Added API calls duplication data in the description showing no duplicate calls to fetch check_runs |
|
setting draft, to give you time to address the comments.. |
theakshaypant
force-pushed
the
perf/cache-check-runs
branch
2 times, most recently
from
b421ba1 to
f078824
Compare
theakshaypant
changed the title
|
/test go-testing |
zakisk
force-pushed
the
perf/cache-check-runs
branch
from
f078824 to
e42d570
Compare
theakshaypant
force-pushed
the
perf/cache-check-runs
branch
from
e42d570 to
1ac8084
Compare
|
/retest |
theakshaypant
force-pushed
the
perf/cache-check-runs
branch
from
1ac8084 to
2a8c9de
Compare
zakisk
force-pushed
the
perf/cache-check-runs
branch
from
2a8c9de to
9670f29
Compare
| } | ||
| } | ||
|
|
||
| func TestGetExistingCheckRunIDCacheHit(t *testing.T) { |
There was a problem hiding this comment.
overall PR looks good! it reduces the list check run api call to 98%. can you please make the unit tests table based as done across project and we also have skill but I don't know why its not working in you prompt to claude.
There was a problem hiding this comment.
Thanks! Simplified the tests so that they can follow the same table format in 1a57509
theakshaypant
force-pushed
the
perf/cache-check-runs
branch
from
9670f29 to
1a57509
Compare
Cache GitHub check-run API responses to avoid repeated paginated API calls during status updates. Use a mutex+channel pattern so concurrent goroutines share a single in-flight fetch. Failed fetches are not cached, allowing retries with exponential backoff on transient API errors. Signed-off-by: Akshay Pant <akpant@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
zakisk
force-pushed
the
perf/cache-check-runs
branch
from
1a57509 to
07ff147
Compare
|
Runner crunch 🙃 |
4 participants
📝 Description of the Change
Cache GitHub check-run API responses to avoid repeated
paginated API calls during status updates. Use a mutex+channel
pattern so concurrent goroutines share a single in-flight fetch.
Failed fetches are not cached, allowing retries with exponential
backoff on transient API errors.
🔗 Linked GitHub Issue
Partial #2379
🧪 Testing Strategy
Doing an analysis of the ghe e2e test suite, no duplicate list_check_runs call remains
All duplicate API calls remaining
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.