theforeman / foreman-ansible-modules

Ansible modules for interacting with the Foreman API and various plugin APIs such as Katello
GNU General Public License v3.0
147 stars 163 forks source link

Compute attributes needs to be dictionary, they do not accept lists #954

Open ezr-ondrej opened 4 years ago

ezr-ondrej commented 4 years ago

Compute attributes interfaces_attributes and volumes_attributes needs to be dicts. These are really dicts in form of:

interfaces_attributes:
  '0':
    type: VirtualE1000
    network: dvportgroup-107
  '1':
    type: VirtualE1000
    network: dvportgroup-106

It's a bit ugly and strange. Maybe it's a good idea to:

  1. Document the correct format and possible parameters of vm_attrs dict
  2. Make the nested parameters a bit more convenient and provide the both data structures as lists instead of dicts.
evgeni commented 4 years ago

Yeah, better examples are indeed a good idea. The second idea is also good, but more work. Are you planning to work on this?

cmeissner commented 4 years ago

Maybe this is an wanted behavior because you can't guarantee same order of lists in ansible on each run. But for network interfaces and volumes the order is essentially. So before here are changes made it has to be clarified whether order can guarantied or not. If not there should be a key rank or so, to reflect the order in that way.

mdellweg commented 3 years ago

From hacking the vcr wrapper around py2/3 incompatibilities, i'd suspect that lists are ordered stable, but dicts are not.

ezr-ondrej commented 3 years ago

From hacking the vcr wrapper around py2/3 incompatibilities, i'd suspect that lists are ordered stable, but dicts are not.

For current format, Foreman will ensure the order. But there is no reason not to use Arrays for APIs. Most languages has ordered stable arrays/lists. I'm trying this out in https://github.com/theforeman/foreman/pull/8006

ezr-ondrej commented 3 years ago

I believe that now we can use arrays from next foreman version as theforeman/foreman#8006 got merged.