-
Notifications
You must be signed in to change notification settings - Fork 727
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
fix: Mikrotik interface print detail
#1867
fix: Mikrotik interface print detail
#1867
Conversation
Update the Mikrotik ```interface print detail``` textfsm template to support routeros 7 changes. New supporting test validation & updated old test yaml to ensure backwards compatibility
@@ -3,27 +3,30 @@ Value DYNAMIC (D) | |||
Value STATUS (X|R) | |||
Value SLAVE (S) | |||
Value NAME (\S+) | |||
Value List DESCRIPTION ((?!\s*$).+[^\s]) | |||
Value List DESCRIPTION (.+) |
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.
Value List DESCRIPTION (.+) | |
Value List DESCRIPTION (.+?) |
avoid accidently grabbing spaces
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.
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.
That looks to do exactly what I wanted it to do. As the template currently stands, it is capturing trailing space, but with the ?
it is capturing only up to the trailing space.
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.
actually, looking at that example, we also want to put a \s*
in front of the description capture to avoid catching any leading whitespace
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.
Oh yes I see. I'll make those changes and update the test files and commit them.
Remove leading & trailing whitespace in multiline comments
@@ -3,27 +3,30 @@ Value DYNAMIC (D) | |||
Value STATUS (X|R) | |||
Value SLAVE (S) | |||
Value NAME (\S+) | |||
Value List DESCRIPTION ((?!\s*$).+[^\s]) | |||
Value List DESCRIPTION (\s*.+?) |
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.
Value List DESCRIPTION (\s*.+?) | |
Value List DESCRIPTION (.+?) |
My statement was a little unclear about this, the leading space should be caught outside of the capture group, this will fix the multiline parsing in the test 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.
Sorry for the all the back and forth with this PR.
So applying these recommendations on my local test environment & the online tool https://textfsm.nornir.tech/ (see screenshot for example). It doesn't drop the last "comment".
What does, and probably the wrong way to go about it, but does drop it is "Value List DESCRIPTION ((?:\S).+?)". Does that seem reasonable?
description: | ||
- "very very long" | ||
- "multiline description" | ||
- " " |
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.
- " " |
^${DESCRIPTION}\s*$$ | ||
^\s*$$ |
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.
^${DESCRIPTION}\s*$$ | |
^\s*$$ | |
^\s*$$ | |
^\s*${DESCRIPTION}\s*$$ |
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.
This update should fix it
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.
That did fix it, thank you!. I've committed those changes.
Update the Mikrotik
interface print detail
textfsm template to support routeros 7 changes.New supporting test validation & updated old test yaml to ensure backwards compatibility with added vrf value