A few thoughts:
- I was thinking that a module template would be nice. I’ve been
using the user module for this purpose, but there could be an official
one. That would encourage modules to be written in a standard way,
could include logger functions, etc.
+1.
Modules outside of core don’t have to be written in Python, but we might
as well have one.
We should keep this in the core repo in the docs/ dir, most likely
and can maintain it as our preferences change.
We can also link this through the docs project.
This would encourage usage of state, failures if states don’t match, etc.
EXCELLENT idea.
- In relation to logging, personally I’d like to see the modules pass
more info back to the overlord when state changes. If standard
functions were used, the same logging function could do both.
So we already have stderr bubbling up to the overlord, for debugging purposes.
I want to maintain the changed True/False logic, but there is nothing saying
we can’t have a standard of an “events” return which contains a series
of log-like messages and include that in the normal JSON, if the module
would like to explain itself in a more standard way. This could be something
that --verbose decides to show, for instance, and I think that’s a great idea
It is important for debugging reasons (such as module failure) that we not
require a structure for stderr.
result = {
changed: True
events: [ ‘updated the foo’, ‘updated the splat’, ‘…’ ]
}
This would of course be totally optional so old modules wouldn’t cause any
problems with it.
- I’ve actually commented out the exec_command that does the logging,
as it was causing my system to hang. Haven’t had time to figure out
why.
Strange. If you find out why, let us know. Maybe some issue with the
logger program on Debian? Seems unlikely to me, but I’m all for removing
the explicit extra calls to logger regardless.
- Could we perhaps pass an optional #DEBUG parameter to modules, that
would cause debug level logging, both locally and bubbling back to the
overlord.
I don’t like the calling convention.
Thinking about this further, I also dislike the convention of having most modules recognize commented out
magic params, just because it increases the amount of boilerplate
they need to handle. This basically makes all the module source code start
to look like a giant hack. So I’m going to make the call that we not do this.
For the no log, that’s basically would have been:
log = True
if args.find(“#NO_LOG”):
log=False
args=args.replace(“#NO_LOG”,“”)
Since we already have the argsfile convention, perhaps it is better to extend the module
calling convention to be more like this:
module-name path-to-json-args-file 1/0
where that last 1/0 is the log/don’t log bit.
That’s much more standardized and easier for people writing modules to parse.
Special care must be taken when making sure this also works with async wrapper, which would
encourage us to not do this often. As a reminder, the async wrapper takes more options, so it would
also take that 1/0 log bit on the end and pass it along to what it wrapped.
Modules that didn’t understand the last parameter also wouldn’t have to change, which would
avoid breaking existing modules.
One thing I’m also going to require is if we decide to make any broad based changes for the log thing, or the events
thing, that in order to accept the patch we must update all existing modules to follow that convention. We have a
bit of a lag in some of the modules not using the json functions now and over time it will become more difficult to tell
which is a “proper” module to use as a starting point for coding style and so forth.
Sound ok?
–Michael