Skip to content

DynamoDB: fix GlobalSecondaryIndexes parity in Create and DescribeTable #12119

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
merged 3 commits into from
Jan 10, 2025
Merged

DynamoDB: fix GlobalSecondaryIndexes parity in Create and DescribeTable #12119

merged 3 commits into from
Jan 10, 2025

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Jan 9, 2025 •
edited
Loading

Motivation

As reported in #12014, we've had a regression in our handling of GlobalSecondaryIndexes, probably due to the update of DDB-local from v1 to v2.

This really broke some users setup with Terraform, causing it to try to redeploy and fail.

I confirmed that with this fix, deploying the Terraform file and trying to applying it again do not fail anymore.

fixes #12014

Changes

  • add test regarding the behavior of GlobalSecondaryIndexes depending on the BillingMode
  • fix the parity of both CreateTable and DescribeTable by adding the missing fields to the responses

@bentsku bentsku added aws:dynamodb Amazon DynamoDB semver: patch Non-breaking changes which can be included in patch releases labels Jan 9, 2025
@bentsku bentsku added this to the 4.1 milestone Jan 9, 2025
@bentsku bentsku self-assigned this Jan 9, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   28m 38s ⏱️ - 1h 24m 22s
1 079 tests  - 2 870  1 020 ✅  - 2 620  59 💤  - 250  0 ❌ ±0 
1 081 runs   - 2 870  1 020 ✅  - 2 620  61 💤  - 250  0 ❌ ±0 

Results for commit 34d894e. ± Comparison against base commit 630b2a1.

This pull request removes 2872 and adds 2 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.dynamodb.test_dynamodb.TestDynamoDB ‑ test_gsi_with_billing_mode[PAY_PER_REQUEST]
tests.aws.services.dynamodb.test_dynamodb.TestDynamoDB ‑ test_gsi_with_billing_mode[PROVISIONED]

♻️ This comment has been updated with latest results.

giograno
giograno approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@giograno giograno left a comment

Choose a reason for hiding this comment

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

Nice fix 👍 ! LGMT ✔️

@bentsku @giograno
remove leftover comment
fff9c86 
Co-authored-by: Giovanni Grano <me@giograno.com>
@bentsku bentsku merged commit 9f208af into master Jan 10, 2025
11 checks passed
@bentsku bentsku deleted the fix-ddb-gsi branch January 10, 2025 22:57
@nicolaschambrier
Copy link

Do you know when it will be included in 4.x?

@bentsku
Copy link
Contributor Author

bentsku commented Jan 21, 2025

Hello @nicolaschambrier, we have a minor release planned for the end of January (tentative 30th of January), so it will be available in a tagged release by then. Hopefully that works for you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giograno giograno giograno approved these changes

@viren-nadkarni viren-nadkarni Awaiting requested review from viren-nadkarni

Assignees

@bentsku bentsku

Labels
aws:dynamodb Amazon DynamoDB semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.

bug: DynamoDB - using terraform on table with on-demand capacity + GSI behaves oddly
3 participants
@bentsku @nicolaschambrier @giograno