Conversation
Signed-off-by: Fatih Acar <fatih@opsmill.com>
This is to workaround a weird concurrency issue with buildx where the build for server and task-worker conflicts... Signed-off-by: Fatih Acar <fatih@opsmill.com>
WalkthroughThis pull request updates the development container configuration files. The docker-in-docker feature references in two devcontainer configuration files are updated to use explicit GitHub Container Registry paths with version tags instead of generic feature names. The 🚥 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. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.devcontainer/devcontainer.json (1)
48-48:⚠️ Potential issue | 🟡 MinorUpdate
"github-cli"to use the full GHCR path for consistency and to avoid short-name resolution issues.The entry currently uses short-name format (
"github-cli": "latest"), whiledocker-in-dockeron line 44 uses the full GHCR path. The short-name format is susceptible to the same resolution failures that affected the docker-in-docker entry.♻️ Proposed fix
- "github-cli": "latest" + "ghcr.io/devcontainers/features/github-cli:1": { + "version": "latest" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/devcontainer.json at line 48, Replace the short image name "github-cli": "latest" with the full GHCR image path to match the docker-in-docker entry and avoid short-name resolution issues; locate the "github-cli" entry in .devcontainer/devcontainer.json and update its value to the corresponding GHCR full path (the same repository namespace used by the "docker-in-docker" entry) so the image is referenced via ghcr.io/...:latest rather than the short name.
🧹 Nitpick comments (1)
.devcontainer/onCreateCommand.sh (1)
1-10: Missingset -e— a failure indemo.pullsilently allowsdev.buildto proceed.Without
set -e(orset -eo pipefail), ifdemo.pullfails mid-prebuild (e.g., network error), the subsequentdev.build --service serverstill runs against a potentially incomplete state.♻️ Proposed fix
#!/bin/bash +set -eo pipefail cat /proc/cpuinfo /proc/meminfo🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/onCreateCommand.sh around lines 1 - 10, Add shell error-fail early behavior to the script so a failure in "uv run invoke demo.pull" stops execution before "uv run invoke dev.build --service server" runs: at the top of the script enable "set -e" (or "set -eo pipefail") so any non-zero exit from commands like "uv run invoke demo.pull" or "git submodule update --init" will abort the script; ensure this is placed before any commands (right after the shebang) so subsequent commands such as "uv run invoke dev.build --service server" cannot run on a failed prebuild.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/onCreateCommand.sh:
- Line 10: The shared onCreateCommand.sh currently runs "uv run invoke
demo.pull" and then always runs "uv run invoke dev.build --service server",
which causes the demo container to build unnecessarily; modify
onCreateCommand.sh so the dev.build step only runs for the dev container: either
split into two scripts (keep dev.build in a new script referenced by
.devcontainer/devcontainer.json and leave demo.pull in the shared script
referenced by .devcontainer/demo-container/devcontainer.json) or add a guard in
onCreateCommand.sh that checks an environment variable/flag (e.g.,
CONTAINER_TYPE or DEMO=true) before executing "uv run invoke dev.build --service
server", leaving "uv run invoke demo.pull" unconditional for the demo flow.
---
Outside diff comments:
In @.devcontainer/devcontainer.json:
- Line 48: Replace the short image name "github-cli": "latest" with the full
GHCR image path to match the docker-in-docker entry and avoid short-name
resolution issues; locate the "github-cli" entry in
.devcontainer/devcontainer.json and update its value to the corresponding GHCR
full path (the same repository namespace used by the "docker-in-docker" entry)
so the image is referenced via ghcr.io/...:latest rather than the short name.
---
Duplicate comments:
In @.devcontainer/demo-container/devcontainer.json:
- Line 49: The "github-cli": "latest" short-name is duplicated between
devcontainer files; update this entry in
.devcontainer/demo-container/devcontainer.json so the short name is unique
(e.g., rename to "github-cli-demo" or use the full extension identifier from the
marketplace) or remove the duplicate if unnecessary; ensure you update any
references that rely on the original short-name (search for "github-cli":
"latest") and keep the value consistent with the extension identifier format
used elsewhere.
---
Nitpick comments:
In @.devcontainer/onCreateCommand.sh:
- Around line 1-10: Add shell error-fail early behavior to the script so a
failure in "uv run invoke demo.pull" stops execution before "uv run invoke
dev.build --service server" runs: at the top of the script enable "set -e" (or
"set -eo pipefail") so any non-zero exit from commands like "uv run invoke
demo.pull" or "git submodule update --init" will abort the script; ensure this
is placed before any commands (right after the shebang) so subsequent commands
such as "uv run invoke dev.build --service server" cannot run on a failed
prebuild.
| uv sync --all-groups | ||
|
|
||
| uv run invoke demo.pull | ||
| uv run invoke dev.build --service server |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all devcontainer.json files and check their onCreateCommand references
find . -name "devcontainer.json" -type f | head -20Repository: opsmill/infrahub
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Check what each devcontainer.json references for onCreateCommand
echo "=== Main devcontainer.json ==="
cat .devcontainer/devcontainer.json 2>/dev/null | grep -A 2 -B 2 "onCreateCommand" || echo "File not found or no match"
echo -e "\n=== Demo devcontainer.json ==="
cat demo-container/devcontainer.json 2>/dev/null | grep -A 2 -B 2 "onCreateCommand" || echo "File not found or no match"Repository: opsmill/infrahub
Length of output: 358
🏁 Script executed:
#!/bin/bash
# List the full contents of both devcontainer.json files to understand the full context
echo "=== Main .devcontainer/devcontainer.json ==="
cat .devcontainer/devcontainer.json
echo -e "\n=== demo-container/devcontainer.json ==="
cat demo-container/devcontainer.jsonRepository: opsmill/infrahub
Length of output: 1629
🏁 Script executed:
#!/bin/bash
# Check if there are other onCreateCommand scripts besides the shared one
find . -name "onCreateCommand*" -type fRepository: opsmill/infrahub
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Check the current content and structure of the onCreateCommand.sh script
echo "=== Full onCreateCommand.sh ==="
cat -n .devcontainer/onCreateCommand.shRepository: opsmill/infrahub
Length of output: 323
🏁 Script executed:
#!/bin/bash
# Read the demo-container devcontainer.json from the correct path
echo "=== .devcontainer/demo-container/devcontainer.json ==="
cat .devcontainer/demo-container/devcontainer.jsonRepository: opsmill/infrahub
Length of output: 1592
Shared onCreateCommand.sh runs dev.build --service server for both dev and demo containers — verify if intentional.
Both .devcontainer/devcontainer.json and .devcontainer/demo-container/devcontainer.json reference the same onCreateCommand.sh script. The script executes uv run invoke demo.pull (line 9) followed by uv run invoke dev.build --service server (line 10).
The demo container's demo.pull is designed to pull pre-built images, so additionally running a full local dev.build --service server inside the demo container appears unintended and will significantly increase prebuild time for that configuration.
If the build step is only meant for the dev container, move it to a separate script referenced only by .devcontainer/devcontainer.json, or guard it with a conditional check.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/onCreateCommand.sh at line 10, The shared onCreateCommand.sh
currently runs "uv run invoke demo.pull" and then always runs "uv run invoke
dev.build --service server", which causes the demo container to build
unnecessarily; modify onCreateCommand.sh so the dev.build step only runs for the
dev container: either split into two scripts (keep dev.build in a new script
referenced by .devcontainer/devcontainer.json and leave demo.pull in the shared
script referenced by .devcontainer/demo-container/devcontainer.json) or add a
guard in onCreateCommand.sh that checks an environment variable/flag (e.g.,
CONTAINER_TYPE or DEMO=true) before executing "uv run invoke dev.build --service
server", leaving "uv run invoke demo.pull" unconditional for the demo flow.
|
This PR has been inactive for 2 weeks and has been marked as stale. If you're still working on this, please:
The stale label will be removed automatically when there's new activity. |
1 participant
Summary by CodeRabbit