feat: interactive ColdFront walkthrough (4 demos + Codespaces)#38
Conversation
…rrect service count in README
…docs, check compose v2
…vcontainer fail-fast
…p pre-menu Phase B
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds a ColdFront walkthrough environment with devcontainer setup, CI validation, runnable documentation, bootstrap scripts, a Compose stack, and shell orchestration for four demonstrations. ChangesColdFront Walkthrough Onboarding
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Up to standards ✅🟢 Issues
|
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 (4)
docs/walkthrough.md (1)
32-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrerequisites list omits
jq.
examples/walkthrough/setup.sh'sREQUIRED_CMDSincludesdocker curl jq psql(plusss/lsof), but the docs list only mentionscurl,bash, andpsql. Readers following the docs without running the one-liner may hit an avoidable missing-dependency error.📝 Proposed fix
-- `curl`, `bash`, and `psql` on the host (the psql commands connect to - the published port). +- `curl`, `bash`, `jq`, and `psql` on the host (the psql commands connect + to the published port).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/walkthrough.md` around lines 32 - 39, The prerequisites section is missing jq, which is required by examples/walkthrough/setup.sh via REQUIRED_CMDS. Update the walkthrough prerequisites list to include jq alongside curl, bash, and psql, and keep the wording aligned with the setup.sh dependency check so readers don’t miss the needed host tools..github/workflows/ci-walkthrough.yml (3)
54-63: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExited-container check is a no-op —
docker compose pshides exited containers by default.Per Docker's docs,
docker compose ps"By default, only running containers are shown";--allis required to include stopped/exited ones. As written,grep -E "Exit|exited"will never match anything, since exited containers are already filtered out — this step can never catch a service that crashed and will always report "All services are running."Note: if you add
--all, ensure the filter also excludes services that intentionally complete and exit (e.g.,lakekeeper-migrate, a one-shot migration container perdocker-compose.yml), to avoid a new false-positive.🔧 Proposed fix
- docker compose -f examples/walkthrough/docker-compose.yml ps - if docker compose -f examples/walkthrough/docker-compose.yml ps | grep -E "Exit|exited"; then + docker compose -f examples/walkthrough/docker-compose.yml ps --all + if docker compose -f examples/walkthrough/docker-compose.yml ps --all --status=exited \ + --filter "name=events" | grep -vE "lakekeeper-migrate" | grep -qE "Exit|exited"; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci-walkthrough.yml around lines 54 - 63, The exited-container check in the CI walkthrough is ineffective because `docker compose ps` only shows running containers by default, so the `grep` in the service-status step can never detect crashed services. Update the check in the workflow step that uses `docker compose -f examples/walkthrough/docker-compose.yml ps` to include exited containers, and then refine the filter so intentionally one-shot services like `lakekeeper-migrate` are excluded from failure detection. Keep the existing `docker compose ... logs` and failure path, but make the status check reliable for actual crashes.
13-23: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winValidate
examples/walkthrough/docker-compose.mesh.ymltoo. TheValidate Docker Composestep only checksexamples/walkthrough/docker-compose.yml, so the distributed demo compose file can drift without CI catching it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci-walkthrough.yml around lines 13 - 23, The Validate Docker Compose step only checks the main walkthrough compose file, so update the workflow’s validate job to also run docker compose config for examples/walkthrough/docker-compose.mesh.yml. Use the existing Validate Docker Compose step in ci-walkthrough and extend it so both compose files are validated in CI, preventing the mesh demo config from drifting unnoticed.
1-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
packages: readto the walkthrough job
.github/workflows/ci-walkthrough.yml:24-36needs explicit package scope for the GHCR login step; addpermissions: packages: readso the base-image pull keeps working under restricted default token settings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci-walkthrough.yml around lines 1 - 12, The walkthrough workflow is missing explicit package read permissions, which can break the GHCR base-image pull under restricted token defaults. Update the ci-walkthrough workflow by adding a permissions block with packages: read for the walkthrough job so the GHCR login step continues to work; use the existing workflow/job definition in ci-walkthrough.yml to place the change alongside the job configuration.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.devcontainer/post-create.sh:
- Around line 6-16: The provisioning script is missing the Linux networking
dependency needed by the walkthrough prereq check. Update the post-create flow
in the .devcontainer/post-create.sh script where it installs packages and runs
examples/walkthrough/setup.sh to also install iproute2 alongside
postgresql-client and jq, matching the dependency expected by setup.sh and the
CI walkthrough setup.
---
Outside diff comments:
In @.github/workflows/ci-walkthrough.yml:
- Around line 54-63: The exited-container check in the CI walkthrough is
ineffective because `docker compose ps` only shows running containers by
default, so the `grep` in the service-status step can never detect crashed
services. Update the check in the workflow step that uses `docker compose -f
examples/walkthrough/docker-compose.yml ps` to include exited containers, and
then refine the filter so intentionally one-shot services like
`lakekeeper-migrate` are excluded from failure detection. Keep the existing
`docker compose ... logs` and failure path, but make the status check reliable
for actual crashes.
- Around line 13-23: The Validate Docker Compose step only checks the main
walkthrough compose file, so update the workflow’s validate job to also run
docker compose config for examples/walkthrough/docker-compose.mesh.yml. Use the
existing Validate Docker Compose step in ci-walkthrough and extend it so both
compose files are validated in CI, preventing the mesh demo config from drifting
unnoticed.
- Around line 1-12: The walkthrough workflow is missing explicit package read
permissions, which can break the GHCR base-image pull under restricted token
defaults. Update the ci-walkthrough workflow by adding a permissions block with
packages: read for the walkthrough job so the GHCR login step continues to work;
use the existing workflow/job definition in ci-walkthrough.yml to place the
change alongside the job configuration.
In `@docs/walkthrough.md`:
- Around line 32-39: The prerequisites section is missing jq, which is required
by examples/walkthrough/setup.sh via REQUIRED_CMDS. Update the walkthrough
prerequisites list to include jq alongside curl, bash, and psql, and keep the
wording aligned with the setup.sh dependency check so readers don’t miss the
needed host tools.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2aec7f1c-f59a-4fc9-aa1c-0c070d453e71
📒 Files selected for processing (5)
.devcontainer/devcontainer.json.devcontainer/post-create.sh.github/workflows/ci-walkthrough.ymldocs/walkthrough.mdexamples/walkthrough/README.md
✅ Files skipped from review due to trivial changes (1)
- examples/walkthrough/README.md
…healthcheck, CI-check, and doc fixes
|
Pushed fixes to the branch (
These are based on current |
…ions, validate mesh compose
|
@coderabbitai Review #38 (review) is addressed as of 4ea5b87:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
git fetch origin 4ea5b87 --depth=1 2>/dev/null || true
git checkout 4ea5b87 2>/dev/null || true
echo "=== post-create.sh ==="
cat .devcontainer/post-create.sh
echo "=== ci-walkthrough.yml permissions ==="
grep -n "permissions" -A3 .github/workflows/ci-walkthrough.yml
echo "=== validate step compose files ==="
grep -n "compose" .github/workflows/ci-walkthrough.yml | head -30
echo "=== exited-check ps -a ==="
grep -n "ps -a\|exited" .github/workflows/ci-walkthrough.yml
echo "=== setup.sh REQUIRED_CMDS ==="
grep -n "REQUIRED_CMDS" -A3 examples/walkthrough/setup.shLength of output: 2876
Thanks for the thorough follow-up — this all looks good. (⌐■_■)ᕗ |
|
@coderabbitai are you happy with the latest fixes to the PR? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Current HEAD ==='
git log -1 --format='%H%n%s'
echo '=== Commits after 4ea5b87 (if available) ==='
git log --oneline 4ea5b87..HEAD 2>/dev/null || true
echo '=== Changed files vs 4ea5b87 ==='
git diff --name-status 4ea5b87..HEAD 2>/dev/null || true
echo '=== Relevant current file outlines ==='
for f in \
.devcontainer/post-create.sh \
.github/workflows/ci-walkthrough.yml \
examples/walkthrough/setup.sh \
examples/walkthrough/install.sh \
examples/walkthrough/guide.sh \
docs/walkthrough.md
do
if [ -f "$f" ]; then
echo "--- $f ---"
case "$f" in
*.sh) ast-grep outline "$f" --items all || true ;;
*) sed -n '1,100p' "$f" ;;
esac
fi
done
echo '=== Targeted safety/consistency references ==='
rg -n -C 2 \
'iproute2|permissions:|packages: read|contents: read|docker-compose\.mesh\.yml|ps -a|lakekeeper-migrate|REQUIRED_CMDS|NONINTERACTIVE|partition_config|--no-deps' \
.devcontainer .github examples/walkthrough docs/walkthrough.md \
|| trueLength of output: 45140 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Full patch after verified commit ==='
git diff --find-renames --unified=80 4ea5b87..HEAD -- \
docs/walkthrough.md \
examples/walkthrough/guide.sh
echo '=== Demo 4 orchestration map ==='
ast-grep outline examples/walkthrough/guide.sh --items all | \
grep -E -C 3 'ensure_mesh_stack|form_mesh|demo_distributed|mesh|subscription' || true
echo '=== Focused Demo 4 implementation context ==='
rg -n -C 10 \
'^(ensure_mesh_stack|form_mesh|demo_distributed)\s*\(\)|spock\.(create_node|create_subscription|sub_wait_for_sync|repset_add_table)|_ensure_claims_replicated' \
examples/walkthrough/guide.sh docs/walkthrough.mdLength of output: 61056
I rechecked the changes since
No further concerns from this review. 🐇✅ |
What this adds
An interactive, self-contained ColdFront walkthrough — a guided demo that
doubles as a manual test harness — plus its user docs and a Codespaces
devcontainer.
examples/walkthrough/guide.sh(+runner.sh): interactive guide with four demosvisibility + concurrent cold-write serialization (the R-A bakery), no 409s
examples/walkthrough/docker-compose.mesh.yml— on-demand 2-node mesh stack forthe Distributed demo (single↔mesh stack switch; a laptop can't hold both)
docs/walkthrough.md— the published, copy-pasteable version of the guide.devcontainer/walkthrough/— Codespaces path (auto-launches the guide)Merges current
main, which the Distributed demo relies on — notably#36 (node-local advisory lock; makes multi-writer-per-node cold writes
conflict-free), plus #31 (lakekeeper healthcheck) and #34.
Testing
Local, on this machine (all green): host preflights (gofmt, go vet,
golangci-lint, unit tests, build, compactor module),
mkdocs build --strict,shellcheck -xon the guide, and a realdemo_distributedNONINTERACTIVEsmoke — 5 concurrent cold writers per node across both nodes, 10/10 landed,
0 conflicts, receipts ledger showing both nodes issuing/acking.
The
ci/matrix.shintegration cell can't run on Docker Desktop for Mac(the harness reaches Lakekeeper/SeaweedFS by container IP, which the Mac host
can't route to), so integration is left to GitHub Actions here. A separate
follow-up will make
run-ci-local.shwork on macOS (publish LK/SW host ports +curl
localhost).Notes
against the branch.