-
Notifications
You must be signed in to change notification settings - Fork 188
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
Update devcontainer image: v1.0.0-rc2 #2670
Conversation
WalkthroughOhayo, sensei! This pull request updates the Docker image version across several configuration files used in the development and CI/CD environments. Specifically, the image version has been changed from Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
.devcontainer/devcontainer.json (1)
5-5
: Consider standardizing the version format, sensei!The version format changed from using a dot separator (
rc.1
) to no separator (rc2
). This might cause confusion or issues with version parsing. Consider maintaining consistency in the version format across all references.Suggested format (choose one consistently):
- With dot:
v1.0.0-rc.2
- Without dot:
v1.0.0-rc2
.github/workflows/ci.yml (3)
Line range hint
13-21
: Ohayo sensei! Consider using predefined paths for better maintainability.The binary management looks good, but we can improve the path handling:
build: runs-on: ubuntu-latest-4-cores + env: + BINS_DIR: bins + RELEASE_DIR: target/release steps: - uses: actions/checkout@v3 - uses: Swatinem/rust-cache@v2 - run: | cargo build -r --bin katana cargo build -r --bin sozo - mkdir -p bins - cp ./target/release/katana bins/ - cp ./target/release/sozo bins/ + mkdir -p $BINS_DIR + cp $RELEASE_DIR/{katana,sozo} $BINS_DIR/ - uses: actions/upload-artifact@v4 with: name: dojo-bins - path: bins + path: $BINS_DIR🧰 Tools
🪛 actionlint
32-32: label "ubuntu-latest-32-cores" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
Line range hint
41-47
: Enhance test setup reliability with error handling.The test setup could be more robust:
- run: | export PATH=/tmp/bins:$PATH chmod +x /tmp/bins/katana chmod +x /tmp/bins/sozo bash scripts/build_cairo_projects.sh /tmp/bins/sozo - tar -xzf spawn-and-move-db.tar.gz -C /tmp/ - tar -xzf types-test-db.tar.gz -C /tmp/ + for db in spawn-and-move-db.tar.gz types-test-db.tar.gz; do + echo "Extracting $db..." + if ! tar -xzf "$db" -C /tmp/; then + echo "Failed to extract $db" + exit 1 + fi + # Verify critical files exist + test -f "/tmp/${db%.tar.gz}/critical_file" || { echo "$db extraction incomplete"; exit 1; } + done🧰 Tools
🪛 actionlint
32-32: label "ubuntu-latest-32-cores" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
Line range hint
89-102
: Enhance Katana binary verification.The current verification could be more robust:
- run: | + # Set timeout for Katana startup + TIMEOUT=30 + START_TIME=$(date +%s) chmod +x ./katana ./katana & KATANA_PID=$! - sleep 2 - if ! kill -0 $KATANA_PID; then - echo "Katana exited with an error" - exit 1 - fi + # Wait for Katana to start or timeout + while true; do + if ! kill -0 $KATANA_PID 2>/dev/null; then + echo "Katana failed to start" + exit 1 + fi + if curl -s http://localhost:5050 >/dev/null; then + echo "Katana started successfully" + break + fi + if [ $(($(date +%s) - START_TIME)) -gt $TIMEOUT ]; then + echo "Timeout waiting for Katana" + kill $KATANA_PID + exit 1 + fi + sleep 1 + done kill $KATANA_PID🧰 Tools
🪛 actionlint
32-32: label "ubuntu-latest-32-cores" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
.devcontainer/devcontainer.json
(1 hunks).github/workflows/bench.yml
(2 hunks).github/workflows/ci.yml
(7 hunks).github/workflows/release-dispatch.yml
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/bench.yml
- .github/workflows/release-dispatch.yml
🔇 Additional comments (1)
.devcontainer/devcontainer.json (1)
5-5
: Ohayo sensei! Let's verify the container image availability.
The version bump from rc.1 to rc2 looks good, but we should ensure the new image is published and accessible.
✅ Verification successful
Ohayo sensei! The container image is verified and ready to roll! ✨
The v1.0.0-rc2
image is published and available in the GitHub Container Registry. It was created on November 10th, 2024, and is also tagged as latest
, indicating it's the most recent stable version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the new container image exists and is accessible
# Note: This only checks if the image manifest can be retrieved, not if it can be pulled
gh api \
-H "Accept: application/vnd.oci.image.index.v1+json" \
/repos/dojoengine/dojo/pkgs/container/dojo-dev/versions \
| jq -r '.[] | select(.metadata.container.tags[] == "v1.0.0-rc2")'
Length of output: 372
Script:
#!/bin/bash
# Let's try a different approach using the GitHub Container Registry API
# First, let's check if the package exists and get its details
gh api \
-H "Accept: application/vnd.github+json" \
/orgs/dojoengine/packages/container/dojo-dev/versions \
| jq -r '.[] | select(.metadata.container.tags | contains(["v1.0.0-rc2"])) | {tags: .metadata.container.tags, created_at: .created_at}'
Length of output: 330
@@ -31,7 +31,7 @@ jobs: | |||
needs: ensure-docker | |||
runs-on: ubuntu-latest-32-cores | |||
container: | |||
image: ghcr.io/dojoengine/dojo-dev:v1.0.0-rc.1 | |||
image: ghcr.io/dojoengine/dojo-dev:v1.0.0-rc2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Centralize Docker image version management.
The Docker image version is duplicated across multiple jobs. Consider using a workflow-level environment variable:
name: ci
env:
CARGO_TERM_COLOR: always
RUST_VERSION: 1.79.0
+ DOJO_IMAGE_VERSION: v1.0.0-rc2
jobs:
test:
container:
- image: ghcr.io/dojoengine/dojo-dev:v1.0.0-rc2
+ image: ghcr.io/dojoengine/dojo-dev:${{ env.DOJO_IMAGE_VERSION }}
Also, consider adding a version verification step:
- name: Verify Docker image version
run: |
if ! docker run --rm ${{ env.DOJO_IMAGE }} cat /version.txt | grep -q "^${{ env.DOJO_IMAGE_VERSION }}$"; then
echo "Docker image version mismatch"
exit 1
fi
Also applies to: 59-59, 126-126, 142-142, 157-157, 166-166, 175-175
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2670 +/- ##
==========================================
+ Coverage 57.41% 57.42% +0.01%
==========================================
Files 403 403
Lines 50818 50818
==========================================
+ Hits 29177 29183 +6
+ Misses 21641 21635 -6 ☔ View full report in Codecov by Sentry. |
Automated changes by create-pull-request GitHub action
Summary by CodeRabbit
New Features
v1.0.0-rc2
across various workflows, enhancing the development and testing environments.Bug Fixes
Documentation