inmanta / inmanta-core

Inmanta is an automation and orchestration tool
https://inmanta.com
Apache License 2.0
27 stars 7 forks source link

Add support for concatenating lists of primitive values #5554

Open arnaudsjs opened 1 year ago

arnaudsjs commented 1 year ago

Timebox to one hour for the parser part, if it acts up, backlog

It should be possible to concatenate primitive lists together:

first = [1, 2, 3]
second = first + [4]

Make it syntactic sugar for the unintuitive c = [a, b]

bartv commented 1 year ago

Do you have a specific use case for this? Because you can only do this for lists assigned to a variable, not with those assigned to an attribute.

arnaudsjs commented 1 year ago

This is the use case: I want to make the packages that should be installed conditional.

pg_packages_without_devel = ["postgresql{{pg_version}}-server", "postgresql{{pg_version}}-contrib"]
pg_packages_with_devel = ["postgresql{{pg_version}}-server", "postgresql{{pg_version}}-contrib", "postgresql{{pg_version}}-devel"]
infra::Packages(
    host=self,
    # Only install the devel package for the default_pg_version due to a dependency conflict.
    name=(pg_version==default_pg_version ? pg_packages_with_devel : pg_packages_without_devel),
    requires=pg_repo,
    provides=self.provides,
)

I don't like the duplication in the content of both lists. Another alternative would be to put the constructor call in an if-statement, but that would result in even more duplication:

if pg_version==default_pg_version:
    # Only install the devel package for the default_pg_version due to a dependency conflict.
    infra::Packages(
        host=self,
        name= ["postgresql{{pg_version}}-server", "postgresql{{pg_version}}-contrib", "postgresql{{pg_version}}-devel"],
        requires=pg_repo,
        provides=self.provides,
    )
else:
    infra::Packages(
        host=self,
        name= ["postgresql{{pg_version}}-server", "postgresql{{pg_version}}-contrib"],
        requires=pg_repo,
        provides=self.provides,
    )
end

So I would like to be able to write:

pg_packages_without_devel = ["postgresql{{pg_version}}-server", "postgresql{{pg_version}}-contrib"]
pg_packages_with_devel = pg_packages_without_devel + ["postgresql{{pg_version}}-devel"]
infra::Packages(
    host=self,
    # Only install the devel package for the default_pg_version due to a dependency conflict.
    name=(pg_version==default_pg_version ? pg_packages_with_devel : pg_packages_without_devel),
    requires=pg_repo,
    provides=self.provides,
)

Sander pointed out that list inside lists are automatically flattened. So it would be possible to write this in the following way:

pg_packages_without_devel = ["postgresql{{pg_version}}-server", "postgresql{{pg_version}}-contrib"]
pg_packages_with_devel = [pg_packages_without_devel, "postgresql{{pg_version}}-devel"]
infra::Packages(
    host=self,
    # Only install the devel package for the default_pg_version due to a dependency conflict.
    name=(pg_version==default_pg_version ? pg_packages_with_devel : pg_packages_without_devel),
    requires=pg_repo,
    provides=self.provides,
)

But I must admit I find this code harder to read than the snippet that uses the unsupported + operator.

bartv commented 1 year ago

In this case it is even simpler because infra::Packages is just a wrapper around std::Package with a for loop

So just split the list:

if pg_version==default_pg_version:
    # Only install the devel package for the default_pg_version due to a dependency conflict.
    infra::Packages(
        host=self,
        name=["postgresql{{pg_version}}-devel"],
        requires=pg_repo,
        provides=self.provides,
    )
end

infra::Packages(
    host=self,
    name= ["postgresql{{pg_version}}-server", "postgresql{{pg_version}}-contrib"],
    requires=pg_repo,
    provides=self.provides,
)
bartv commented 1 year ago

But I do agree that not being able to add items to a list is often a limitation. However, I am a bit scared in adding this to variables and then how do we explain that this is not possible for attributes?

Or would it really be just doing c = a + b?

arnaudsjs commented 1 year ago

Hmm. I see your concern regarding the inconsistency that would be introduced between the handling of attributes and variables.

sanderr commented 1 year ago

Or would it really be just doing c = a + b?

That's how I interpret the description. I wouldn't be in favor of making lists updatable, but a simple concatenation operator could be nice. It could even just be syntactic sugar for the unintuitive c = [a, b].

Hugo-Inmanta commented 1 year ago

The parser won, putting this in the backlog as suggested in the ticket description