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

Add AwsSystemsManagerParameterStoreSettingsSource #385

Closed
wants to merge 4 commits into from

Conversation

alukach
Copy link

@alukach alukach commented Sep 10, 2024

What I'm adding

In a spirit similar to #175, this PR adds support for AWS Systems Manager Parameter Store. The name is a bit of a mouthful, but I errored on the side of verbosity and named the source AwsSystemsManagerParameterStoreSettingsSource. Something like AwsSsmParameterStoreSettingsSource might be better.

For Pydantic v1, we have been using https://github.com/developmentseed/pydantic-ssm-settings/ on a number of projects, however having the functionality included in Pydantic Settings core seems preferred.

How I did it

I more or less copied the pattern of AzureKeyVaultSettingsSource.

Uncertainties

The testing mocks out the AWS SSM Client, however typically I use something like moto to mock out the AWS services when testing. I wasn't sure about this project's appetite for the added test dependency.

Additionally, I was a bit uncertain about typing the SSMClient. Projects like boto3-stubs provide such types, however I wasn't sure about the project's appetite for adding this dependency.

docs/index.md Outdated Show resolved Hide resolved
@hramezani
Copy link
Member

Thanks @alukach for this PR.

First of all, I'm not sure we will support this settings source.
I'll check with the team and get back to you first.

docs/index.md Outdated Show resolved Hide resolved
@alukach
Copy link
Author

alukach commented Sep 10, 2024

@hramezani Sounds good, I'll stand by and wait for the green light before committing any more time to this. Given @samuelcolvin's endorsement of supporting AWS SecretsManager, I figured AWS Parameter Store wouldn't be much of a stretch. Once this is merged, I intend also to add functionality similar to Secrets Manager.

@hramezani
Copy link
Member

@alukach I discussed with the team and we decided to not accept this source for now. because:

  • It makes the code base mode complex. we need to maintain and handle one more settings source
  • There is a possibility for users to have their own settings source
  • It is not much request from users for this settings source.

BTW, if we get requests for this settings source, we can add it later.

Thanks for your effort!

@alukach
Copy link
Author

alukach commented Sep 11, 2024

@hramezani thanks for the quick follow up. I'll close this and track the feature request as an issue (#399).

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.

2 participants