ci: require release for docker image so that python version is available #171
Conversation
📝 WalkthroughWalkthroughRemoved tag gating from the Release job, added tag-based gating to the Publish to PyPI step, made the Docker job depend on Release, and fixed a typo in a dataset creation completion message. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant Release as Release Job
participant Docker as Docker Job
participant PyPI as Publish Step
GitHub->>Release: trigger workflow (any ref)
Release-->>GitHub: completes
GitHub->>Docker: starts (needs release)
GitHub->>PyPI: runs publish step (only if tag ref)
Docker-->>GitHub: pushes images (if not PR)
PyPI-->>GitHub: publishes package (on tag)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
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: 1
🤖 Fix all issues with AI agents
In @.github/workflows/cd.yaml:
- Line 128: The docker job currently declares a hard dependency via needs:
[release], causing it to be skipped when the release job is skipped (e.g., on
PRs, branch pushes, workflow_dispatch); remove the needs: [release] entry from
the docker job definition so the docker job can run independently per its
metadata (tags for type=ref,event=branch and type=ref,event=pr); keep the
release job intact and ensure the docker job continues to reference its
Dockerfile (Dockerfile.kubimo) and any local build steps without relying on
release artifacts.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cd.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: linux (aarch64)
- GitHub Check: linux (x86)
- GitHub Check: linux (x86_64)
- GitHub Check: macos (x86_64)
- GitHub Check: windows (x64)
- GitHub Check: macos (aarch64)
- GitHub Check: windows (x86)
- GitHub Check: Build and publish npm package
- GitHub Check: Test Workspace
- GitHub Check: Test WASM (chrome)
- GitHub Check: Test WASM (safari)
- GitHub Check: Test WASM (firefox)
jpopesculian
force-pushed
the
fix-ci-docker-py-version
branch
from
1447d69 to
210204f
Compare
jpopesculian
force-pushed
the
fix-ci-docker-py-version
branch
from
210204f to
2503cec
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/cd.yaml (1)
163-165: Address the TODO comment before merging.The commented line suggests the push behavior should be conditional (
push: ${{ github.event_name != 'pull_request' }}), but currently it's set topush: trueunconditionally. This means Docker images are pushed on every event including PRs.Consider whether this TODO should be resolved as part of this PR or tracked separately.
🤖 Fix all issues with AI agents
In @.github/workflows/cd.yaml:
- Line 128: The docker job currently lists a hard dependency "needs: [release]"
which causes unnecessary waiting on non-tag events; either remove the needs
entry from the docker job or make the dependency conditional for tag events only
by adding an if condition on the docker job (use the existing tag check
startsWith(github.ref, 'refs/tags/')). Update the docker job definition that
contains "needs: [release]" so it either drops that needs array entirely or
keeps it but adds the if: startsWith(github.ref, 'refs/tags/') guard to ensure
the release dependency is only required for tag-triggered runs.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cd.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: linux (x86_64)
- GitHub Check: macos (aarch64)
- GitHub Check: Test Workspace
- GitHub Check: Test WASM (safari)
- GitHub Check: Test WASM (firefox)
- GitHub Check: Test WASM (chrome)
- GitHub Check: Build and publish npm package
- GitHub Check: windows (x86)
- GitHub Check: linux (aarch64)
- GitHub Check: linux (x86)
- GitHub Check: macos (x86_64)
- GitHub Check: windows (x64)
🔇 Additional comments (1)
.github/workflows/cd.yaml (1)
118-118: LGTM! Conditional PyPI publishing enables broader release job execution.Moving the tag gate from the job level to the publish step allows the release job to run on all events while only publishing to PyPI on tag pushes. This supports the docker job dependency without requiring tag-only execution.
| docker: | ||
| name: Build and Push Docker Image | ||
| runs-on: ubuntu-latest | ||
| needs: [release] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check docker-bake.hcl and Dockerfile for references to wheel artifacts or version info
echo "=== Checking docker-bake.hcl for artifact or version references ==="
if [ -f "docker-bake.hcl" ]; then
cat docker-bake.hcl
else
echo "docker-bake.hcl not found"
fi
echo -e "\n=== Searching for Dockerfiles that might reference wheels or version ==="
find . -name "Dockerfile*" -type f | head -20 | xargs -I {} sh -c 'echo "--- {} ---"; cat "{}"'
echo -e "\n=== Checking for any references to wheel files or maturin in docker context ==="
rg -n "\.whl|maturin|PYTHON_VERSION|pyproject" . --type docker || echo "No references found in docker files"
echo -e "\n=== Checking cd.yaml workflow structure around docker and release jobs ==="
sed -n '100,200p' .github/workflows/cd.yamlRepository: aqora-io/cli
Length of output: 4844
🏁 Script executed:
#!/bin/bash
# Check if release job has any outputs defined
echo "=== Full release job definition in cd.yaml ==="
sed -n '/^ release:/,/^ [a-z]/p' .github/workflows/cd.yaml | head -50
echo -e "\n=== Check if docker job uses any outputs from release ==="
sed -n '/^ docker:/,/^ [a-z]/p' .github/workflows/cd.yaml | grep -i "release\|needs\|env\|output"
echo -e "\n=== Check git history for this change ==="
git log --oneline --all -n 20 | head -20Repository: aqora-io/cli
Length of output: 1682
Remove the release job dependency or condition it on tag events only.
The docker job depends on the release job, which causes it to wait for all platform builds on every event (PRs, main pushes, tags). However:
- The docker build does not download or reference any artifacts from the release job
- The release job defines no outputs to pass to the docker job
- The Dockerfile compiles the Rust binary from source via cargo—it has no dependency on the wheel artifacts the release job publishes
- The docker metadata action generates tags purely from git refs, not from release job data
The commit message claims "python version is available" from this dependency, but no mechanism exists to pass any data between the jobs. On non-tag events, this dependency is wasteful because the release job doesn't publish anyway (if: "startsWith(github.ref, 'refs/tags/')" is conditional).
Either remove the dependency entirely, or condition it on tag events: if: startsWith(github.ref, 'refs/tags/').
🤖 Prompt for AI Agents
In @.github/workflows/cd.yaml at line 128, The docker job currently lists a hard
dependency "needs: [release]" which causes unnecessary waiting on non-tag
events; either remove the needs entry from the docker job or make the dependency
conditional for tag events only by adding an if condition on the docker job (use
the existing tag check startsWith(github.ref, 'refs/tags/')). Update the docker
job definition that contains "needs: [release]" so it either drops that needs
array entirely or keeps it but adds the if: startsWith(github.ref, 'refs/tags/')
guard to ensure the release dependency is only required for tag-triggered runs.
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.