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.

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

DynamoDB billing mode support #753

Merged
merged 17 commits into from
Oct 15, 2021
Merged

DynamoDB billing mode support #753

merged 17 commits into from
Oct 15, 2021

Conversation

marknet15
Copy link
Contributor

@marknet15 marknet15 commented Oct 11, 2021 •
edited
Loading

SUMMARY

Current the dynamodb_table module only supports provisioned table creations, this adds support for defining a billing_mode to create either pay_per_request or provisioned tables.

https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb.html#DynamoDB.Client.create_table

This change does also allow for changing the billing_mode on existing tables.

fixes: #609

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

dynamodb_table

ADDITIONAL INFORMATION

Example of creating a DynamoDB table with pay_per_request:

- name: Create a pay-per-request table
  community.aws.dynamodb_table:
    name: my-table
    region: eu-west-1
    hash_key_name: id
    hash_key_type: STRING
    billing_mode: PAY_PER_REQUEST

@tremble
Copy link
Contributor

tremble commented Oct 11, 2021

Thanks for following up here @marknet15

@tremble tremble linked an issue Oct 11, 2021 that may be closed by this pull request
Create PAY_PER_REQUEST dynamodb table #609
Closed
@ansibullbot
Copy link

cc @jillr @loia @markuman @s-hertel @wimnat
click here for bot help

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request integration tests/integration module module plugins plugin (any type) tests tests labels Oct 11, 2021
@tremble
Copy link
Contributor

tremble commented Oct 11, 2021

Is it possible to change from provisioned to per-request?

@marknet15
Copy link
Contributor Author

@tremble I think it is yes, I'll test it out locally and extend the tests:
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/dynamodb.html#DynamoDB.Client.update_table

@marknet15 marknet15 marked this pull request as ready for review October 12, 2021 10:57
@marknet15
Copy link
Contributor Author

recheck

tremble
tremble reviewed Oct 12, 2021
View reviewed changes
plugins/modules/dynamodb_table.py Outdated
description:
- Controls whether provisoned pr on-demand tables are created.
choices: ['PROVISIONED', 'PAY_PER_REQUEST']
default: 'PROVISIONED'
Copy link
Contributor

Choose a reason for hiding this comment

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

My instincts tell me that setting a default here, and relying on it, is going to result in weird things happening when people try to make minimal changes to a Pay-Per-Request cluster if they don't specify billing_mode. For example they try to add a secondary index and the whole cluster changes from provisioned to pay-per-request.

It might be better to see if this can be read from the current table (except when creating a table)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll re-work things a bit as that makes sense yes 👍🏻

@tremble
Copy link
Contributor

tremble commented Oct 13, 2021

recheck

@marknet15
Copy link
Contributor Author

@tremble when you get a moment would you be able to re-review ?

tremble
tremble approved these changes Oct 15, 2021
View reviewed changes
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Thanks @marknet15, LGTM.

@tremble tremble added the gate label Oct 15, 2021
ansible-zuul[bot]
ansible-zuul bot approved these changes Oct 15, 2021
View reviewed changes
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

@ansible-zuul ansible-zuul bot merged commit dd2cb26 into ansible-collections:main Oct 15, 2021
@nick-zh
Copy link

nick-zh commented Oct 16, 2021

Many thanks everyone, awesome 🎉

@marknet15 marknet15 mentioned this pull request Oct 16, 2021
Merged
ansible-zuul bot pushed a commit that referenced this pull request Oct 16, 2021
@marknet15
Add missed changelog fragment for dynamodb (#766)
0367fb5 
Add missed changelog fragment for dynamodb

SUMMARY
Add missed change_log fragment for #753
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
dynamodb_table

Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
@nick-zh
Copy link

nick-zh commented Nov 4, 2021

@marknet15 @tremble is another tag planned soon, so i can use this feature in the galaxy collection? 🤩

@marknet15
Copy link
Contributor Author

@nick-zh version 2.1 of the collection was released today 🚀

@nick-zh
Copy link

nick-zh commented Nov 14, 2021

Works like a charm, thx again 🎉

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

@tremble tremble tremble approved these changes

@ansible-zuul Ansible Zuul Ansible Zuul approved these changes

@alinabuzachis alinabuzachis Awaiting requested review from alinabuzachis

Assignees
No one assigned
Labels
community_review feature This issue/PR relates to a feature request integration tests/integration module module plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create PAY_PER_REQUEST dynamodb table
4 participants
@marknet15 @tremble @ansibullbot @nick-zh