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 GigX/X/X support and Duplex a-half interface support. #108

Merged
merged 6 commits into from
Apr 7, 2017

Conversation

jasonbarbee
Copy link
Contributor

Fix - Add GigX/X/X support - Catalyst Stack Support, like 2960X.
Fix - 2950s report half duplex as “a-half”.
Added variety of test results to the raw parser test.

Fix - Add GigX/X/X support - Catalyst Stack Support, like 2960X.
Fix - 2950s report half duplex as “a-half”.
Added variety of test results to the raw parser test.
@GGabriele
Copy link
Contributor

Just run pytest on the main folder and I saw that the current template is failing with your new raw data. Would you mind taking a look please?

Removed Fa0 entry from raw test.
@jasonbarbee
Copy link
Contributor Author

Sorry, I had fully run a test on it before a last minute change. At the last minute I decided about removing Fa0 management interface out of this PR. Which is not included in this PR.
The Parser does not include "fa\d+", I added then I I took it out but forgot to modify the raw file. But overall, it's not a common use case, and depending on the usage, may have unintended consequences, at least it did for me when I ran it. I'm open for more comment on that. It's easy to add in, but if people are using it, and bulk changing interfaces identified through this script it could break the management access.

@GGabriele
Copy link
Contributor

I think the problem is actually that you're modifying parsed and raw files but not fixing the template accordingly (or simply not including it in this PR :) )

I've just used this template and worked with and without the Fa0 interface

Value Required PORT (\S+)
Value NAME (\S+((\s\w+)+)?)
Value STATUS (\w+)
Value VLAN (\w+)
Value DUPLEX (a-full|auto|half|a-half)
Value SPEED (auto|\w+-\d+|\d+)
Value TYPE (.*)

Start
  ^${PORT}\s+${NAME}\s+${STATUS}\s+${VLAN}\s+${DUPLEX}\s+${SPEED}\s+${TYPE} -> Record
  ^${PORT}\s+${STATUS}\s+${VLAN}\s+${DUPLEX}\s+${SPEED}\s+${TYPE} -> Record

Original name string
Value NAME (\S+(\s\w+\s\w+)?)
fails to match descriptions with ‘ in the description like this
Gi2/0/46  Jerrick Kellum's P connected    16         a-full  a-100
10/100/1000BaseTX
Adjusted to .* to match symbols in the description field.
@jasonbarbee
Copy link
Contributor Author

I see the failure, I will try to run the test script on my side and post an update. I ran into a new change today and tried to work it in.
The current codebase failed to parse descriptions with ' in them, or at least in certain cases.

@itdependsnetworks
Copy link
Contributor

I think this may be a bit cleaner, let me know what you think?

Value PORT (\S+)
Value NAME (.{0,18}?)
Value STATUS (\w+)
Value VLAN (\w+)
Value DUPLEX (a-full|full|auto|half|a-half)
Value SPEED (auto|\w+-\d+|\d+)
Value TYPE (.*)

Start
  ^${PORT}\s+${NAME}\s+${STATUS}\s+${VLAN}\s+${DUPLEX}\s+${SPEED}\s+${TYPE}$$ -> Record

Thanks itdependsnetworks for the regex recommendation.
@jasonbarbee
Copy link
Contributor Author

Excellent - much more readable. I tested it locally, and have added it to the PR.

@GGabriele GGabriele merged commit 80832a4 into networktocode:master Apr 7, 2017
@GGabriele
Copy link
Contributor

Looks good to me, thanks @jasonbarbee !

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.

3 participants