Skip to content

Conversation

pinzon
Copy link
Member

@pinzon pinzon commented Apr 18, 2023

Conditional attributes are not completely/correctly resolved during the execution of add_defaults method of a GenericBaeseModel. The current patch allows to have a DynamoDB table that has multiple conditional attributes to be deployed.

There's more work required to completely fix the issue but with these small changes the amplify-localstack plugin can finally deploy a API stack to LocalStack.

Changes:

  • Test with a small example of what amplify tries to deploy.
  • Move the addition of default parameters to another phase where conditionals are resolved.

@pinzon pinzon changed the title Fix passing attributes as conditionals fix default ProvisionedTrhoughput in DynamoDB Apr 19, 2023
@pinzon pinzon marked this pull request as ready for review April 19, 2023 19:40
@pinzon pinzon requested a review from dominikschubert as a code owner April 19, 2023 19:40
@github-actions
Copy link

LocalStack Community integration with Pro

1 913 tests   1 707 ✔️  1h 12m 30s ⏱️
       2 suites     206 💤
       2 files           0

Results for commit 02e4dca.

@whummer whummer changed the title fix default ProvisionedTrhoughput in DynamoDB fix default ProvisionedThroughput in DynamoDB Apr 19, 2023
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

Let's unblock amplify 🚀

Thanks for adding the integration test as well 👍

]
)
@pytest.mark.parametrize("billing_mode", ["PROVISIONED", "PAY_PER_REQUEST"])
def test_billing_mode_as_conditional(deploy_cfn_template, snapshot, aws_client, billing_mode):
Copy link
Member

Choose a reason for hiding this comment

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

nit: Would be great to have a short comment here explaining the context of why this test was added

@thrau thrau merged commit 65e08f6 into master Apr 21, 2023
@thrau thrau deleted the fix-cfn-dynamodb-conditional branch April 21, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants