-
Notifications
You must be signed in to change notification settings - Fork 70
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 for issue #286 - traceback when banner section contains white-spaces lines #291
fix for issue #286 - traceback when banner section contains white-spaces lines #291
Conversation
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.
Looks good to me. The single updated test file handles all the edge cases?
hmm, good point. Currently, the updated UT file handles only blank lines (empty and with the white spaces). In order to handle rogue configs, I'd need to come up with the negative test case. Will do soon |
@mharista, I have added a false positive case to UT fixture file to cover the case, please review |
I've ran also systest (with the latest update) - all have passed |
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.
LGTM
this PR fixes issue #286
The issue has been introduced with PR#220 - that PR introduced cached parsing of the running config, which is mostly indent-structured text, however, we entirely overlooked a banner section.
Unlike the rest of the config, the banner section(s) is a free text (i.e. unstructured) and therefore may produce:
The banner is now parsed into a separate section.