Skip to content

Update dynamodb snapshot to unblock pipeline #9910

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.

Sign up for GitHub

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

Jump to bottom
Merged
dominikschubert merged 2 commits into from
Dec 19, 2023
Merged

Update dynamodb snapshot to unblock pipeline #9910

dominikschubert merged 2 commits into from
Dec 19, 2023

Conversation

@dominikschubert
Copy link
Member

@dominikschubert dominikschubert commented Dec 19, 2023 •
edited
Loading

Motivation

We've started seeing repeatable failures in CI due to a snapshot mismatch for tests/aws/services/cloudformation/resources/test_lambda.py::TestCfnLambdaIntegrations::test_cfn_lambda_dynamodb_source.

=================================== FAILURES ===================================
__________ TestCfnLambdaIntegrations.test_cfn_lambda_dynamodb_source ___________
>> match key: describe_table_result
	(+)  /Table/DeletionProtectionEnabled ( False )

My investigation pointed towards a new update of DynamoDB local as the culprit (AWS recently released a 2.2.0 version with support for Table delete protection)

Was running into some issues with the initial PR in #9909

I still want to tackle the problem with a more long-term solution to avoid these unpredictable dependency update but for now this should unblock the pipeline.

Changes

  • Update snapshot for tests/aws/services/cloudformation/resources/test_lambda.py::TestCfnLambdaIntegrations::test_cfn_lambda_dynamodb_source

@dominikschubert dominikschubert added the semver: patch Non-breaking changes which can be included in patch releases label Dec 19, 2023
@dominikschubert dominikschubert self-assigned this Dec 19, 2023
This was referenced Dec 19, 2023
Merged
Merged
Merged
simonrw
simonrw approved these changes Dec 19, 2023
View reviewed changes
Copy link
Contributor

@simonrw simonrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a fair change for now. I appreciate you trying to resolve the problem in a more complete way, but with this change we can implement the "proper" solution in our own time.

@github-actions
Copy link

github-actions bot commented Dec 19, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 11m 10s ⏱️
2 410 tests 2 182 ✔️ 228 💤 0 ❌
2 411 runs  2 182 ✔️ 229 💤 0 ❌

Results for commit 589f546.

viren-nadkarni
viren-nadkarni approved these changes Dec 19, 2023
View reviewed changes
localstack/services/dynamodb/server.py
log_listener=_log_listener,
auto_restart=True,
name="dynamodb-local",
env_vars={"DDB_LOCAL_TELEMETRY": "0"},
Copy link
Member

@viren-nadkarni viren-nadkarni Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one 👍

@dominikschubert dominikschubert merged commit 6c8df9c into master Dec 19, 2023
@dominikschubert dominikschubert deleted the hotfix-dynamodb-snapshot-update branch December 19, 2023 11:51
@alexrashed alexrashed mentioned this pull request Dec 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simonrw simonrw simonrw approved these changes

@viren-nadkarni viren-nadkarni viren-nadkarni approved these changes

@pinzon pinzon Awaiting requested review from pinzon

@giograno giograno Awaiting requested review from giograno

Assignees

@dominikschubert dominikschubert

Labels

semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dominikschubert @simonrw @viren-nadkarni