-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
DDB: regenerate snapshots for WarmThroughput #13257
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.
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
Conversation
This reverts commit 6c8d6fa.
Test Results - Alternative Providers676 tests 416 ✅ 33m 49s ⏱️ Results for commit ef6db2a. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 39m 4s ⏱️ Results for commit ef6db2a. ♻️ This comment has been updated with latest results. |
joe4dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for unblocking the pipeline @bentsku and going the extra mile to update more snapshots and snapshot skips 👏
| def check_esm_active(): | ||
| return aws_client.lambda_.get_event_source_mapping(UUID=uuid)["State"] != "Creating" | ||
|
|
||
| assert wait_until(check_esm_active) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Any particular reason why we cannot use _await_event_source_mapping_enabled here?
| ) | ||
| @markers.aws.validated | ||
| def test_event_source_mapping_lifecycle( | ||
| def test_event_source_mapping_lifecycle_delete_function( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ I didn't fully understand these changes in lifecycle and lifecycle_delete_function (might be a git diff issue) 🤔 It kinda looks like swapping the order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is really weird! I haven't touched this code at all, looks like a git diff issue! Same for the question/comment above, I haven't touched it and it must be the old code moving up. Very weird indeed
Motivation
Follow up from #13235, the test selection didn't pick up some Lambda tests and CloudFormation.
Regenerated the snapshots, took the opportunity to remove some old skips.
DynamoDB isn't listed as a hard dependency of Lambda, only DynamoDB Streams. As we don't want to string along all dependencies of dependencies in our test selection, this is somewhat expected behavior. As it seems DynamoDB is actually quite heavily tied to Lambda, maybe we should declare it as an optional dependency (this is a different dependency than say SNS and SQS). In order to quickly fix the issue, this PR does not consider the root cause and I'll look into a follow-up PR to make sure this doesn't happen again (at least on the lambda side, CFN is a different, known thing)
Changes