search

HENNGE / aiodynamo

Asynchronous, fast, pythonic DynamoDB Client
https://aiodynamo.readthedocs.io/
Other
69 stars 20 forks source link

Add support for pay_per_request billing mode during table creation #95

Closed TheEdgeOfRage closed 2 years ago

TheEdgeOfRage commented 2 years ago

Currently aiodynamo creates tables using the provisioned billing mode. DynamoDB also supports pay_per_request where no throughput config is provided. This PR is backwards compatible, but also adds the support to provide a billing_mode parameter to create_table. The throughput parameter has been changed to optional as the pay_per_request billing mode doesn't use it.

PS. I'd be very grateful if you could also add the hacktoberfest-accepted label to this PR once it is accepted so that it can be counted as a hacktoberfest PR. Thanks :)

TheEdgeOfRage commented 2 years ago

Hmm, I see that Protocol isn't supported in 3.7. I'll have to revert it to the old Callable type hints

dimaqq commented 2 years ago

@ojii do you reckon we could bump package requirements to python^3.8 ?

ojii commented 2 years ago

Thank you for your pull request.

This PR is backwards compatible, but also adds the support to provide a billing_mode parameter to create_table. The throughput parameter has been changed to optional as the pay_per_request billing mode doesn't use it.

I don't think this is backwards compatible, since a positional argument got changed to a keyword-only argument.

However, I would like to change how this is modeled anyway which would also keep this backwards compatible. ProvisionedThroughput and BillingMode are strongly linked and this should be represented at the type level in aiodynamo. If BillingMode is PROVISIONED, ProvisionedThroughput must be specified, if it is PAY_PER_REQUEST it must not be specified. So if we change the throughput parameter in create_table to Union[Throughput, PayPerRequest] BillingMode can be derived from that.

Both Throughput and PayPerRequest would have their encode methods set BillingMode and possibly ProvisionedThroughput.

@ojii do you reckon we could bump package requirements to python^3.8 ?

I don't see a problem with that.

TheEdgeOfRage commented 2 years ago

I have found out that Protocol has been backported to 3.7 with typing_extensions, so there is no need to bump to 3.8.

Also @ojii, I think that it should be fully backwards compatible now.

I have also made some minor changes here and there, since mypy was complaining about a few things in the test suite.

ojii commented 2 years ago

@TheEdgeOfRage thanks for the changes, looking good now. Just need to add the new model to https://github.com/HENNGE/aiodynamo/blob/master/docs/usage.rst#models

TheEdgeOfRage commented 2 years ago

@ojii I also added an explanation on the create_table method documentation, just to make it clear.