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

Allow me to do math on variables #1381

Closed
wants to merge 2 commits into from

Conversation

bobtfish
Copy link
Contributor

@bobtfish bobtfish commented Apr 3, 2015

I want to be able to make a module which generates ASGs of the same min and desired size, but with a max size of the desired size+1, and I want to be able to pass the size in as a variable (i.e. a string), however math only works on ints.

Therefore I wrote a toint function, to allow me to be able to write code like this:

max_size = "${toint(var.cluster_size)+1}"

Full example:
https://github.com/bobtfish/terraform-aws-coreos-kubernates-cluster/blob/master/nodes.tf#L28

I'm not sure if this is the right way forward (it'd be nice if we were to implicitly convert strings to ints), but it appears to work for my use-case right now, and was trivial to implement, so I thought I'd share it to prompt some discussion :)

@@ -23,6 +23,7 @@ func init() {
"element": interpolationFuncElement(),
"replace": interpolationFuncReplace(),
"split": interpolationFuncSplit(),
"toint": interpolationFuncToint(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be clearer if this was to_int for UX. "toint" sounds like a single word to be and could be confusing. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 - toint is also only 1 character off from taint which is a different important concept within TF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you like the explicit conversion (and don't think Terraform should be doing this implicitly) then I also like to_int more - I will rename and add tests some time in the next couple of days. :)

@bobtfish
Copy link
Contributor Author

Updated to rename the function as requested, and add docs for the website.

@knuckolls
Copy link
Contributor

code looks good to me and i like the idea. probably needs tests before merging to ensure future regressions don't occur.

@mitchellh
Copy link
Contributor

Fixed through #1521.

We always meant for these type conversions to actually be implicit. In fact, in 0.4.x you can do this: ${1+var.foo} and it'll work. There was a bug in the type inference which wasn't working if the first operand was not a primitive. We've fixed this in the above PR.

I understand the value of explicit types (after all, we do code in Go), but I think for the TF configuration language, it is actually preferred to have dynamic typing rather than static.

@ghost
Copy link

ghost commented May 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants