About the new loop construct, giving feedback

[elav...@redhat.com]

Hi,

in his blog entry [1], Dylan asked for feedback on the new loop feature.

As much as I understand that having a syntax with changing keywords isn't optimal, I dislike the new lookup-based syntax for two reasons:

1. we've had so many issues with quoting within quoting of lookup that I don't look forward having even more lookups in my code.
2. having code within a string makes it less readable, you lose the clean structure and code validation of YAML and the syntax highlighting in your editor, which leads to potentially more errors.

As said, I understand that having a with_* structure in code isn't nice, hence I would suggest an alternative consisting of two main points:

1. make it a clean loop structure with options
2. merge the loop_control structure with the loop structure

This means that the following could be written instead of a with_items:

- debug:
    msg: one line of {{ myitem }}
  loop:
    with: items # instead of with_items, easier to parse hopefully
    through: # you could still use a string with fancy lookup here
      - item1
      - item2
    as: myitem # loop_var is a bit clumsy IMHO but also fine
    pause: ...etc...

or using all the (assumed) defaults:

- debug:
    msg: one line of {{ item }} # default var is still 'item'
  loop:
    through: # assuming 'items' is the default 'with'
      - item1
      - item2

This is just a suggestion as basis for discussion, and I can definitely live with an alternative syntax, but it should avoid having too much placed in strings and strings within strings, which is implied IMHO by the plain lookup-syntax.

KR, Eric

[1] https://www.ansible.com/blog/loop-plays-past-present-future

[Brian Coca]

I'm unsure what the problem you state with strings is, `loop` also
accepts a list directly:

debug:
msg: one line of {{ item }}

loop:
- item1
- item2

Your proposal is very similar to my preferred design, sadly
loop_control was added prior to loop so we ended up making it 2
directives, but i originally thought:

loop:
over: # list of items
variable: # default 'item', aka 'var
display: # default to variable value, aka 'loop_control.label'
index: # variable that contains the loop index, default index
pause: # default 0
forks: # parallelization, default 1
squash: no|yes # push the loop to run on the remote, default no as
it adds many limitations to loops
until: # end condition for loop, default `index > len(over)`, it
would also eliminate the need of current until/retries keywords


FYI, loop is equivalent to with_list, with_items: '{{mylist}}' is the
same as loop: '{{ mylist|flatten(1)}}', items flattens 1 level of
lists, which is 'magic' that confuses people and that we wanted to
move away from.

The move to loop was also in part to remove the use of 'lookups as
filters' instead of being a plugin that query external data. Since
many of them manipulate existing data for use in loops do the same job
as existing filters .. or they should be filters as that is the
purpose of that plugin type.

--
----------
Brian Coca

[Eric Lavarde]

Hi Brian,

On 2018-05-02 16:55, Brian Coca wrote:> I'm unsure what the problem you

state with strings is, `loop` also
> accepts a list directly:
>
> debug:
> msg: one line of {{ item }}
> loop:
> - item1
> - item2

Yeah, my fault, I took the very simple example where the new construct
isn't really different from the old one.

If I take the more complex example from the documentation:

- name: give users access to multiple databases
mysql_user:
name: "{{ item[0] }}"
priv: "{{ item[1] }}.*:ALL"
append_privs: yes
password: "foo"
loop: "{{ query('nested', [ 'alice', 'bob' ], [ 'clientdb',
'employeedb', 'providerdb' ]) }}"

It used to be:

- name: give users access to multiple databases
mysql_user:
name: "{{ item[0] }}"
priv: "{{ item[1] }}.*:ALL"
append_privs: yes
password: "foo"
with_nested:
- [ 'alice', 'bob' ]
- [ 'clientdb', 'employeedb', 'providerdb' ]

What I mean is that the loop's string with the query/lookup is now a
string containing lists of strings, with no syntax highlighting and a
worse readability (and some strange behaviours if you don't get the
quoting right).

As for the actual implementation, yours is as good as mine, important to
my eyes is that it remains a YAML structure and no string.

KR, Eric

PS: sorry for the first private answer, hit the wrong button...

[Matt Martz]

FWIW, not all lookup plugins are necessarily the best fit for use with `loop`.  Lookups took on a set of use cases over the years, that didn't really make them the best implementation for looping.  lookup or query should help identify that their goal isn't really about data manipulation, but about looking up or querying data.

Often times you may find that there are better alternatives utilizing filters, and maybe some restructuring of your task slightly.

In your `with_nested` example, this would likely be better written with loop like the following:

loop: "{{ users|product(databases)|list }}"

vars:

  users: [ 'alice', 'bob' ]

  databases: [ 'clientdb', 'employeedb', 'providerdb' ]

In an example of `with_dict` you have a few options:

- debug:

    msg: "key: {{ item.0 }}, value: {{ item.1 }}" 

  loop: "{{ some_dict|dictsort|list }}"

or 

- debug:

    msg: "key: {{ item.key }}, value: {{ item.value }}" 

  loop: "{{ some_dict|dict2items }}"

`with_flatten` or some cases of `with_items`, could be best represented using the `|flatten` filter.

In many cases, using filters, to manipulate your data, instead of using lookups is the better path than purely switching from `with_FOO` to `loop: "{{ query('FOO', ...) }}"`

--
You received this message because you are subscribed to the Google Groups "Ansible Project" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ansible-project+unsubscribe@googlegroups.com.
To post to this group, send email to ansible-project@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ansible-project/ce27ffe5-5ba2-7713-87da-7c4033089855%40redhat.com.

For more options, visit https://groups.google.com/d/optout.

--

Matt Martz
@sivel
sivel.net

[Brian Coca]

Well, nested is one of the 'bad lookups' i was referring to, its
cleaner as a filter:

- name: give users access to multiple databases
mysql_user:
name: "{{ item[0] }}"
priv: "{{ item[1] }}.*:ALL"
append_privs: yes
password: "foo"

loop: '{{ l1|product(l2)|list }}'
vars:
l1: [ 'alice', 'bob' ]
l2: [ 'clientdb', 'employeedb', 'providerdb' ]



------------------
Brian Coca

[Eric Lavarde]

Hi,

On 2018-05-02 21:35, Brian Coca wrote:
> Well, nested is one of the 'bad lookups' i was referring to, its
> cleaner as a filter:

Starting to understand what you and Matt are trying to tell me, I'm now
at the point where I think that what's missing is a mapping between the
old with_/lookup approach and the new loop/filter approach for each kind
of `with_*`.

You listed a few but a consolidated list would be cool as they're not
exactly obvious, at least not to me.

Thanks for the clarification,
Eric

[Brian Coca]

That is on my list, also making sure alternative filters exist for all
the 'non lookup lookups' (why dict2items exists), iirc only
'subelements' is not covered.


--
----------
Brian Coca