Skip to content
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

fix(diffDynamoDbGSI): ignore ordering of non_key_attributes in equali… #9988

Merged
merged 3 commits into from
Sep 2, 2020

Conversation

shalka
Copy link

@shalka shalka commented Sep 4, 2019

…ty check to stop forced reconstruction of GSI [fixes ##3828]

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Release note for CHANGELOG:


Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

@shalka shalka requested a review from a team September 4, 2019 18:32
@ghost ghost added size/M Managed by automation to categorize the size of a PR. service/dynamodb Issues and PRs that pertain to the dynamodb service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Sep 4, 2019
@shalka shalka force-pushed the master branch 3 times, most recently from 2de76c9 to 15402af Compare September 17, 2019 21:30
…ty check to stop forced reconstruction of GSI [fixes #hashicorp#3828]
@cakejelly
Copy link

Would love to see this merged. Is there any update?

@twnbay78
Copy link

twnbay78 commented May 7, 2020

Can we please get this reviewed and merged? We are employing some not-so-pretty workarounds for this currently.

@maryelizbeth maryelizbeth linked an issue Aug 17, 2020 that may be closed by this pull request
Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

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

Hi @shalka 👋 Thanks so much for this PR! It's off to a great start 👍 Just a couple moments around the use of a Set when calling the non_key_attributes from the resource data. Please let me know if you have any additional questions or concerns regarding the changes.

Output of acceptance tests

--- FAIL: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes (102.32s) <-- should be fixed up 
with the proposed changes as the update isn't taking place as expected in this test where the # of 
non_key_attributes increases from 1 to 2

--- PASS: TestDiffDynamoDbGSI (0.04s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecificationValidation (14.35s)
--- PASS: TestAccAWSDynamoDbTable_disappears (47.99s)
--- PASS: TestAccAWSDynamoDbTableItem_rangeKey (53.26s)
--- PASS: TestAccAWSDynamoDbTableItem_withMultipleItems (57.53s)
--- PASS: TestAccAWSDynamoDbTableItem_basic (60.85s)
--- PASS: TestAccAWSDynamoDbTable_basic (66.47s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Enabled (61.32s)
--- PASS: TestAccAWSDynamoDbTable_tags (80.00s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdateValidation (23.55s)
--- PASS: TestAccAWSDynamoDbTableItem_updateWithRangeKey (97.82s)
--- PASS: TestAccAWSDynamoDbTableItem_update (98.31s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_PayPerRequestToProvisioned (101.90s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (105.52s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_PayPerRequestToProvisioned (110.05s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateCapacity (112.09s)
--- PASS: TestAccAWSDynamoDbTable_enablePitr (115.27s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Disabled (68.40s)
--- PASS: TestAccAWSDynamoDbTable_disappears_PayPerRequestWithGSI (120.43s)
--- PASS: TestAccAWSDynamoDbTable_encryption (149.06s)
--- PASS: TestAccAWSDynamoDbTable_extended (294.61s)
--- PASS: TestAccAWSDynamoDbTable_Replica_Single (368.92s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdate (550.45s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes (604.70s)
--- PASS: TestAccAWSDynamoDbTable_Replica_Multiple (736.12s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_ProvisionedToPayPerRequest (1339.53s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_ProvisionedToPayPerRequest (1359.85s)

aws/structure.go Outdated Show resolved Hide resolved
aws/resource_aws_dynamodb_table_test.go Show resolved Hide resolved
aws/structure.go Outdated Show resolved Hide resolved
@anGie44 anGie44 self-assigned this Aug 19, 2020
@anGie44 anGie44 added the waiting-response Maintainers are waiting on response from community or contributor. label Aug 19, 2020
@anGie44 anGie44 removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 1, 2020
@anGie44
Copy link
Contributor

anGie44 commented Sep 1, 2020

Hi @shalka 👋 thanks again for this PR! Just dropping a note that I'll be continuing the work here, as our general response time from community PRs is 2 weeks, and hope to get these changes into an upcoming release of the provider :) if you have any questions, please let me know!

@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Sep 1, 2020
@anGie44 anGie44 removed their assignment Sep 1, 2020
@anGie44
Copy link
Contributor

anGie44 commented Sep 1, 2020

Output of acceptance tests:

--- PASS: TestDiffDynamoDbGSI (0.00s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecificationValidation (16.11s)
--- PASS: TestAccAWSDynamoDbTable_disappears (49.66s)
--- PASS: TestAccAWSDynamoDbTableItem_basic (57.19s)
--- PASS: TestAccAWSDynamoDbTableItem_rangeKey (59.28s)
--- PASS: TestAccAWSDynamoDbTableItem_withMultipleItems (62.66s)
--- PASS: TestAccAWSDynamoDbTable_basic (71.37s)
--- PASS: TestAccAWSDynamoDbTable_tags (83.63s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdateValidation (25.00s)
--- PASS: TestAccAWSDynamoDbTableItem_updateWithRangeKey (97.73s)
--- PASS: TestAccAWSDynamoDbTableItem_update (101.41s)
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (113.76s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Enabled (63.72s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_PayPerRequestToProvisioned (114.79s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes_emptyPlan (102.05s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateCapacity (120.00s)
--- PASS: TestAccAWSDynamoDbTable_enablePitr (121.57s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_PayPerRequestToProvisioned (124.68s)
--- PASS: TestAccAWSDynamoDbTable_disappears_PayPerRequestWithGSI (129.02s)
--- PASS: TestAccAWSDynamoDbTable_Ttl_Disabled (73.88s)
--- PASS: TestAccAWSDynamoDbTable_encryption (140.50s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateNonKeyAttributes (303.51s)
--- PASS: TestAccAWSDynamoDbTable_extended (310.73s)
--- PASS: TestAccAWSDynamoDbTable_attributeUpdate (579.53s)
--- PASS: TestAccAWSDynamoDbTable_gsiUpdateOtherAttributes (644.06s)
--- PASS: TestAccAWSDynamoDbTable_Replica_Single (564.01s)
--- PASS: TestAccAWSDynamoDbTable_Replica_Multiple (949.75s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_GSI_ProvisionedToPayPerRequest (1046.09s)
--- PASS: TestAccAWSDynamoDbTable_BillingMode_ProvisionedToPayPerRequest (1049.24s)

Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

This should certainly improve the situation. 🚀

@bflad bflad added the bug Addresses a defect in current functionality. label Sep 2, 2020
@anGie44 anGie44 added this to the v3.5.0 milestone Sep 2, 2020
@anGie44 anGie44 merged commit c49a8f9 into hashicorp:master Sep 2, 2020
anGie44 added a commit that referenced this pull request Sep 2, 2020
@ghost
Copy link

ghost commented Sep 3, 2020

This has been released in version 3.5.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Oct 3, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Oct 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/dynamodb Issues and PRs that pertain to the dynamodb service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No method to ignore changes in DynamoDB GSI
6 participants