Conversation
WalkthroughUpdated the default LocalStack version from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
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📝 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 |
d001385 to
5d17b36
Compare
Here we go:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cratedb_toolkit/testing/testcontainers/localstack.py`:
- Line 36: The fallback LOCALSTACK_VERSION constant currently set to "2026.3"
(used by LocalStackContainerWithKeepalive()) may be too new for some workflows
(kinesis.yml lacks a version matrix); either change the default to a
known-stable version used in your test matrix (e.g., revert LOCALSTACK_VERSION
to the previous stable tag) or add an explicit localstack-version matrix entry
to kinesis.yml so callers won’t implicitly use 2026.3; update the environment
default in the LOCALSTACK_VERSION declaration or add the matrix in kinesis.yml
to make the version selection explicit.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f64f0038-5c6c-4eae-ad7d-a3362757220b
📒 Files selected for processing (2)
.github/workflows/dynamodb.ymlcratedb_toolkit/testing/testcontainers/localstack.py
| """ | ||
|
|
||
| LOCALSTACK_VERSION = os.environ.get("LOCALSTACK_VERSION", "4.14") | ||
| LOCALSTACK_VERSION = os.environ.get("LOCALSTACK_VERSION", "2026.3") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== LocalStack container call sites =="
ast-grep --pattern 'LocalStackContainerWithKeepalive($$$)' || true
echo
echo "== Workflow entries that explicitly set LocalStack version =="
rg -n -C2 'LOCALSTACK_VERSION|localstack-version' .github/workflows || true
echo
echo "== Current fallback definition =="
rg -n -C2 'LOCALSTACK_VERSION = os\.environ\.get\("LOCALSTACK_VERSION"' cratedb_toolkit/testing/testcontainers/localstack.pyRepository: crate/cratedb-toolkit
Length of output: 1440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Recent commits affecting localstack.py =="
git log --oneline -20 -- cratedb_toolkit/testing/testcontainers/localstack.py 2>/dev/null || echo "No git history available"
echo
echo "== Full context of line 36 with nearby lines =="
cat -n cratedb_toolkit/testing/testcontainers/localstack.py | sed -n '30,45p'
echo
echo "== Search for exit code 55 in logs or issues =="
rg -i "exit.*55|code.*55" --type-list | head -20 || true
echo
echo "== Check all workflow files for LocalStack references =="
fd -e yml -e yaml .github/workflows | xargs grep -l "localstack\|LocalStack" || true
echo
echo "== Check if there are other conftest files using LocalStackContainerWithKeepalive =="
rg -l "LocalStackContainerWithKeepalive" tests/Repository: crate/cratedb-toolkit
Length of output: 1844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Git diff for recent localstack.py commit =="
git show 5d17b36 -- cratedb_toolkit/testing/testcontainers/localstack.py | head -100
echo
echo "== Check previous version of line 36 =="
git show 5d17b36^:cratedb_toolkit/testing/testcontainers/localstack.py | sed -n '30,40p' || echo "Previous version not available"
echo
echo "== Search workflow files in .github/workflows =="
find .github/workflows -type f \( -name "*.yml" -o -name "*.yaml" \) -exec grep -l "localstack\|LocalStack" {} \;
echo
echo "== Content of kinesis workflow (if exists) =="
if [ -f ".github/workflows/kinesis.yml" ]; then grep -C3 -i "localstack" ".github/workflows/kinesis.yml" || echo "No LocalStack refs"; else echo "kinesis.yml not found"; fi
echo
echo "== Look for any CI test failures in comments or docs =="
rg -i "exit.*55|container.*exit|failed.*local" docs/ README* .github/ 2>/dev/null | head -20 || trueRepository: crate/cratedb-toolkit
Length of output: 3685
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Content of kinesis.yml workflow =="
cat .github/workflows/kinesis.yml
echo
echo "== All files in cratedb_toolkit/testing/testcontainers/ =="
ls -la cratedb_toolkit/testing/testcontainers/
echo
echo "== Check git log to see if localstack.py existed before =="
git log --all --oneline -- cratedb_toolkit/testing/testcontainers/localstack.py | head -10Repository: crate/cratedb-toolkit
Length of output: 3505
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if LocalStack was previously imported/configured elsewhere =="
git log --all -p --follow -S "LocalStack" -- "**/*.py" | grep -A5 -B5 "LOCALSTACK_VERSION\|LocalStackContainer" | head -100
echo
echo "== Look at conftest.py files to see how they configured LocalStack before =="
git show c83a622:tests/io/kinesis/conftest.py 2>/dev/null | head -60 || echo "File doesn't exist in that commit"
echo
echo "== Check commit c83a622 (race condition fix) details =="
git show c83a622 --stat
echo
echo "== Check previous conftest.py before new localstack.py file =="
git show fe406b6:tests/io/kinesis/conftest.py 2>/dev/null | head -60 || echo "Not available"Repository: crate/cratedb-toolkit
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Commit message for c83a622 (race condition fix) =="
git log --format=fuller c83a622 -1
echo
echo "== Commit message for fe406b6 (Update to LocalStack 4.14) =="
git log --format=fuller fe406b6 -1
echo
echo "== Check kinesis conftest.py current state =="
cat -n tests/io/kinesis/conftest.py
echo
echo "== Check dynamodb conftest.py current state =="
cat -n tests/io/dynamodb/conftest.pyRepository: crate/cratedb-toolkit
Length of output: 8518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if kinesis.yml ever had a LOCALSTACK_VERSION matrix =="
git log --all -p -- .github/workflows/kinesis.yml | grep -A20 -B5 "LOCALSTACK_VERSION\|localstack-version" | head -80
echo
echo "== Check if there are any TODO or FIXME comments in localstack.py =="
grep -n "TODO\|FIXME\|XXX" cratedb_toolkit/testing/testcontainers/localstack.py
echo
echo "== Verify the state of dynamodb.yml =="
cat .github/workflows/dynamodb.yml | grep -A30 "matrix:"Repository: crate/cratedb-toolkit
Length of output: 128
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full dynamodb.yml matrix section =="
cat .github/workflows/dynamodb.yml | sed -n '/matrix:/,/env:/p'
echo
echo "== Check full kinesis.yml for any environment variables or version setup =="
cat .github/workflows/kinesis.yml | grep -A50 "jobs:" | grep -A40 "tests:"
echo
echo "== Look for any issue or discussion about LocalStack 2026.3 =="
rg -i "2026\.3|exit.*55" . --type markdown --type text 2>/dev/null | head -20 || true
echo
echo "== Verify the exact git status of localstack.py - was it created or modified =="
git show --stat 5d17b36 | grep localstack.pyRepository: crate/cratedb-toolkit
Length of output: 1528
Reconsider the fallback version for unmatrixe d workflows.
This new file introduces 2026.3 as the default fallback for all callers of LocalStackContainerWithKeepalive() that don't set LOCALSTACK_VERSION. While dynamodb.yml has a test matrix to override this, kinesis.yml has no version matrix and will implicitly use 2026.3 for all runs. Given the recent race condition fixes in LocalStack readiness handling (commit c83a622), consider using a more battle-tested version as the fallback, or add an explicit matrix to kinesis.yml.
Suggested approach
Either:
- Revert the fallback to a stable version known to work with the test suite:
- LOCALSTACK_VERSION = os.environ.get("LOCALSTACK_VERSION", "2026.3")
+ LOCALSTACK_VERSION = os.environ.get("LOCALSTACK_VERSION", "4.14")- Or, add a localstack-version matrix to
kinesis.ymlsimilar todynamodb.ymlto make the version choice explicit.
📝 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.
| LOCALSTACK_VERSION = os.environ.get("LOCALSTACK_VERSION", "2026.3") | |
| LOCALSTACK_VERSION = os.environ.get("LOCALSTACK_VERSION", "4.14") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cratedb_toolkit/testing/testcontainers/localstack.py` at line 36, The
fallback LOCALSTACK_VERSION constant currently set to "2026.3" (used by
LocalStackContainerWithKeepalive()) may be too new for some workflows
(kinesis.yml lacks a version matrix); either change the default to a
known-stable version used in your test matrix (e.g., revert LOCALSTACK_VERSION
to the previous stable tag) or add an explicit localstack-version matrix entry
to kinesis.yml so callers won’t implicitly use 2026.3; update the environment
default in the LOCALSTACK_VERSION declaration or add the matrix in kinesis.yml
to make the version selection explicit.
About
Just a version bump, following LocalStack's announcement Project Update: Consolidation into the Unified LocalStack Image:
Evaluation
Updating to LocalStack NG does not work out of the box for integration testing the DynamoDB and Kinesis data I/O nozzles. It is possible we need a newer localstack Python package, but possibly, the testcontainers package is not ready yet.
/cc @hampsterx 🌻