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

Replace confparse w netutils #1565

Merged

Conversation

jvanderaa
Copy link
Contributor

Fixes #1550

This replaces parsing with Netutils to handle parsing of configuration lines.

Testing thus far has been in a separate Python script that will get added as well to the PR. There were no previous tests written for the previous parser, so I found a basic BGP config for the first part to get the section bgp. The second and third configurations are tests with data from Cisco's documentation.

 python try_netutils_config_parse.py
>>> NAPALM CURRENT HELPER (SECTION PARSING)
['router bgp 65010', ' bgp log-neighbor-changes', ' network 192.168.1.0', ' neighbor 192.0.2.1 remote-as 65550']
['router bgp 65010', ' bgp log-neighbor-changes', ' network 192.168.1.0', ' neighbor 192.0.2.1 remote-as 65550']
<<<< NETUTILS HELPER

>>> NAPALM CURRENT HELPER (BGP PARSING)
[' bgp listen range 172.21.0.0/16 peer-group group172', ' neighbor group172 peer-group', ' neighbor group172 remote-as 45000', ' neighbor group172 activate']
[' bgp listen range 172.21.0.0/16 peer-group group172', ' neighbor group172 peer-group', ' neighbor group172 remote-as 45000', ' neighbor group172 activate']
<<<< NETUTILS HELPER

>>> NAPALM CURRENT HELPER (BGP Parsing)
[' address-family ipv4 multicast', ' address-family ipv4 vrf vpn1']
[' address-family ipv4 multicast', ' address-family ipv4 vrf vpn1']
<<<< NETUTILS HELPER

This shows the use cases that have been presented in ios.py around BGP parsing and section parsing.

Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

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

Except a minor change, this looks good to me.

Deferring to @ktbyers for more input.

napalm/base/helpers.py Outdated Show resolved Hide resolved
@ktbyers
Copy link
Contributor

ktbyers commented Feb 13, 2022

LGTM, let's remove that one logger.error line and we should be good to go.

Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

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

Noice!

@mirceaulinic mirceaulinic merged commit ff83c56 into napalm-automation:develop Feb 13, 2022
@z-8-d
Copy link

z-8-d commented Feb 14, 2022

May I ask when napalm could have a release(or a cherry-pick) for this change please?

@z-8-d
Copy link

z-8-d commented Feb 18, 2022

@mirceaulinic @ktbyers @jvanderaa - Would you please kindly show the plan when we could have the new release with the feature in this PR?

@ktbyers
Copy link
Contributor

ktbyers commented Feb 18, 2022

The next release depends on when Mircea gets a chance to do it. I think it should be shortly (i.e. in the next few weeks).

But the code is there i.e. just git clone the develop branch or git clone to the commit right after this is incorporated i.e. there is nothing stopping you from getting and using the code.

@z-8-d

@z-8-d
Copy link

z-8-d commented Feb 21, 2022

@ktbyers - Thanks for your information!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question about napalm License
4 participants