(Developers): Module code sharing implemented, request for comment, plans going forward

WARNING: exciting but uber-technical internals discussion. If you're just using the app and not hacking on it, you may ignore this.

I have implemented a method for Ansible to reuse code in modules without first having to transfer any dependencies and not have a build time step, both things
I really don't want to have. This is pretty cool actually.

For a preliminary example, see here:

https://github.com/ansible/ansible/blob/devel/library/slurp
(See this is really short!)

What this means is that when the string #<<INCLUDE_ANSIBLE_MODULE_COMMON>> occurs in a Python file, it will be automatically substituted by Ansible. No code generation is required, which means we aren't going to have a build step and people can just continue to run things from checkout.

The code behind this lives in https://github.com/ansible/ansible/blob/devel/lib/ansible/module_common.py

and that is what is automatically injected into each module.

You can do all sorts of things to specify required variables, valid choices for variable values, and aliases for variables.

For instance, this works:

module = AnsibleModule(
    argument_spec = dict(
        alpha = dict(required=True),
        beta = dict(required=True, choices=['a','b','c']),
        gamma = dict(required=True, aliases=['delta', 'epsilon']),
        omega = dict(default='last')
    )
)

module.exit_json(
    alpha=module.params.get('alpha','?'),
    beta=module.params.get('beta','?'),
    gamma=module.params.get('gamma', '?'),
    omega=module.params.get('omega', '?')
)

There is also a fail_json function. module.params is considered "supported" in the API, and returns the argument dictionary.

There's not much else. There may be a few more things added later, like md5 helper functions and so forth, but we'll consider that carefully.

Given that the base class is responsible for required arguments, aliases, and so forth, each module then only has to do the basic checks for things like file existence, ensuring that messaging is consistent.

I'd like those who develop modules in core to take a look at see if these classes look reasonably acceptable. If they are, we can begin simplifying existing modules to all use these functions.

Once this stabilizes, and 0.6 is released, this will allow for cleaning up the module authoring documentation, explaining both this and how to do it the
hard way.

Note that the "MODULE_COMMON" stuff is going to become a rather strict API. Any method that is not private (does not start with "_", can never be removed or change in signature, other than adding optional key=value arguments on the end of the call chain. This means we need to get it mostly right now, so if you can think of any capabilities this can't do, or anything that needs to be added to the argument_spec, we should do it now.

The command module is obviously a bit different in argument parsing. I see the command module subclassing the AnsibleModule class. Actually, that's exactly what it will do.

The "file" module is currently written such that it does not require python_simplejson, such that ansible can be used to install python-simplejson by way of adding an EPEL configuration file for yum, etc. This is primarily a thing for RHEL and CentOS 4, and I honestly don't think we need to keep that up.

The raw module can still be used to install RPMs and bootstrap python-simplejson in fringe cases.

If you want to test library/slurp now, hacking/test-module has been adapted to understand this, as has ansible proper. If you run hacking/env-setup it will essentially code-generate on the fly, and if you need to dig inside the file it generates, it will tell you where it saves it.

Comments welcome.

--Michael

I wonder if it will be helpful to have a “help” attribute for each option that the module can accept. For example

We're not going to do this, because documentation maintained in two places is likely to drift, and I want the web docs to be authoritative.

(aside: These will probably get versioned at some point in the future, so you can see docs based on the release)

--Michael

Just made a slight tweak to this, by inserting a main function, which
is a good thing to have anyway, and including the boilerplate at the
bottom of the file versus the top, tracebacks should they occur, are
no longer subject to offset shifts in the line numbers. This is a
pretty nice win.

The decision makes sense for modules that are one day going to be a core of ansible. What about custom modules that people write inside their company? The only way for them to figure out what the module does is to go through the code and figure out the issue.

I would suggest that the module owner be able to specify a link to the module documentation that gets printed along with error messages. That would really be a nice little thing to have.

Something like

I would suggest that the module owner be able to specify a link to the
module documentation that gets printed along with error messages. That
would really be a nice little thing to have.

+1

        -JP

Picking a particular host and contacting it remotely to ask it about documentation links seems a little strange to me, and I don't really want to add the extra plumbing for that.

I'd definitely recommend having a Wiki page about ansible or something at your company and you could put the docs there.

What I’m suggesting is that we simply print the link to the documentation for a module. For example, I may have a wiki inside my company documenting the module but just want the module to print that link if not used as expected.

The doc_link param can be optional and can be used by the author to supply the doc link to ansible. I don’t think this is much of work. All one needs to do is append the doc_link to the failure message.

Yeah, we're not going to do this, sorry.

BTW, if you want to add this for your company modules, you can of course subclass AnsibleModule for your company and override the fail_json method. Not hard to do.

You could, probably fairly easy, generate the doc for the web from this data, though.

/andreas

That time may come later.

To get this back on track -- I wanted to talk about argument parsing and results returning, and whether the class fulfills perceived requirements for what Ansible *currently* does.

Anybody developing modules (or patching them) have feedback on that part?

Anybody developing modules (or patching them) have feedback on that part?

I’ll look at it today, Michael.

This is more of a cosmetic thing, but could the shared module code be read from a file instead of a string within a file?

That way IDEs correctly highlight the shared code, and code intelligence will recognize it as Python code and not as a comment.

  • Mark

Sorry, having it in the Python code makes it run nicely from checkout, and I don’t want to have to keep doing os.read calls on it, etc.

To get this back on track – I wanted to talk about argument parsing and results returning, and whether the class fulfills perceived requirements for what Ansible currently does.

I like the arg checking and testing for msg in error responses.

I think, given the syntax of params, we should always use (k, v) = x.split(“=”,1) instead of (k, v) = x.split(“=”) for those rare times when a param contains an ‘=’.

I like that it’s minimalist. In particular, leaving error handling particulars up to the implementer.

I think, given the syntax of params, we should always use (k, v) = x.split(“=”,1) instead of (k, v) = x.split(“=”) for those rare times when a param contains an ‘=’.

Definitely. send me a patch.

This is more of a cosmetic thing, but could the shared module code be read from a file instead of a string within a file?

Sorry, having it in the Python code makes it run nicely from checkout, and I don’t want to have to keep doing os.read calls on it, etc.

I agree with Mark here: if it’s source code, keep it source code as long as possible.

I’ve been on about 5 Python projects that decided they could do it better than ‘import.’ Usually the initial reasons seem provocative. Usually it ends up being a net loss.

I might be missing something on the remote end here, but I’d recommend, copying ansible_common.py to the same directory as the module, assuming modules cd to their directory before running.

I might be missing something on the remote end here, but I'd recommend,
copying ansible_common.py to the same directory as the module, assuming
modules cd to their directory before running.

With all due respect, this is really bikeshedding.

This is not a module, I use a non-colored console all the time, and
you will live.

Please recommend things when they really really really really matter.

This does not.

Michael,

What this means is that when the string #<<INCLUDE_ANSIBLE_MODULE_COMMON>> occurs

Just wanted to say Thanks for this. Wading through bits of copy/pasted
code in the modules used to be a pain. If I look at the "copy" and the
"mysql*" modules they suddenly look very easy. :slight_smile: A world of difference.

        -JP