foreman-ansible-modules icon indicating copy to clipboard operation
foreman-ansible-modules copied to clipboard

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

Open ezr-ondrej opened this issue 4 years ago • 5 comments

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.

ezr-ondrej avatar Sep 15 '20 12:09 ezr-ondrej

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

evgeni avatar Sep 15 '20 14:09 evgeni

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.

cmeissner avatar Sep 15 '20 15:09 cmeissner

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

mdellweg avatar Sep 16 '20 08:09 mdellweg

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 avatar Sep 19 '20 11:09 ezr-ondrej

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

ezr-ondrej avatar Oct 15 '20 14:10 ezr-ondrej