mpenning / ciscoconfparse

Parse, Audit, Query, Build, and Modify Arista / Cisco / Juniper / Palo Alto / F5 configurations.
http://www.pennington.net/py/ciscoconfparse/
GNU General Public License v3.0
793 stars 220 forks source link

find_object_branches() suggestions #178

Closed andywhitaker closed 3 years ago

andywhitaker commented 4 years ago

I really like the new find_object_branches() method and just wanted to make two suggestions: 1) kwarg option to set a 0-indexed "start_level" such as 0 for root, 1 for a single "level' in, etc... None would be default and would operate is it currently does where the first pattern could be found on any "level" of indentation.

2) A keyword option as a boolean that would allow you to specify all regexes must match to be returned. Currently if any matches occur in the left-most patterns it will return a row with None for specific patterns that do not match. I iterate through the matches and do something like "if matches[-1]: do_something" for each row which works well. It would be nice to have perhaps a "strict" keyword or something similar where if all the regex patterns do not match it does not get returned and then we wouldn't have to parse the returned rows further to make sure all patterns matched.

So something like this:

p.find_object_branches(('pattern1', 'pattern2', 'pattern3'), start_level=2, strict=True)   # not sure I like "strict" the best as kwarg but IDK

Neither of these are a big deal but I just thought they would be nice features if you agree / wanted to consider them. Either way thanks for the great lib and all the hard work!

mpenning commented 4 years ago

Hi,

Apologies for the delay...

I'm not sure I see the need for the first index argument proposal.

I will add a keyword option for request number 2.

andywhitaker commented 4 years ago

Thanks! Sorry for my delay as well, I was off the grid several days on a trip.

Thanks for adding in number 2! I appreciate that!

I don't recall the specific use-case I was thinking for number 1. I believe I was thinking of parsing FortiOS configs where the levels can be repetitive in the keywords present at different levels. I believe I was thinking having a base-level might be speedier if you know you want to ignore items matched too low in the tree and make it so you don't have to define all levels as patterns to do so.

I'd say I agree that item number 1 should best be ignored for now. I can't think today of what specific use case really makes it necessary and even if so it could be enough of an edge-case it makes sense to just handle on my own side.

Thanks again for considering adding number 2. That one I definitely will be using! Thanks!

mpenning commented 3 years ago

Hello @andywhitaker ,

I just pushed v1.5.23, which has a new find_object_branches() option... allow_none=False will reject any branches with None in the value...

This is example usage...

branchspec = (r"^config", r"^\s+interfaces", r"loopback")

parse = CiscoConfParse("/path/to/bracedelimited/config", syntax="junos")
branches = parse.find_object_branches(branchspec, allow_none=False)
andywhitaker commented 3 years ago

@mpenning, first big thanks for spending the time doing this! I'm sorry to say though that it does not appear to be working for me. I downloaded 1.5.23 via pip and this is my result:

image

Thanks again for all you do!

andywhitaker commented 3 years ago

Just an FYI I did just download the current master branch from the repo here and pip install -e'd that into a new venv and verified I'm getting the same result so not just what is pushed to pip. Just wanted to save you some time looking at it. Thanks!

mpenning commented 3 years ago

@andywhitaker please test version 1.5.24 and let me know if there's still a problem... I will add this case to unittests after I confirm it's fixed now...

andywhitaker commented 3 years ago

@mpenning, looks like it's working great now! Thanks so much!