A few weeks ago, I opened a github issue (although I should have sent an email to the list, which I am doing now), to see about addressing an issue that is starting to form, in regards to the AWS and Rax modules and shared documentation.
We have started pushing common code for these modules into ansible.module_utils.{ec2, rax} and largely this is around authentication standardization.
Basically, the docs for the modules, that use the common code, get copy/pasted into all new modules. Additionally the ‘notes’ portion of the DOCUMENTATION is identical or should be identical between all rax modules.
I’m not sure if it possible, but I am wondering if we could find a way to not have to duplicate all of those docs.
The current method for getting these docs, is to open() the module files, and use ‘ast’ to parse and loop through the files looking for DOCUMENTATION and EXAMPLES, and then if DOCUMENTATION is found, use yaml.safe_load to parse it. You can see this code in ansible.utils.module_docs.
This would make it complicated to share the code, since the files are not evaluated, just parsed, unless we also had a way of indicating that the docs generation should supplement the DOCUMENTATION string with a string found elsewhere. I think this is the “best” solution I have come up with, and I am more than happy to work on an implementation.
I just wanted to bring it up on the list first, to see if it is actually something that we want or is desirable.
import imp
rax = imp.load_source('library/cloud/rax')
# rax.DOCUMENTATION, rax.rax_slugify etc. are all
# available at this point
I don't get it why you need to yaml.safe_load anything... Just set
DOCUMENTATION to whatever is appropriate and Ansible will fire up Jinja2
templating engine later on. Right?
Note, that this would work iff Ansible modules required the following,
Python-proper, style:
# At the top of the module file
from ansible.module_utils.basic import *
# module contents...
if __name__ = "__main__":
main()
Actually, why doesn't Ansible do it this way? I can understand the
reasoning behind putting the import at the bottom, but I simply do not
understand why
if __name__ = "__main__":
is not there!
imp module has been available since Python 2.2.1, and there is no need
to do the ast parsing magic. I am more than happy to work on an
implementation!
I guess if we did actually evaluate the module, it could make it easier, but there would be a larger chance of issues that we could run into. Ideally, we would need to use ModuleReplacer to avoid additional problems. But simply not running main() on load, may not be sufficient enough to keep code from executing, but we could try to enforce this.
We also run into problems with people with custom modules that may be building docs for their internal use. We then “break” backwards compatibility.
I think something like adding an additional step after yaml.safe_load, and looking for special “pointers” of where to source additional data from could be the better solution. That additional data cloud live in the module_utils files.
I’ve been thinking about this some and am starting to see that this would be useful.
We’d want to do this in a way that ansible-doc could understand it, and there’s some common code there that the module formatter uses to emit the documentation.
As such, it may make sense that the datastructures used to overlay across the docs exist in the ansible python package, as code.
ansible/utils/module_docs_fragments.py
In which the overlays would be python datastructures representing common arguments.
Then in the module source itself, there would be a YAML element:
“extends_documentation_fragment: ‘rackspace’”
Which would blend the YAML docstring with the datastructure in the python package.
I don’t think it would be too hard to get done. Thoughts?
It would be a little arcane, but if we do it in the formatter code we’ll miss ansible-doc being able to use it.
My thoughts were to implement this functionality in utils/module_docs.py, using nearly exactly what you mention of using something like “extends_documentation_fragment: ‘rackspace’”
After the yaml.safe_load in get_docstring(), we could check if that key exists, and then .update() the dict with the fragment.
As an aside, for the Rackspace side of things, and it could become useful for others, I’d like to be able to create multiple base fragments. Basically, for those modules which can work with other OpenStack cloud and those that don’t. The recent rax_cdb is a “proprietary” implementation, so some attributes don’t necessarily apply. I don’t think this would be an issue, thinking about the implementation, I just wanted to mention it so that it is on our minds while implementing.
My only other thought would be allowing the “notes” section to be merged, instead of overwritten. So something like .extend() the notes, and .update() the rest.
I agree that it shouldn’t be too hard to get done.
If you’re tackling this, go ahead and we can fast-track reviews and getting it in whenever it arrives, if it’s instead something you’d rather we take, I can incorporate some basics over the next few weeks.