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

Should concat() support lists of complex types? #7146

Closed
phinze opened this issue Jun 12, 2016 · 8 comments
Closed

Should concat() support lists of complex types? #7146

phinze opened this issue Jun 12, 2016 · 8 comments

Comments

@phinze
Copy link
Contributor

phinze commented Jun 12, 2016

Over in #7145 I've submitted a fix for the reported panic in #7030. This fix is the stop gap measure of properly reporting the lack of support in concat() for anything but lists of strings.

We should decide what we want the semantics of concat() to be for the following:

  • List of maps
  • List of lists
  • Arbitrarily nested versions of the above

It would also help us decide how to prioritize making this functionality work if we could gather together a few use cases for the various scenarios.

@phinze
Copy link
Contributor Author

phinze commented Jun 12, 2016

@oillio did you have a use case in mind when you ran into the panic in #7030?

@phinze phinze changed the title concat() should support lists of complex types Should concat() support lists of complex types? Jun 12, 2016
@oillio
Copy link

oillio commented Jun 13, 2016

I don't have a use case that actually works :)

I ran into the crash when I was trying to find a way to use multiple sources of values to configure an aws_elastic_beanstalk_environment.setting field. I couldn't find a way to use variable data in the setting, so I don't currently have a use for concat on complex variable types.
I will want to use concat like this if a feature like #7034 is ever implemented.

@jen20
Copy link
Contributor

jen20 commented Jun 22, 2016

This is fairly incompatible with the type checker in HIL, since concat has to explicitly declare the return type (hence the backward compatibility note of no longer being able to use concat() with strings). We should probably consider whether to make the type checker smarter or whether to add differently named mechanisms for concatenating other data types.

@phinze
Copy link
Contributor Author

phinze commented Jun 27, 2016

I think the return type for concat() will always be a TypeList here - it's just the type of the contents of said list that change.

A shot at target semantics:

  • concat() - combine lists, e.g. concat(["foo", "bar"], ["baz"]) => ["foo, "bar", "baz"]
  • merge() - combine maps e.g. merge({foo = 1}, {bar = 2}) => {foo = 1, bar = 2}

@tecnobrat
Copy link

Looks like this was fixed by the mentioned pr?

@phinze
Copy link
Contributor Author

phinze commented Jul 26, 2016

Indeed, this was taken care of in #7528 by @jbardin

@phinze phinze closed this as completed Jul 26, 2016
@ju2wheels
Copy link

@phinze Im not following what the overall decision here was. Is concat supposed to work on lists of maps or only lists of strings?

Using 0.9.6, Im trying to do the following:

resource "aws_rds_cluster_parameter_group" "my_rcpg" {
  description = "My Cluster - ${var.my_module_instance_name}"
  family      = "${var.my_rds_cluster_parameter_group_family}"
  name        = "mymodule-${var.my_module_instance_name}"
  parameter   = "${concat(list(map("apply_method", "immediate", "name", "aws_default_s3_role", "value", aws_iam_role.my_role.arn)), var.my_rds_cluster_parameter_group_parameters)}"
  tags        = "${var.my_rds_cluster_parameter_group_tags}"
}

# tfvars
my_rds_cluster_parameter_group_parameters = []

Although im getting the following error:

* aws_rds_cluster_parameter_group.my_rcpg: parameter: should be a list

Im pretty sure this will work if I just pass it a var of list of maps instead of trying to construct a default list with a map object and appending any additional maps to it. Is this a separate bug or expected concat behavior?

@ghost
Copy link

ghost commented Apr 8, 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 Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants