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

feat: Add DynamoDB provider to parameters module #1091

Merged
merged 12 commits into from
Mar 20, 2023

Conversation

scottgerring
Copy link
Contributor

@scottgerring scottgerring commented Mar 16, 2023

Issue #, if available:
#1083

Description of changes:

Adds a DynamoDbProvider to the parameters module. Follows the style of the existing parameters provider and the schema used in the python powertools.

Checklist

Breaking change checklist

N/A - new functionality

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@scottgerring scottgerring changed the title [WIP feat] Add DynamoDB provider to parameters module [feat] Add DynamoDB provider to parameters module Mar 17, 2023
@scottgerring scottgerring changed the title [feat] Add DynamoDB provider to parameters module feat: Add DynamoDB provider to parameters module Mar 17, 2023
.item(responseData)
.build());
// Act
Assertions.assertThrows(NullPointerException.class, () -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure it should be nullpointer? Couldn't we have a better (eventually custom) one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Throwing a DynamoDbProviderSchemaException now with the error text describing the missing column

@scottgerring
Copy link
Contributor Author

@jeromevdl I think that should be it! I've cleaned up the exceptions too.

@scottgerring
Copy link
Contributor Author

@jeromevdl anything else to polish? 👼

Copy link
Contributor

@jeromevdl jeromevdl left a comment

Choose a reason for hiding this comment

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

LGTM

@scottgerring scottgerring merged commit 365587d into master Mar 20, 2023
@scottgerring scottgerring deleted the feat-params-dynamodb branch March 20, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants