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

Merging a list in pillars #28394

Closed
ldelossa opened this issue Oct 29, 2015 · 7 comments
Closed

Merging a list in pillars #28394

ldelossa opened this issue Oct 29, 2015 · 7 comments
Labels
Duplicate Duplicate of another issue or PR - will be closed
Milestone

Comments

@ldelossa
Copy link

I'm noticing that simply merging lists within pillars is not possible. I'm trying to take two pillar files with the following lists:

packages:
  - traceroute
  - nmap
packages:
  - mtr
  - ping

I would like to apply these two pillars to a minion, and have them merge into one conhesive list.

This seems to be only possible in a hackish way using nested dictionaries. Are there any plans to do this with the simple merge data struct?

packages:
  install:
    open-vm-tools:
    lsof:
    tcpdump:
    mtr:
    traceroute:
    telnet:
    bind-utils:
    curl:
    wget:
    ftp:
    tftp:
    samba:
    samba-client:
    ntp:
    yum-utils:
  remove:
    postfix:
    NetworkManager:

This is the structure that will allow merging. I cannot have similar keys anywhere. Also seems to break with Ints.

@mlalpho
Copy link
Contributor

mlalpho commented Oct 31, 2015

dupe of #3991 ?

@ldelossa
Copy link
Author

Looks like that is the case. It's cool to close this one out then.

@jfindlay jfindlay added the Duplicate Duplicate of another issue or PR - will be closed label Nov 3, 2015
@jfindlay jfindlay added this to the Approved milestone Nov 3, 2015
@jfindlay
Copy link
Contributor

jfindlay commented Nov 3, 2015

@ldelossa, @mlalpho, thanks for reporting.

@uvsmtid
Copy link
Contributor

uvsmtid commented Nov 21, 2015

Strictly speaking, issue #3991 (based on examples in the initial posts here and there) is supposed to solve completely different problem of merging dictionaries.

The problem of merging dictionaries is solved as I commented there. If two dictionaries from different pillar files contain conflicting key names, the value of the first one gets overwritten by the second one (however, the order which one is the 1st is not known to me). If there is no key name conflicts, both key-values are simply accepted in the combined dictionary.

The problem of merging lists is not solved. Lists are just value objects like strings, numbers, or anything else (except dictionaries which recurse during merging). However, default strategy is not clear:

  • Should lists be overwritten (as it is now)?
  • Should they be combined? If yes, then which order they should be combined in (because order of list items is often used as a feature).
  • And if different strategies are required, there should be a way to control it.

Dictionaries are more straightforward in this sense as deep merging (with value overwrites in case of conflicting keys) is almost always acceptable default behaviour.

Should this issue be reopened and be focused strictly on merging list items?

@ldelossa
Copy link
Author

I believe so.

The idea of having a hierarchical pillar structure is difficult without the
merging of lists. I can merge dictionaries but I'm left with valueless keys
in some situations which is more of a hack then anything.

Example:

/core/packages.sls has list of 14 packages

/rabbitmq/packages.sls has 3 more packages

If both the core.packages and rabbitmq.packages pillars are applied to the
host, then all 17 list items should be applied to the host.

It would be nice if in the actual top.sls we can define if two pillars with
the same .sls name are to be merged or are they to overwrite. Since
overwriting is still a very functional procedure in a hierarchical design.

On Saturday, November 21, 2015, Alexey Pakseykin [email protected]
wrote:

Strictly speaking, issue #3991
#3991 (based on examples in the
initial post here and there) is supposed to solve completely different
problem of merging dictionaries.

The problem of merging dictionaries is solved as I commented there
#3991 (comment).
Currently, if dictionary keys from two different pillars match, their
values gets overwritten (I'm not sure if there is a defined order which
gets overwritten by which).

The problem of merging lists is not solved. Lists are just value objects
like strings, numbers, or anything else (except dictionaries which recurse
during merging). However, default strategy is not clear:

  • Should lists be overwritten (as it is now)?
  • Should they be combined? If yes, then which order they should be
    combined in (because order of list items is often used as a feature).
  • And if different strategies are required, there should be a way to
    control it.

Dictionaries are more straightforward in this sense as deep merging (with
value overwrites in case of conflicting keys) is almost always acceptable
default behaviour.

Should this issue be reopened and be focused strictly on merging list
values?


Reply to this email directly or view it on GitHub
#28394 (comment).

@uvsmtid
Copy link
Contributor

uvsmtid commented Nov 21, 2015

@ldelossa
You see (I refer to my points above), making it "nice" may unnecessarily complicate rules and add new keywords just for the sake of merging lists while valueless keys in dictionaries already provide acceptable solutions in most of the cases.

Technically speaking, I don't feel urge for this feature (and I'm OK if this issue stay closed until something escalates the case).

In fact, there is only one unresolved (rare?) need - combining items in the order they appear:

# file_a.sls:
some_values:
    one: ~
    two: ~
# file_a.sls:
some_values:
    three: ~
    four: ~
# Result of `pillar.items`:
some_values:
    four: ~
    one: ~
    two: ~
    three: ~

When combined dictionary is traversed, the order of keys is undefined. Lists could solve the order issue.

What I really want (just to make public aware to strive for pointful and focused discussion) is:

  • Disambiguation: dicts and lists are different topics.
  • Known solutions: dicts have already been solved and lists have known acceptable workaround.

@ldelossa
Copy link
Author

I understand you're point. However to I do believe valueless keys is not an
intuitive method for merging. A new user to salt would have trouble
understanding why this is the method when a list is a simple collection of
items.

I'm happy with the solution and work around and can live with it, but in my
opinion it does add to the already steep learning curve of salt

On Saturday, November 21, 2015, Alexey Pakseykin [email protected]
wrote:

@ldelossa https://github.com/ldelossa
You see (I refer to my points above
<#1512ad8377c7e0aa_issuecomment-158652721>), making it "nice" may
unnecessarily complicate rules and add new keywords just for the sake of
merging lists while valueless keys in dictionaries already provide
acceptable solutions in most of the cases.

Technically speaking, I don't feel urge for this feature (and I'm OK if
this issue stay closed until something escalates the case).

In fact, there is only one unresolved (rare?) need - combining items in
the order they appear:

file_a.sls:

some_values:
one: ~
two: ~

file_a.sls:

some_values:
three: ~
four: ~

Result of pillar.items:

some_values:
four: ~
one: ~
two: ~
three: ~

When combined dictionary is traversed, the order of keys is undefined.
Lists could solve the order issue.

What I really want (just to make public aware to strive for pointful and
focused discussion) is:

  • Disambiguation: dicts and lists are different topics.
  • Known solutions: dicts have already been solved and lists have known
    acceptable workaround.


Reply to this email directly or view it on GitHub
#28394 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate Duplicate of another issue or PR - will be closed
Projects
None yet
Development

No branches or pull requests

4 participants