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

Filter object #155

Merged

Conversation

targuan
Copy link

@targuan targuan commented Jun 11, 2018

As discussed in #152, this is a basic implementation of Django's Q object filtering schema.
In order to use it, one have to construct a filtering object F which will represent how one want to filter.
The objects can be combined with the logicals operators

  • & which allow object matching the conditions of both F object
  • | which allow object matching the conditions of one of the F object
  • ~ which allow object matching none of the conditions of the F object

Tests are quite explicit I think but I can clarify if needed.
I have integrated the others django operators (mostly the contains operator) as in #152.

This implementation is done in the plugins section as I think as this as a helper to filter with more granularity than the basic key=value method.
If for consistency reason (or other reasons) the F filtering method should be kept as the only method (by converting the **kwargs to a F object) it can be moved to the core.

@coveralls
Copy link

coveralls commented Jun 11, 2018

Pull Request Test Coverage Report for Build 698

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.5%) to 92.11%

Totals Coverage Status
Change from base Build 693: -0.5%
Covered Lines: 1004
Relevant Lines: 1090

💛 - Coveralls

@@ -0,0 +1,79 @@
import copy
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should consider this core functionality so I'd place it under nornir/core/filter.py

@@ -0,0 +1,43 @@
from nornir.plugins.inventory.helpers.filters import F
Copy link
Contributor

Choose a reason for hiding this comment

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

As per above comment this should be "tests/core/test_filter.py".

Also, please, follow the same pattern as with the rest of the tests, please.

@dbarrosop
Copy link
Contributor

Overall looks great, I have to do some more testing of my own and play a bit with yet but I think this is the right direction :)

@targuan
Copy link
Author

targuan commented Jun 27, 2018

I moved the filter to nornir/core.
Can you check if the updated tests are what you had in mind.

follow the same pattern as with the rest of the tests, please.

@dbarrosop
Copy link
Contributor

Awesome work! :D

I am going to merge it into a branch and play a bit with it as I add a note in the tutorial about this :)

@dbarrosop dbarrosop changed the base branch from develop to filter_object July 10, 2018 12:56
@dbarrosop dbarrosop changed the title [Open for discussion] Feature/filter helper Filter object Jul 10, 2018
@dbarrosop dbarrosop merged commit 1e4b493 into nornir-automation:filter_object Jul 10, 2018
@ktbyers
Copy link
Collaborator

ktbyers commented Jul 10, 2018

Thanks for doing this work @targuan !

dbarrosop pushed a commit that referenced this pull request Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants