-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Add Support for dynamodb billing #6648
Conversation
886f4b9
to
f2d56e2
Compare
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.
Thanks for submitting this @sunilkumarmohanty! If this PR is no longer work in progress, we should remove the WIP in the title. I merged the dependency update as well so this PR can be rebased.
Left initial comments below. Please let us know if you have any questions or do not have time to implement feedback as we'll likely want to get this in before middle of next week. 👍
…ROVISIONED and other small fixes
dcb6419
to
a4bb354
Compare
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.
When running the updated acceptance testing for the resource, I discovered some additional issues. Please take a look at the below and let us know if you have any questions.
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.
Thanks @sunilkumarmohanty! This should be good for the initial implementation. 🚀
In the future, when the Terraform provider SDK gives us the ability, we can add plan-time validation for read/write capacity if the billing mode is provisioned. 👍
--- PASS: TestAccAWSDynamoDbTable_streamSpecificationValidation (1.63s)
--- PASS: TestAccAWSDynamoDbTable_importTags (50.68s)
--- PASS: TestAccAWSDynamoDbTable_tags (68.57s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode (87.00s)
--- PASS: TestAccAWSDynamoDbTable_basic (110.25s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (125.15s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdateValidation (1.27s)
--- PASS: TestAccAWSDynamoDbTable_importBasic (130.24s)
--- PASS: TestAccAWSDynamoDbTable_importTimeToLive (130.35s)
--- PASS: TestAccAWSDynamoDbTable_ttl (53.10s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateCapacity (178.94s)
--- PASS: TestAccAWSDynamoDbTable_encryption (129.89s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes (251.68s)
--- PASS: TestAccAWSDynamoDbTable_extended (323.92s)
--- PASS: TestAccAWSDynamoDbTable_enablePitr (520.96s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdate (512.32s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes (573.46s)
|
This has been released in version 1.51.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
I would suggest creating a separate issue for making GSI capacities optional. 👍 |
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.
this PR is breaking the ability to terraform dynamodb_table in govcloud (apparently BillingMode is not supported by their implemntation yet)
* aws_dynamodb_table.tfstate: ValidationException: One or more parameter values were invalid: Unsupported input parameter BillingMode
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Fixes #6632
Changes proposed in this pull request: