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

status regex includes non-whitespace #1941

Merged
merged 5 commits into from
Jan 6, 2025

Conversation

tfchristie-wi
Copy link
Contributor

No description provided.

@mjbear
Copy link
Contributor

mjbear commented Dec 20, 2024

resolves #1938

@mjbear
Copy link
Contributor

mjbear commented Dec 20, 2024

It doesn't appear that .* is too loose (greedy) based on how your test data behaves when parsed. 🙂

Copy link
Contributor

@jmcgill298 jmcgill298 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tfchristie-wi
Copy link
Contributor Author

The test that fails seems to be one where keywords are in the description/name.
I went back and looked at the regex for NAME and it is: Value NAME (.+?)
I am not even sure how to parse that. It seems like a field length: (.{18})\s would be more sane.
changing :?.*\s to :?.{0,9}\s seems to work as well.
NB: the test file: cisco_ios_show_interfaces_status_with_keywords_in_name.raw
has 3 lines for the same interface:

Fa0/4 uplink notconnect 123 auto auto 10/100BaseTX
Fa0/4 downlink notconnect 456 auto auto 10/100BaseTX
Fa0/4 suspended customer disabled 789 auto auto 10/100BaseTX

@mjbear
Copy link
Contributor

mjbear commented Dec 21, 2024

The test that fails seems to be one where keywords are in the description/name. I went back and looked at the regex for NAME and it is: Value NAME (.+?) I am not even sure how to parse that.

There are different lines for the various matches.
I'll take to this and submit some changes your way @tfchristie-wi

It seems like a field length: (.{18})\s would be more sane. changing :?.*\s to :?.{0,9}\s seems to work as well. NB: the test file: cisco_ios_show_interfaces_status_with_keywords_in_name.raw has 3 lines for the same interface:

I'll see what I can work up - hopefully something simple. 😁

Fa0/4 uplink notconnect 123 auto auto 10/100BaseTX Fa0/4 downlink notconnect 456 auto auto 10/100BaseTX Fa0/4 suspended customer disabled 789 auto auto 10/100BaseTX

It is mostly cosmetic that there are three of the same ... though it could be helpful to have them be unique port names.

* Add optional regex to capture TDR text if TDR is running,
  otherwise work as before with normal non-TDR output
* Swap out duplicate Fa0/4 port names for unique ones in raw output for
  ios show interfaces status with keywords
* So the match is more realistic and specifically a keyword with
  whitespace around it
@mjbear
Copy link
Contributor

mjbear commented Dec 21, 2024

@tfchristie-wi
I submitted tfchristie-wi/pull/1 for you to review.

I include the below workflow description in case you're new to git and GitHub.
Once you've reviewed my PR against your fix-issue-1938 branch, you can approve it which will merge my changes into your branch thus making it part of your upstream NTC templates PR.

Fix greedy regex when TDR text isn't present
@mjbear
Copy link
Contributor

mjbear commented Jan 3, 2025

@jmcgill298
This PR should be ready for re-review.
Once this is in I'll rebase and prepare another PR I have for this template. Thank you!

@jmcgill298 jmcgill298 merged commit 5235423 into networktocode:master Jan 6, 2025
9 of 12 checks passed
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.

cisco_ios_show_interfaces_status.textfsm throwing error when status has additional text
3 participants