fix vllm tagging and add cloud images w/o tmux#3049
Conversation
📝 WalkthroughWalkthroughThe workflow matrix in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/main.yml (1)
96-107: Missing base build for non-vllm PyTorch 2.7.1 cloud imagesThe
build-axolotljob only builds PyTorch 2.7.1 withaxolotl_extras: vllm. In both thebuild-axolotl-cloud(lines 96–107) andbuild-axolotl-cloud-no-tmux(lines 160–171) matrices, you’ve added a 2.7.1 entry withoutaxolotl_extras: vllm. That computes aBASE_TAG(e.g.…-py3.11-cu126-2.7.1) that doesn’t exist, causing the docker/build-push-action to 404 when pulling the base image.To fix, choose one:
- Add a non-vllm 2.7.1 entry to the build-axolotl base matrix; or
- Remove the non-vllm 2.7.1 entries from both cloud matrices; or
- Ensure your base image naming logic accounts for non-vllm tags.
Minimal diff to remove the problematic entries in each matrix:
--- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ jobs.build-axolotl-cloud.matrix - - cuda: 126 - cuda_version: 12.6.3 - python_version: "3.11" - pytorch: 2.7.1 - axolotl_extras: - is_latest: - cuda: 126 cuda_version: 12.6.3 python_version: "3.11" pytorch: 2.7.1 axolotl_extras: vllm is_latest: true @@ jobs.build-axolotl-cloud-no-tmux.matrix - - cuda: 126 - cuda_version: 12.6.3 - python_version: "3.11" - pytorch: 2.7.1 - axolotl_extras: - is_latest: - cuda: 126 cuda_version: 12.6.3 python_version: "3.11" pytorch: 2.7.1 axolotl_extras: vllm is_latest: trueAlso consider dropping the null
is_latest:lines or setting them explicitly tofalseif you retain any non-vllm variants.
🧹 Nitpick comments (2)
.github/workflows/main.yml (2)
101-101: Avoid null is_latest: either omit key or set false for clarity.YAML
is_latest:with no value is null; the expression treats it as falsy, but it’s noisy. Prefer removing the line oris_latest: false.- is_latest: + # omit is_latest to default to false
165-165: Drop null is_latest here as well.Keep it explicit (false) or omit the key entirely.
- is_latest: + # omit is_latest to default to false
| - cuda: 126 | ||
| cuda_version: 12.6.3 | ||
| python_version: "3.11" | ||
| pytorch: 2.7.1 | ||
| axolotl_extras: | ||
| is_latest: | ||
| - cuda: 126 | ||
| cuda_version: 12.6.3 | ||
| python_version: "3.11" | ||
| pytorch: 2.7.1 | ||
| axolotl_extras: vllm | ||
| is_latest: true |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Same base mismatch for no-tmux: non-vllm 2.7.1 will reference a non-existent base image.
The no-tmux job includes a 2.7.1 non-vllm variant, but build-axolotl doesn’t produce the corresponding base tag. This will fail when resolving BASE_TAG.
Either add a non-vllm 2.7.1 base build in build-axolotl or remove this entry here. Minimal fix in this PR:
- cuda: 126
cuda_version: 12.6.3
python_version: "3.11"
- pytorch: 2.7.1
- axolotl_extras:
- is_latest:
- - cuda: 126
- cuda_version: 12.6.3
- python_version: "3.11"
- pytorch: 2.7.1
- axolotl_extras: vllm
- is_latest: true
+ pytorch: 2.7.1
+ axolotl_extras: vllm
+ is_latest: trueTo double-check, parse the base matrix:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
awk '/^ build-axolotl:/{flag=1} flag && /^ [^ ]/{if ($0 !~ /^ build-axolotl:/) exit}; flag{print}' .github/workflows/main.yml \
| sed -n '/matrix:/,/runs-on:/p' | rg -n 'pytorch:\s*2\.7\.1' -A3 -B3Length of output: 609
Remove non-vllm PyTorch 2.7.1 entry from no-tmux matrix
The build-axolotl job only publishes a vllm variant for PyTorch 2.7.1 on CUDA 12.6.3—there is no corresponding non-vllm base image, so the current non-vllm entry will fail to resolve BASE_TAG.
• File: .github/workflows/main.yml
• Section: no-tmux job matrix (around lines 160–171)
Apply this diff:
- cuda: 126
cuda_version: 12.6.3
python_version: "3.11"
- pytorch: 2.7.1
- axolotl_extras:
- is_latest:
- - cuda: 126
- cuda_version: 12.6.3
- python_version: "3.11"
- pytorch: 2.7.1
- axolotl_extras: vllm
- is_latest: true
+ pytorch: 2.7.1
+ axolotl_extras: vllm
+ is_latest: true📝 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.
| - cuda: 126 | |
| cuda_version: 12.6.3 | |
| python_version: "3.11" | |
| pytorch: 2.7.1 | |
| axolotl_extras: | |
| is_latest: | |
| - cuda: 126 | |
| cuda_version: 12.6.3 | |
| python_version: "3.11" | |
| pytorch: 2.7.1 | |
| axolotl_extras: vllm | |
| is_latest: true | |
| - cuda: 126 | |
| cuda_version: 12.6.3 | |
| python_version: "3.11" | |
| pytorch: 2.7.1 | |
| axolotl_extras: vllm | |
| is_latest: true |
🤖 Prompt for AI Agents
In .github/workflows/main.yml around lines 160 to 171, remove the matrix entry
for PyTorch 2.7.1 on CUDA 12.6.3 that does not include the vllm variant, as the
build-axolotl job only supports the vllm variant for this configuration and the
non-vllm entry will fail to resolve BASE_TAG. Keep only the entry with
axolotl_extras set to vllm and is_latest set to true.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Description
Motivation and Context
How has this been tested?
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)
Summary by CodeRabbit