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(parameters): adds setParameter function to store SSM parameters #3020

Merged

Conversation

daschaa
Copy link
Contributor

@daschaa daschaa commented Sep 3, 2024

Summary

Implements the setParameter functionality. Closes #2871

Changes

With this change the SSMProvider is extended to have a set method that allows to pass in a name and an options object.
In the options the value property is mandatory to set a value. Along with the value there are multiple properties that were suggested here.
The docstring for this method is mainly copy, pasted and adjusted from the get method in the same class.

Additional to the implementation in the SSMProvider, a setParameter function has been implemented and export via the barrel file. It supports the same structure like the set method of the SSMProvider class.
The docstring is also mainly copy, pasted and adjusted from the getParameter function.

There is also a section added to the documentation in the docs folder for storing parameters with setParameter. I decided to include an basic example and one example for overwriting a parameter value (because this was something I experienced myself in the past, that I need the overwrite flag and I thought this might be a handy example).

Please let me know if i should adjust the documentation and/or add stuff. I tried to keep everything basic and extend later on after the review.

Open topics

  • Implement end-to-end tests
  • Check the doc folder if something needs to be adjusted

Issue number: #2871


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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@boring-cyborg boring-cyborg bot added parameters This item relates to the Parameters Utility tests PRs that add or change tests labels Sep 3, 2024
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Sep 3, 2024
@daschaa daschaa changed the title feat: adds implementation for setParameter feat(parameters): adds implementation for setParameter Sep 3, 2024
@daschaa daschaa changed the title feat(parameters): adds implementation for setParameter feat(parameters): adds setParameter function to store SSM parameters Sep 3, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Sep 3, 2024
@pull-request-size pull-request-size bot added size/XL PRs between 500-999 LOC, often PRs that grown with feedback and removed size/L PRs between 100-499 LOC labels Sep 3, 2024
@daschaa daschaa marked this pull request as ready for review September 3, 2024 12:44
@daschaa daschaa requested review from a team as code owners September 3, 2024 12:44
@dreamorosi
Copy link
Contributor

Hi @daschaa, thank you for the PR!

I'll start reviewing it by tomorrow end of the day at the latest.

@github-actions github-actions bot added the feature PRs that introduce new features or minor changes label Sep 5, 2024
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

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

Hi @daschaa, thank you for the patience while I reviewed the PR.

High quality as usual, just left some minor comments - looking forward to merge this!

packages/parameters/tests/unit/setParameter.test.ts Outdated Show resolved Hide resolved
packages/parameters/tests/unit/SSMProvider.test.ts Outdated Show resolved Hide resolved
packages/parameters/src/ssm/SSMProvider.ts Outdated Show resolved Hide resolved
packages/parameters/src/ssm/SSMProvider.ts Outdated Show resolved Hide resolved
@daschaa
Copy link
Contributor Author

daschaa commented Sep 9, 2024

@dreamorosi Thanks for the review, I think I have covered everything :)

Copy link

@dreamorosi dreamorosi self-requested a review September 10, 2024 18:00
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

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

Thank you for working on this PR, @daschaa!

I appreciate the time you've put into this, both in the initial implementation and subsequent iterations.

I also ran the end to end tests and they were green 🎉

@dreamorosi
Copy link
Contributor

@leandrodamascena, @sthulb, @am29d - if either of you see this, please feel free to approve & merge this.

I had to do a couple of touch ups in the form of suggestions and I can't merge it myself because I'm the last one to have committed.

Thanks!

@leandrodamascena leandrodamascena merged commit 8fd5479 into aws-powertools:main Sep 11, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature PRs that introduce new features or minor changes parameters This item relates to the Parameters Utility size/XL PRs between 500-999 LOC, often PRs that grown with feedback tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Put parameter
3 participants