-
Notifications
You must be signed in to change notification settings - Fork 313
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
[Resolve #390] Cast all parameters to strings #401
[Resolve #390] Cast all parameters to strings #401
Conversation
sceptre/stack.py
Outdated
# recurse | ||
new_list = [self._format_value(item) for item in value] | ||
return ",".join(new_list) | ||
if value is True: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want to downcase this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If your parameter is defined like this:
Parameters:
Key:
Type: String
AllowedValues:
- False
- True
Only "false"
/"true"
will work, not "False"
/"True"
. Same goes for False
/True
. So you need to downcase it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ngfgrant who is authorised to approve this it looks good to me though
d6d017c
to
e1c1c77
Compare
@ngfgrant Can you look into this PR please? |
e1c1c77
to
903b66f
Compare
Hey @matrinox - yeh this is definitely on my list. I am aiming to get a v1 release out this week and then will look at this as potentially something for v2 release. It is definitely on my list though ;) |
Awesome, thanks for the update! |
@matrinox I am going to get another couple of PRs in relating to refactoring the way config is handled and then after I am done would it be ok if I ask you to update this from |
@ngfgrant When you finish doing that, please mention again and I will get to it :). |
901d20a
to
05c6742
Compare
Any chance of this being picked up? Numbers / booleans in the stack config file not being coerced to strings and this causing failure is the most frequent cause of |
8bc6895
to
bc830e2
Compare
boto3 only supports strings for parameters, even if the parameter itself is non a string type On branch support-non-string-type-parameters - Wed 13 Jun 2018 13:25:46 PDT by Geoff Lee <geofflee25@gmail.com>
bc830e2
to
971cd9a
Compare
Any chance of this or similar being merged? This has been a big pain point for my team. |
@alexharv074 we already migrated to terraform so I'm not interested in updating this branch any longer. Sorry :( You're welcome to take the code and run with it though, I don't mind. |
I have proposed an alternative idea to help with this in #925 |
Problem
Sceptre allows you to pass non-string values as parameters. Boto3 only allows string parameters. Refer to #390 for more details.
Example config:
Solution
To solve this, all non-string values are coerced into strings. An exception is made for booleans (True/False becomes "true"/"false"). List values are now recursively formatted. This has the side effect of supporting nested array values for parameters (no tests for this).
Alternatives