Skip to content
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

Closed
wants to merge 5 commits into from

Conversation

rb83
Copy link
Contributor

@rb83 rb83 commented Apr 25, 2019

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.

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.
@rb83 rb83 marked this pull request as ready for review April 26, 2019 13:24
@netniV
Copy link
Member

netniV commented Apr 26, 2019

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.

@netniV netniV self-assigned this Apr 26, 2019
@netniV netniV added this to the v1.3.0 milestone Apr 26, 2019
@netniV netniV added CLI CLI related issue enhancement General tag for an enhancement labels Apr 26, 2019
@rb83
Copy link
Contributor Author

rb83 commented Apr 26, 2019

Sure, it's GPL, so you can already do whatever you want with/to it.

@cigamit
Copy link
Member

cigamit commented May 11, 2019

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.

cigamit added a commit that referenced this pull request May 11, 2019
Allow application of automation rules on CLI
@cigamit
Copy link
Member

cigamit commented May 11, 2019

Resolved this a bit differently. Thanks for you work regardless. One of these days we will move to getopt().

@cigamit cigamit closed this May 11, 2019
@rb83
Copy link
Contributor Author

rb83 commented May 13, 2019

@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.)

@netniV
Copy link
Member

netniV commented May 16, 2019

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.)

@netniV netniV removed this from the v1.3.0 milestone Sep 2, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLI CLI related issue enhancement General tag for an enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants