-
Notifications
You must be signed in to change notification settings - Fork 421
feat(parameters): add get_parameters_by_name for SSM params in distinct paths #1678
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): add get_parameters_by_name for SSM params in distinct paths #1678
Conversation
cc @peterschutt as this is the first PR that adds support for |
Two main questions to discuss in the original issue:
Did two tests within Lambda:
Quick results
Without much further digging, this is largely due to CPU slicing in Lambda (one real core only at ~1.7G), combined with LWP cost and SSM throttling. |
Signed-off-by: heitorlessa <[email protected]>
|
Thanks @jplock. Great to hear from you as usual. For the first point, that's what we're gonna do with a small difference - if one enables decrypt as a default Or per parameter, we need to use GetParameter. Doing the latter in parallel can lead to timeouts easily, will do serially for decrypt calls instead.
The former. We already have a solution for the latter via get_parameters(path="/develop/service") |
a0c2da1
to
215b2d7
Compare
another TIL Service model for Need to create an issue in Boto repo. Actual API returns an empty list. ...
stubber = stub.Stubber(provider.client)
response = {
"Parameters": [
{
"Name": dev_param,
"Type": "String",
"Value": "string",
"Version": mock_version,
"Selector": f"{dev_param}:{mock_version}",
"SourceResult": "string",
"LastModifiedDate": datetime(2015, 1, 1),
"ARN": f"arn:aws:ssm:us-east-2:111122223333:parameter/{dev_param.removeprefix('/')}",
"DataType": "string",
},
{
"Name": prod_param,
"Type": "String",
"Value": "string",
"Version": mock_version,
"Selector": f"{prod_param}:{mock_version}",
"SourceResult": "string",
"LastModifiedDate": datetime(2015, 1, 1),
"ARN": f"arn:aws:ssm:us-east-2:111122223333:parameter/{prod_param.removeprefix('/')}",
"DataType": "string",
},
],
"InvalidParameters": [], # NOTE: stub fails on validation despite not having any
}
expected_params = {"Names": param_names}
stubber.add_response("get_parameters", response, expected_params) # fail here
stubber.activate() Validation error when adding response E botocore.exceptions.ParamValidationError: Parameter validation failed:
E Invalid length for parameter InvalidParameters, value: 0, valid min length: 1 |
ASCII diagram to ease understanding of how we decide when to use one or both APIs to fetch parameters.
|
Signed-off-by: Heitor Lessa <[email protected]>
22da47f
to
285c431
Compare
Codecov ReportBase: 99.34% // Head: 99.27% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1678 +/- ##
===========================================
- Coverage 99.34% 99.27% -0.07%
===========================================
Files 128 129 +1
Lines 5917 6050 +133
Branches 377 400 +23
===========================================
+ Hits 5878 6006 +128
- Misses 18 20 +2
- Partials 21 24 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Merging after extensive tests and self-review - as Ruben and Leandro are on PTO This PR made it clear that we have to prioritize refactoring Parameters entirely. Hit the following issues which delayed completion by nearly a week:
|
Issue number: #1040
Summary
Today, customers can fetch multiple SSM parameters recursively by using a
path
. This happens server-side.As described in the issue, customers may need to fetch multiple parameters in distinct paths. As of now, this requires additional boilerplate to get it done properly.
Changes
This PR introduces
get_parameter_by_name
high-level function that accepts a dictionary containing:max_age
,transform
,decrypt
, etc.)Non-related but necessary changes
typing-extensions
as a dependency to provide strict typing for this feature. Depending on thetransform
value, the return type is precise.transform_method
andtransform_value
to better support strict typing and related bugsadd_to_cache
method to support refactoring and handlemax_age=0, max_age=-1
scenarios we didn't cover beforehas_not_expired_in_cache
method to ease testingUser experience
Overrides fail-fast to support graceful error handling
Checklist
If your change doesn't seem to apply, please leave them unchecked.
GetParameters
as default. UseGetParameter
whendecrypt
is set on a per parameterIs this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
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.