Skip to content

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.

@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.

@alukach alukach force-pushed the main branch 4 times, most recently from 67d148a to 73130ce Compare September 10, 2024 20:32
@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