DynamoDB: Fix startup in non-default account ID/region#10157
Merged
viren-nadkarni merged 3 commits intomasterfrom Feb 6, 2024
Merged
DynamoDB: Fix startup in non-default account ID/region#10157viren-nadkarni merged 3 commits intomasterfrom
viren-nadkarni merged 3 commits intomasterfrom
Conversation
S3 Image Test Results (AMD64 / ARM64) 2 files 2 suites 3m 16s ⏱️ Results for commit 4da325e. ♻️ This comment has been updated with latest results. |
81ef6a5 to
f4990cf
Compare
To be reverted before merge
f4990cf to
4da325e
Compare
This reverts commit 31dc31c.
giograno
approved these changes
Feb 5, 2024
LocalStack Community integration with Pro 2 files 2 suites 1h 21m 52s ⏱️ Results for commit ab98792. ♻️ This comment has been updated with latest results. |
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
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
It was observed that
test_time_to_live_deletionwould fail in non-default account/region setting (#8204), specifically the second of the below two assertions that it performs:localstack/tests/aws/services/dynamodb/test_dynamodb.py
Lines 127 to 128 in 75042c5
The first assertion would pass with HTTP 200 but for the second, the response value would be
''cause JSON deserialisation error.There were two root problems:
roloworks. By default, if an invalid route is queried at the LocalStack endpoint, it returns an HTTP 200 with an empty response by default. See Invalid routes must return HTTP 404 Not Found #10167 for detailsconnect_externally_to(): This is used when DDB Local is started to do a health check. Lack of explicit credentials meant that the factory would fallback to env values, which we are removing in Use non-default formatted access key ID for tests #8204. Boto client creation would fail, causing the healthcheck to fail, which in turn prevented the custom routes from being registered.localstack/localstack/services/dynamodb/provider.py
Lines 472 to 476 in 75042c5
Changes
000000000000) to theconnect_externally_to()usage. The client is only used to check if DDB returns a valid response, and we don't really care about the actual value.Tests
Changes were tested with non-default credentials, see commit 4da325e, where the
test_time_to_live_deletionpasses 🟢