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.

Sign up for GitHub

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

Jump to bottom

XR SHOW CONTROLLERS FABRIC FIA ERRORS INGRESS: Add new template #358

Merged
merged 5 commits into from
Feb 23, 2019
Merged

XR SHOW CONTROLLERS FABRIC FIA ERRORS INGRESS: Add new template #358

merged 5 commits into from
Feb 23, 2019

Conversation

jmcgill298
Copy link
Contributor

ISSUE TYPE
  • New Template Pull Request
COMPONENT

cisco_xr_show_controllers_fabric_fia_errors_ingress_location

SUMMARY

Add new template

vaneuk and others added 3 commits January 26, 2019 17:09
@vaneuk @jmcgill298
XR SHOW CONTROLLERS FIA ERRORS INGRESS: Add new template
d97b5b4
@jmcgill298
XR SHOW CONTROLLERS FABRIC FIA ERRORS INGRESS: Add new template
d04e8d3 
GENERAL UPDATES:
- Change whitespace to use `\s+`
- Change `Record` to happen on "FIA" line
@jmcgill298
XR SHOW CONTROLLERS FABRIC FIA ERRORS INGRESS: Redesign capture groups
63d3e25 
CHANGES:
- Change variable data to be a list
  * Allows for platforms with additional capacity to still work
  * Changed capture groups:
    `TO_XBAR_UC_CRC_$variable` -> `TO_XBAR_UNICAST_LOCATION` & `TO_XBAR_UNICAST_CRC_COUNT`
    `TO_XBAR_MC_CRC_$variable` -> `TO_XBAR_MULTICAST_LOCATION` & `TO_XBAR_MULTICAST_CRC_COUNT`

TEST FILES:
- Update parsed file to use lists with new capture group names
@jmcgill298
Copy link
Contributor Author

@vaneuk My last commit to this branch makes some big changes. I think it makes sense to capture the data as lists in order to account for different architectures. Please let me know what you think of my data representation.

@vaneuk
Copy link
Contributor

vaneuk commented Jan 28, 2019

@jmcgill298 please give me a moment to think about this.

@jmcgill298
Copy link
Contributor Author

@vaneuk have you had a chance to look at this more?

@vaneuk
Copy link
Contributor

vaneuk commented Feb 4, 2019

@jmcgill298 i checked the changes. i do not like Lists in this case for the following reasons:

  • Now some counters are integers and some a lists. It makes parsed data harder to use in monitoring systems. For example when all counters are integers you can easily sum all of the counters and create an alert based on the sum value. And when some counters are actually lists you will have to convert lists to integer first.
  • Adding Lists does not make regex template really much more flexible. Output of command is platform dependent. The tests that i provided in my commit were made for Typhoon cards. And output for Trident cards is completely different.

please let me know if the points above make sense

@jmcgill298
Copy link
Contributor Author

@vaneuk I have reverted that commit

@jmcgill298 jmcgill298 merged commit 8564e7b into master Feb 23, 2019
This was referenced Feb 23, 2019
Merged
Merged
Closed
@jmcgill298 jmcgill298 deleted the xr_fia_errors_ingress branch February 23, 2019 16:11
jvanderaa pushed a commit that referenced this pull request Nov 10, 2021
@jmcgill298
XR SHOW CONTROLLERS FABRIC FIA ERRORS INGRESS: Add new template (#358)
99604e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jmcgill298 @vaneuk