-
Notifications
You must be signed in to change notification settings - Fork 406
Adjusts parser for "show ap cdp neighbor" #948
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution.
A few things
-
Please include a unittest for this change. You can find existing unittests for this parser here
-
While I won't hold it against you, since you're updating this parser anyways I would appreciate if you could update line 1152 from
Optional("neighbor_ip_addresses"): list
toOptional("neighbor_ip_addresses"): ListOf(str)
as it's a bit out of date for what we expect from a parser these days
Thank you @ThomasJRyan, and apologies for the delay in response. I have pushed some additional commits that (hopefully correctly) addresses your feedback. Please let me know :) |
Looks like your changes are causing existing unittests to fail. This will need to be fixed up before we can merge. You can run the unittests locally by going into the |
Apologies for the delay in replying. I fully intend to get this fixed up so it can be merged. Thank you for the tip on how to run the tests locally!, I hadn't spend enough time to figure that out yet. |
I've looked into these most recent failures a little more but it's not clear to me the fix. When I run the tests "directly" by changing into the
Are you able to advise, @ThomasJRyan? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your branch is not up to date with main. Please merge with the latest.
Resolves CiscoTestAutomation#723 Adjust regex to include the ap neighbor IP as it is output on 9800 platform v17.x
rebased on top of main (dc5e9db). created a clean venv using python 3.12.6
cd into tests, running |
Description
Resolves #723
Adjust regex to include the ap neighbor IP as it is output on 9800 platform v17.x
Motivation and Context
The parser fails as it's not able to match when the ap cdp neighbor IP is in the same line as the other information.