ci: add Docker Hub authentication to mitigate pull rate limits#2393
ci: add Docker Hub authentication to mitigate pull rate limits#2393yongwww merged 3 commits intoflashinfer-ai:mainfrom
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughAdds Docker Hub login steps ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/pr-test.yml:
- Around line 173-178: The "Login to Docker Hub" step using
docker/login-action@v3 should be skipped for external-fork PRs; add an if
condition to the step to only run when the repository owner is the org/user
(e.g., github.repository_owner == 'flashinfer'). Modify the step that currently
uses docker/login-action@v3 (named "Login to Docker Hub") to include if: ${{
github.repository_owner == 'flashinfer' }} so external contributors' workflows
don't attempt to use the repo secrets.
- Around line 218-223: The Docker Hub login step ("Login to Docker Hub" using
docker/login-action@v3 with username flashinfer) needs the same fork-PR guard as
earlier; add an if condition to the step (e.g. if: ${{
github.event.pull_request.head.repo.full_name == github.repository }}) so the
docker/login step is skipped for external/forked PRs to avoid failing when
secrets are unavailable.
- Around line 124-129: The Docker Hub login step ("Login to Docker Hub" using
docker/login-action@v3 with username flashinfer and password ${{
secrets.DOCKERHUB_TOKEN }}) will fail for forked PRs because secrets are not
exposed; update that step to run only when the PR is not from a fork (i.e., skip
login for pull_request events where head repo differs from the base repo) so
anonymous pulls are used instead, and apply the same conditional guard to the
other two docker/login-action@v3 login steps in the workflow.
| - name: Login to Docker Hub | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| username: flashinfer | ||
| password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
|
There was a problem hiding this comment.
Login will fail for fork PRs due to inaccessible secrets.
GitHub Actions does not expose repository secrets to workflows triggered by pull requests from forked repositories. This causes the login step to fail for external contributors, breaking their CI.
Add a condition to skip login for fork PRs (they'll fall back to anonymous rate limits):
🔧 Proposed fix
- name: Login to Docker Hub
+ if: github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request'
uses: docker/login-action@v3
with:
username: flashinfer
password: ${{ secrets.DOCKERHUB_TOKEN }}Apply this same fix to all three login steps in this file.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Login to Docker Hub | |
| uses: docker/login-action@v3 | |
| with: | |
| username: flashinfer | |
| password: ${{ secrets.DOCKERHUB_TOKEN }} | |
| - name: Login to Docker Hub | |
| if: github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request' | |
| uses: docker/login-action@v3 | |
| with: | |
| username: flashinfer | |
| password: ${{ secrets.DOCKERHUB_TOKEN }} |
🤖 Prompt for AI Agents
In @.github/workflows/pr-test.yml around lines 124 - 129, The Docker Hub login
step ("Login to Docker Hub" using docker/login-action@v3 with username
flashinfer and password ${{ secrets.DOCKERHUB_TOKEN }}) will fail for forked PRs
because secrets are not exposed; update that step to run only when the PR is not
from a fork (i.e., skip login for pull_request events where head repo differs
from the base repo) so anonymous pulls are used instead, and apply the same
conditional guard to the other two docker/login-action@v3 login steps in the
workflow.
| - name: Login to Docker Hub | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| username: flashinfer | ||
| password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
|
There was a problem hiding this comment.
Apply the same conditional fix here.
Same fork PR issue as noted above. Add the if condition to prevent failures for external contributors.
🤖 Prompt for AI Agents
In @.github/workflows/pr-test.yml around lines 173 - 178, The "Login to Docker
Hub" step using docker/login-action@v3 should be skipped for external-fork PRs;
add an if condition to the step to only run when the repository owner is the
org/user (e.g., github.repository_owner == 'flashinfer'). Modify the step that
currently uses docker/login-action@v3 (named "Login to Docker Hub") to include
if: ${{ github.repository_owner == 'flashinfer' }} so external contributors'
workflows don't attempt to use the repo secrets.
| - name: Login to Docker Hub | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| username: flashinfer | ||
| password: ${{ secrets.DOCKERHUB_TOKEN }} | ||
|
|
There was a problem hiding this comment.
Apply the same conditional fix here.
Same fork PR issue as noted above. Add the if condition to prevent failures for external contributors.
🤖 Prompt for AI Agents
In @.github/workflows/pr-test.yml around lines 218 - 223, The Docker Hub login
step ("Login to Docker Hub" using docker/login-action@v3 with username
flashinfer) needs the same fork-PR guard as earlier; add an if condition to the
step (e.g. if: ${{ github.event.pull_request.head.repo.full_name ==
github.repository }}) so the docker/login step is skipped for external/forked
PRs to avoid failing when secrets are unavailable.
2 participants
📌 Description
We are running both Jenkins (should be disabled soon though) and GHA workflow for the public unit tests at this moment, the docker pull might hit rate limit of 100 pulls/6hr, for example this job: https://github.com/flashinfer-ai/flashinfer/actions/runs/21193948826/job/60965961746.
In this pr, we are trying to add docker/login-action to authenticate with Docker Hub before running tests. This increases the pull rate limit from 100 pulls/6hr (anonymous) to 200 pulls/6hr (authenticated), reducing the likelihood of rate limit errors when running concurrent CI jobs.
🔍 Related Issues
🚀 Pull Request Checklist
Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).Reviewer Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.