-
-
Notifications
You must be signed in to change notification settings - Fork 407
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow application of automation rules on CLI #2646
Conversation
This bring a first version of a CLI script which allows to apply automation rules to devices. This comes in handy for large(ish) installations that additionally make use of numerous automation rules (and possibly not all of them being optimized), where using this through the web interface often results in timeouts and/or incomplete runs, or would require crazy high max_execution_time settings. This currently simply wraps around automation_update_device(), for which logging is available through cacti_log() - no output will be sent to stdout.
This completes the first version of the CLI script to apply automation rules to devices. Devices can either be specified directly by their ids, or through SQL LIKE-compatible expressions on hostname and description.
I love the idea of this script but we would want to make quite a few changes to it for coding style, copyright and other minor tweaks. In order to do that though, would you be willing to allow us to assume control/copyright over the script? Also, due to the nature of the change I would be placing this script into the 1.3 branch which we have started, as it should be properly tested since it's a new feature. If we put it straight into the 1.2 branch, we'd have to support it on all systems once the next patch was released and we've all been a little busy this past month so better safe than sorry. |
Sure, it's GPL, so you can already do whatever you want with/to it. |
This function, though very nice, has to be reworked to follow are somewhat arcane coding standard. So, it might not get merged, but it will be incorporated, although coded somewhat differently. I hope that is okay. |
Allow application of automation rules on CLI
Resolved this a bit differently. Thanks for you work regardless. One of these days we will move to getopt(). |
@cigamit Uhm. Not commenting on the code; since that in the end boils down to taste, more or less. But completely erasing the commit history and hence scrubbing all author information is...well. Let's say not nice. (Especially after I've gone through the trouble to hand over copyright, etc.) |
Many changes in the code are suggestions from others where they provide it merely as patch info so we have to manually add it. When people provide a merge that can be committed, that is normally done with minor tweaks afterwards and is the normal process we go through. In the case of your code, it was deemed more work to correct the coding/styling/naming to our standards than simple implement the idea ourselves given we liked your idea, so we did just. I'm not sure why you believe that there was any disrespect or insult in doing that, it was a logical choice. Your idea was still kept, you had already allow us to publish as we saw fit and this pull request exists that shows the author of the idea plus the request number is used in the commit that shows this full history. The code format we have is not just down to taste but a set of predefined standards that we have agreed amongst the group and can be reviewed in both the existing code plus the documentation that actually covers it. (btw, typing all this is quite painful for me at the moment as I've just had am operation on my right hand so I do hope you see the importance we feel in ensuring that our contributors feel happy and will continue to do so in the future.) |
This brings a CLI script which allows to apply automation rules to devices. This comes in handy for large(ish) installations that additionally make use of numerous automation rules (and possibly not all of them being optimized), where using this through the web interface often results in timeouts and/or incomplete runs, or
would require crazy high max_execution_time settings.
Tested on a 1.1.38 and a 1.2.2 installation, rebased commits against develop branch.