-
Notifications
You must be signed in to change notification settings - Fork 760
Add Cisco IOS show vlan group #2093
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: master
Are you sure you want to change the base?
Add Cisco IOS show vlan group #2093
Conversation
Please let me know if there is a better way to handle vlan output like this in textfsm
- vlan_group_name: "MULTI_VLAN_TEST_2"
vlan_ids: "761-763, 197, 116, 118" |
Since those VLAN IDs are all on one line, the string capture as you have it is probably the simplest route. If they were on different lines we could have had the chance to use |
Value Required VLAN_GROUP_NAME (\S+) | ||
Value VLAN_IDS (\d+.*) | ||
|
||
Start | ||
^vlan\s+group\s+${VLAN_GROUP_NAME}\s+:\s+${VLAN_IDS}$$ -> Record |
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.
I thought about this for a bit ... we've normalized/standardized on VLAN_ID
in other templates. For the raw output here there can be one or more VLAN IDs, but that holds true for other templates where VLAN_ID
is used (cisco_ios example).
If you agree, please click the "commit suggestions" button to pull my suggestion into your pull request. Thank you!
Value Required VLAN_GROUP_NAME (\S+) | |
Value VLAN_IDS (\d+.*) | |
Start | |
^vlan\s+group\s+${VLAN_GROUP_NAME}\s+:\s+${VLAN_IDS}$$ -> Record | |
Value Required VLAN_GROUP_NAME (\S+) | |
Value VLAN_ID (\d+.*) | |
Start | |
^vlan\s+group\s+${VLAN_GROUP_NAME}\s+:\s+${VLAN_ID}$$ -> Record |
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.
Yep that's fine. The only thing I would say about the use of VLAN_ID in other templates is usually you can rely on it being a single vlan (\d+) so I thought using a different identifier would prompt the user that it will need more post processing (which is what I'm doing in production) in the event of a vlan range. But I can see in the example you linked its not always the case.
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.
so I thought using a different identifier would prompt the user that it will need more post processing (which is what I'm doing in production) in the event of a vlan range.
You have a good point there to differentiate and call out the need for post processing.
We can certainly get some input from others.
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.
The more I think about it, I ask "how often is a vlan group a single VLAN ID?"
Probably less frequently a single than multiple VLAN IDs.
That IOS template I linked to where there were multiple IDs is probably more an "exception" for that particular command output.
In this use case, it seems more likely that a vlan group will contain multiple VLAN IDs in practice. While the parsed capture group will be a string and not multiple separate values, naming the capture group with a plural indicates the parsed data likely contains more than one VLAN ID.
I'm in agreement with you @forrejam, my thought is it may be best to make it plural VLAN_IDS
for clarity/post-processing reasons.
Thank you for being patient and allowing me time to revisit this. 😅
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.
No problem @mjbear, either way will work for me, but I think VLAN_IDS
is more fitting in this instance. Perhaps VLAN_IDS
can become convention for other similar contexts.
Co-authored-by: Michael Bear <[email protected]>
Add support for "show vlan group" on cisco IOS