Allow using OrderedDict when parsing the yaml files.

[smoothify]

I am would like the ability to set OrderedDicts as the default dictionary class when parsing yaml files, primarily for the purposes of allowing dictionaries in vars to be automatically ordered.

Using Ansible as an API, and building a wrapper around Playbook, I can mostly achieve this, however there are are few issues, that make it complicated.

So I wanted to ask here if there was any interest in allowing OrderedDicts directly in Ansible (e.g. with a config option to enable it) , or failing that whether it was possible to merge in some changes that are blocking me from taking this approach even when using Ansible as an API.

Here are the areas affected:

1) Numerous conditions in various Ansible files that check if a value == dict or != dict, rather than using is_instance(value, dict) therefore subclasses of dict will not work. Is there a particular reason why these checks are so specific?

2) If ordered dicts are used then, it will be necessary to pass in the OrdereredDict class as a global when using utils.safe_eval(), so when a template returns the string form of an OrderedDict it can be evaluated correctly.

3) Finally, if points 1 + 2 are resolved, then some yaml constructors could be added that allow the option of setting the default dict class as OrderedDict for all yaml files. 

Interested to get feedback on this, and more than willing to create the relevant pull requests if required.

[James Cammarata]

So one of the issues with using OrderedDict is that it was only introduced in python 2.7 - we'd have to vendor in the OrderedDict class (or require python-orderedict be installed), which is something we have avoided in the past.

Beyond that, how would you make the YAML loader work with the OrderedDict? Something like this I assume: http://stackoverflow.com/questions/5121931/in-python-how-can-you-load-yaml-mappings-as-ordereddicts

--
You received this message because you are subscribed to the Google Groups "Ansible Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ansible-deve...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[smoothify]

Hi James,

I do see the issue with the OrderedDict class, my initial thinking was it is would be off by default, and would only be attempted to load the class if the config variable was set. If it couldn't load and the config variable was set, it would just return an error.

I did notice that salt stack do vendor in the class fwiw:

https://github.com/saltstack/salt/blob/develop/salt/utils/odict.py

Regarding the yaml loader, yes it would be one of the approaches on that stackoverflow page - I am currently using the second approach, haven't evaluated the first yet.

In my wrapper around ansible playbook, points 1 and 2 are the big blockers, as working around them involves overriding lots of code, or dirty hacks!

[James Cammarata]

I'm absolutely in favor of cleaning up use of type() for isinstance() - that should be done anyway. 

As for point #2, when I was recently rewriting safe_eval I had originally used OrderedDict, which is why I knew it was a 2.7-only feature :) So I'm not totally against the idea of vendoring that in, we'd just have to discuss it. That code appears to be very stable and hasn't changed much in the last 3 years, and having OrderedDicts available would be nice in some places where we've otherwise avoided using them.

[smoothify]

I've created a pull request for point #1 as that seems to be the low hanging fruit here.

https://github.com/ansible/ansible/pull/7831

As mentioned there, I also did the list checks too, as I thought it might be best to do it all at once. Let me know if you want me to split the pull request up.

[Michael DeHaan]

Let's step back and talk about use cases here first, and then understand what problems we are trying to solve.

From then, we can talk about implementation aspects.

Generally the nature of dicts is that order shouldn't matter.

So I need to understand the why here.

[smoothify]

The primary reason why I would like to be able to use OrderedDicts is for nginx configuration.

I'm creating a comprehensive nginx config role for vhosts and location blocks, but the order of directives can be very important.

For instance this dict here, I need the try_files to be at the bottom.

- location: '~* /imagecache/'

  access_log: off

  expires: '30d'

  try_files: '$uri @drupal'

For clarity purposes I'd prefer to use this layout than having to use a list of dicts,  and since I want to make the config flexible, i don't want to hard code the order of everything in the templates.

As I mentioned in my earlier post, I fully understand why this shouldn't be the default.

[Michael DeHaan]

Perhaps a good approach here would be a list of hashes, so order is preserved

params:

   - { location: blah }

   - { access_log: blah }

Though it seems like it would be easier to just deal with this in the template and get out specific values out of the structure.

While you say this is hard coding, it would also allow you to not have to preserve order where you have the settings.

[smoothify]

Hi Michael

Yes, there are ways to get around it, however that was just a small example, the full requirement allows a hierarchical structure that can go several layers deep.  

As I mentioned before, I am utilising Ansible as an API, so perhaps it's not a direct use case for users of Ansible itself. I'm just finding the explicit dict type checking a bit too restrictive.

Regarding the earlier pull request, is that something you would consider in principle? It doesn't change any behaviour, just adds consistent and flexible type checking.

[Michael DeHaan]

Type checks look ok.  I'm a little unclear on the move of the HostVars import.

[smoothify]

Hi Michael,

It's not a move as such, i'm introducing the import.

Currently, the code there assumes that if its an instance of a dict but not actually a base dict, then it must be a HostVars, so it returns as it is. (otherwise it processes the keys for template values). 

So, I want to introduce an explicit check for the HostVars class, which if when using isinstance() requires an import. I can't put the the import at the top, as that causes an exception due to a circular import scenario (runner already imports utils.template for obvious reasons).

The three ways I can think of to fix this are:

1) scoped import like i've done.

2) use var.__name__ == 'HostVars' to type check without import.

3) move HostVars class to a different file.

[Michael DeHaan]

Understood, it's in queue and we can take a look when we get to it.

Thanks!