-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
aws_dynamodb_table: Work around tagging limitation in dynamodb-local #6149
Conversation
aws/resource_aws_dynamodb_table.go
Outdated
|
||
// Do not fail when interfacing with dynamodb-local | ||
if err != nil { | ||
if aerr, ok := err.(awserr.Error); ok { |
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.
Suggest using isAWSErr
if applicable (requires knowing the value of awserr.Error.Code()
).
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.
Updated. Thanks!
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.
Looks great, @stefansundin, thanks! 🚀
--- PASS: TestAccAWSDynamoDbTable_streamSpecificationValidation (1.66s)
--- PASS: TestAccAWSDynamoDbTable_tags (38.77s)
--- PASS: TestAccAWSDynamoDbTable_importTimeToLive (69.31s)
--- PASS: TestAccAWSDynamoDbTable_importTags (97.33s)
--- PASS: TestAccAWSDynamoDbTable_basic (109.78s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdateValidation (1.40s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (125.43s)
--- PASS: TestAccAWSDynamoDbTable_importBasic (140.33s)
--- PASS: TestAccAWSDynamoDbTable_encryption (40.15s)
--- PASS: TestAccAWSDynamoDbTable_ttl (132.50s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateCapacity (204.47s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes (185.93s)
--- PASS: TestAccAWSDynamoDbTable_extended (229.43s)
--- PASS: TestAccAWSDynamoDbTable_enablePitr (293.28s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdate (426.47s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes (530.16s)
Thanks for the quick merge @bflad. Can you check on my other PRs that I linked above? :) I think at least the first one can be merged, but the other two I need help on. |
@stefansundin left a comment on #4488, I'm not sure when I'll have time to properly review #5728, and #5812 might be able to get in when all the other work associated with cutting version 2.0.0 is getting merged in. This has been released in version 1.41.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
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! |
Changes proposed in this pull request:
I have yet to try more complicated things with dynamodb-local.
Here's how I use dynamodb-local:
Terraform:
This would fail with:
I don't think there is a need to work around setting tags at this time. In Terraform 0.12 that can probably be easily done with the new
if
statement.This problem is referenced in this issue (but also other problems): #1059
P.S. Can you please take a second look at some of my other PRs?
Thank you!!