Add syslog-ng state and execution module #14033
Conversation
|
Test Failed. If the failures are unrelated to your code, don't stress, a core developer will know these apart. |
| s_gsoc2014: | ||
| syslog_ng.config: | ||
| - config: | ||
| source: |
There was a problem hiding this comment.
Hmm, nested dicts like this can be confusing for users, if a data structure that is not as deeply nested works or uses a list can be used it would be better
There was a problem hiding this comment.
I tried to find the most natural representation of a syslog-ng configuration as a YAML document. I use this kind of format, because syslog-ng's configuration can be deeply nested.
|
This is fantastic! Please take a look at the test errors so we can get this merged in! |
|
Test Failed. If the failures are unrelated to your code, don't stress, a core developer will know these apart. |
|
Test Failed. If the failures are unrelated to your code, don't stress, a core developer will know these apart. |
|
Test Failed. If the failures are unrelated to your code, don't stress, a core developer will know these apart. |
|
As far as I know the failure isn't caused by my patch. Could you check it, please? |
Add syslog-ng state and execution module
|
Looks good! Thanks! |
|
I've made some style fixes (see the coding style guide) in this pull request. @ihrwein Can you please address why this state is full of print statements? States should never, EVER, print. They should return data, which is then stored in the job cache and also printed by the master. See here for more info. Also, I noticed that this state has a number of functions which don't appear to be stateful tasks at all:
These all seem like actions that should be taken in the execution module, not in the state. The same goes for most of the private functions (those beginning with an underscore). The general rule in Salt is that execution modules do the work, and states just leverage functions from the execution modules to do things. The states should be little more than logic and calls to execution functions. |
|
I talked with @thatch45 and we decided to move this state and execution module to salt-contrib so it can be refined there. Once all issues are addressed, we'll definitely move it back. Once this is merged, the state and execution module will be merged into salt-contrib and you can make further pull requests against that repository. Thanks for all the work you put into this. We're excited to get this added to Salt, we just need it to be refined a bit. |
|
Thanks you for your reply! About print statementsYou can see, that I don't use stdin or stderr in print statement, I just fill a buffer with it, there is no exception. Is this OK? About the mentioned functionsI think I had some misconception here. I will move all functions to the execution module except config (the real logic will be in the exec. module). |
|
I think I refined it, could you take a look at the result? Should I make After the merge, I would update the docs in salt's develop branch. 2014-07-12 18:45 GMT+02:00 Erik Johnson notifications@github.com:
|
Syslog-ng is an open source logging daemon. This pull request contains a state and an execution modul for it.
The state module is able to generate syslog-ng configuration from YAML format, and start, stop or reload syslog-ng. The commit id of the patch is: 80f84196231db7f1a232194e3dcb6eacb3ae3583.
The execution module can check the version of the generated configuration or get some information about syslog-ng, e.g. version.
This pull request requires #14032 to be accepted.