🐛 use LocalStack legacy CloudFormation engine#85
Merged
Conversation
- Fix AggregatorDLQAlarmName output to use DeployAggregatorAlarms condition instead of DeployAggregator (fixes template error when aggregator enabled but alarms disabled) - Update CLAUDE.md with LocalStack Docker socket requirement - Add pytest markers for aws and e2e tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…#81) Add test_stack_create_and_delete_minimal to document and verify the LocalStack CloudFormation v2 engine bug where stack deletion fails with: "Template format error: Unresolved resource dependencies [AggregatorDLQ]" Root cause: LocalStack's new CloudFormation v2 engine incorrectly tries to resolve !GetAtt references in conditional resources during deletion, even when those resources were never created (condition evaluated to false). Workarounds: 1. Use legacy engine: PROVIDER_OVERRIDE_CLOUDFORMATION=engine-legacy 2. Wrap delete_stack() in try/except and delete resources directly The test: - Creates a minimal stack (no aggregator, no alarms) - Attempts deletion (expected to fail on LocalStack v2 engine) - Uses xfail to document the known issue - Falls back to direct DynamoDB table deletion for cleanup Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
LocalStack's CloudFormation v2 engine has a bug where stack deletion fails with "Unresolved resource dependencies [AggregatorDLQ]" due to incorrect resolution of !GetAtt references in conditional resources. Workaround: Use PROVIDER_OVERRIDE_CLOUDFORMATION=engine-legacy Note: Legacy engine has its own bug where CloudWatch Alarm Threshold parameters are passed as strings. Tests use aggregator_stack_options (no alarms) to avoid this issue. Updated: - CLAUDE.md documentation - docs/infra/localstack.md - .github/workflows/ci.yml (integration and benchmark jobs) - tests/test_integration_localstack.py docstring and cleanup - tests/test_stack_manager.py (expect success, not xfail) - examples/fastapi-demo/docker-compose.yml - tests/fixtures/localstack-v2-bug-repro.yaml (minimal reproduction) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 task
Contributor
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.
| Benchmark suite | Current: 05d83ea | Previous: 7e8b69a | Ratio |
|---|---|---|---|
tests/test_performance.py::TestConcurrentThroughputBenchmarks::test_sequential_acquisitions |
4.970156682205006 iter/sec (stddev: 0.11962265277986306) |
6.652134556790673 iter/sec (stddev: 0.08528911949683002) |
1.34 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #85 +/- ##
==========================================
+ Coverage 87.17% 87.37% +0.20%
==========================================
Files 18 18
Lines 1933 1933
==========================================
+ Hits 1685 1689 +4
+ Misses 248 244 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
sodre
added a commit
that referenced
this pull request
Jan 12, 2026
This reverts commit 1bbc649. Prefer using LocalStack's v2 CloudFormation engine (the default) rather than the legacy engine. While v2 has a stack deletion bug with conditional resources, the legacy engine has its own bug with CloudWatch Alarm Threshold parameters. The v2 engine is the future of LocalStack CloudFormation support, and we should track the upstream bug rather than permanently work around it. Upstream issue: localstack/localstack#13609 Re-opens #81 for tracking until LocalStack fixes the v2 engine. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This was referenced Jan 12, 2026
sodre
added a commit
that referenced
this pull request
Jan 12, 2026
This reverts commit 1bbc649. Prefer using LocalStack's v2 CloudFormation engine (the default) rather than the legacy engine. While v2 has a stack deletion bug with conditional resources, the legacy engine has its own bug with CloudWatch Alarm Threshold parameters. The v2 engine is the future of LocalStack CloudFormation support, and we should track the upstream bug rather than permanently work around it. Upstream issue: localstack/localstack#13609 Re-opens #81 for tracking until LocalStack fixes the v2 engine. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sodre
added a commit
that referenced
this pull request
Jan 12, 2026
…#86) * ⏪ revert: undo LocalStack legacy CloudFormation engine workaround (#85) This reverts commit 1bbc649. Prefer using LocalStack's v2 CloudFormation engine (the default) rather than the legacy engine. While v2 has a stack deletion bug with conditional resources, the legacy engine has its own bug with CloudWatch Alarm Threshold parameters. The v2 engine is the future of LocalStack CloudFormation support, and we should track the upstream bug rather than permanently work around it. Upstream issue: localstack/localstack#13609 Re-opens #81 for tracking until LocalStack fixes the v2 engine. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * 🐛 fix(infra): address code review feedback for PR #86 1. Fix CloudFormation condition mismatch: AggregatorDLQAlarmName output now uses DeployAggregatorAlarms condition (not DeployAggregator) to match the resource it references. 2. Add test_cloudformation_stack_deployment_no_alarms: Tests the edge case where EnableAggregator=true but EnableAlarms=false. 3. Add test_stack_create_and_delete_minimal: Integration test for full stack lifecycle with graceful handling of LocalStack v2 deletion bug. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * 📝 docs: remove references to non-existent e2e pytest marker The e2e marker was removed by the revert. Update CLAUDE.md to reflect that integration tests now include full stack lifecycle tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PROVIDER_OVERRIDE_CLOUDFORMATION=engine-legacyto work around LocalStack v2 engine stack deletion bugProblem
LocalStack's CloudFormation v2 engine has a bug where stack deletion fails with:
This occurs when:
Condition:attribute (conditionally created)!GetAttinside an!Iffalseduring creation (resource was NOT created)The v2 engine incorrectly tries to resolve
!GetAttreferences during deletion even for resources that were never created.Solution
Use the legacy CloudFormation engine via
PROVIDER_OVERRIDE_CLOUDFORMATION=engine-legacy. This is a workaround until LocalStack fixes the v2 engine bug upstream.Note: The legacy engine has its own bug where CloudWatch Alarm Threshold parameters are passed as strings instead of numbers. Tests use
aggregator_stack_options(no alarms) to avoid this issue.Changes
.github/workflows/ci.yml- Added legacy engine to integration and benchmark jobsCLAUDE.md- Updated LocalStack Docker commands with legacy enginedocs/infra/localstack.md- Updated examples and added warning calloutexamples/fastapi-demo/docker-compose.yml- Added legacy engine env vartests/test_integration_localstack.py- Updated docstring, fixed cleanuptests/test_stack_manager.py- Updated test to expect success (not xfail)tests/fixtures/localstack-v2-bug-repro.yaml- Minimal reproduction templateTest plan
pytest tests/ -v --ignore=tests/test_performance.py(375 passed)pytest -m integration -v(23 passed)Closes #81