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

Add a RuleCollection object instead of a "loader" module #1063

Merged
merged 20 commits into from
Apr 5, 2021

Conversation

rw-access
Copy link
Contributor

Issues

None, just refactoring

Summary

Follow up from #1029

Created a RuleCollection which is a mutable collection of rules. You can add directories, load from a dict, load from TOML and all the things we normally do, and rules get added to the collection. I don't want to use class methods on the collection, because you always load into a collection. So instead of saying "get me a collection for this directory", you can ask for a collection with RuleCollection() and then call load_directory as a method on it. This gives more flexibility for what the caller can do.

I also noticed that are a ton of functions that allow you to specify rule name, id or file.
And that seems like it adds some annoying complexity and I wonder if there's a better way to do it.

Maybe make every CLI function work on a file, because that works well for autocomplete and is the most intuitive. And we can add a few new functions to lookup a file by name or by ID. Then there's one extra step in the process, but it simplifies a lot of the code base.

@rw-access rw-access added the python Internal python for the repository label Mar 24, 2021
rw-access and others added 3 commits March 24, 2021 13:40
Co-authored-by: Justin Ibarra <brokensound77@users.noreply.github.com>
rw-access and others added 2 commits March 24, 2021 14:50
Co-authored-by: Justin Ibarra <brokensound77@users.noreply.github.com>
Copy link
Collaborator

@brokensound77 brokensound77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, well done

@rw-access rw-access merged commit 6ed1a39 into elastic:main Apr 5, 2021
This was referenced Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Internal python for the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants