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

fix(ssm): Make decrypt an explicit option and refactoring #123

Merged
merged 13 commits into from
Aug 24, 2020

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Aug 21, 2020

Issue #, if available:

Description of changes:

Make decrypt an explicit parameter of SSMProvider. Currently although decrypt is part of the documentation, it is not
part of the method signature and therefore confusing as to how it works.

I only changed aws_lambda_powertools/utilities/parameters/ssm.py as this option only applies to the ParameterStore.

NOTE: This also unrelated cleanup and refactoring

Checklist

No need to update the docs or unit tests, this seems to be backwards compatible.

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

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2020

Codecov Report

Merging #123 into develop will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           develop      #123   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          701       706    +5     
  Branches        66        66           
=========================================
+ Hits           701       706    +5     
Impacted Files Coverage Δ
..._lambda_powertools/utilities/parameters/secrets.py 100.00% <ø> (ø)
aws_lambda_powertools/utilities/parameters/base.py 100.00% <100.00%> (ø)
aws_lambda_powertools/utilities/parameters/ssm.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0aaad5...99e4bef. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Aug 21, 2020

This pull request introduces 1 alert when merging d70eaa1 into 6736af2 - view on LGTM.com

new alerts:

  • 1 for First parameter of a method is not named 'self'

Michael Brewer and others added 9 commits August 21, 2020 13:29
Changes:
ssm.py - get_parameters - pass through the **sdk_options and merge in
the recursive and decrypt params
ssm.py - get_parameter - add explicit option for decrypt
- `decrypt` should be false by default
- `recursive` should be true by default
Changes:
* capture_method should yield from within the "with" statement
* Add missing test cases

Closes #112
Changes:
- base.py - update get_multiple to reduce the overall complexity
- base.py - `_has_not_expired` returns whether a key exists and has not expired
- base.py - `transform_value` add `raise_on_transform_error` and default to True
- test_utilities_parameters.py - Add a direct test of transform_value
@michaelbrewer michaelbrewer changed the title fix(ssm): Make decrypt an explicit option fix(ssm): Make decrypt an explicit option and refactoring Aug 23, 2020
Michael Brewer added 3 commits August 23, 2020 08:57
Changes:
* Add type hint to `values` as it can change later on in transform
* Use a slightly faster and easier to read for each over dict
  comprehension
@nmoutschen
Copy link
Contributor

Looks good to me and all tests pass. 😄 Thanks a lot! I'll merge that ASAP.

@nmoutschen nmoutschen merged commit 96b7b8f into aws-powertools:develop Aug 24, 2020
@michaelbrewer michaelbrewer deleted the fix-ssm-decrypt branch August 25, 2020 03:09
@heitorlessa heitorlessa added the bug Something isn't working label Aug 25, 2020
heitorlessa referenced this pull request in ran-isenberg/aws-lambda-powertools-python Aug 26, 2020
* develop:
  docs: Fix doc for log sampling (#135)
  fix(logging): Don't include `json_default` in logs (#132)
  chore: bump to 1.4.0
  docs: add Lambda Layer SAR App url and ARN
  fix: upgrade dot-prop, serialize-javascript
  fix heading error due to merge
  formatting for bash script
  add layer to docs and how to use it from SAR
  moved publish step to publish workflow after pypi push
  fix(ssm): Make decrypt an explicit option and refactoring (#123)
  change to eu-west-1 default region
  remove tmp release flag and set trigger to release published
  add overwrite flag for ssm
  add relase tag simulation
  more typos
  fix typo in branch trigger
  fix indent, yaml ...
  line endings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants