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

Allow exceptions in the textfsm parsing process to be handled by caller #348

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

forrejam
Copy link
Contributor

Description

When using the convenience function response.textfsm_parse_output() to parse output from commands, there is currently no way to tell the difference between a parsing exception and an "empty" command output. For example the output from show vrf on a router with no vrfs will be empty and will return [], the same empty array will return if the vrf command fails to parse. There is currently a log message that will tell you something is wrong, however this cannot effect the control flow of the program allowing us to handle this error.

This change adds an optional raise_err field to textfsm_parse_output() to bubble the parsing exception up the stack. As its an optional field, existing code bases will continue to function as before.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tests have been run

Checklist:

  • My code follows the style guidelines of this project (no GitHub actions complaints! run make lint before
    committing!)
  • I have commented my code, pydocstyle and darglint are happy, docstrings are in google docstring format, and all
    docstrings include a summary, args, returns and raises fields (even if N/A)
  • I have added tests that prove my fix is effective or that my feature works, if adding "functional" tests please
    note if there are any considerations for the vrnetlab setup
  • New and existing unit tests pass locally with my changes

@carlmontanari
Copy link
Owner

Hey there! Quickly glancing at this and seems great! Will check it more tomorrow, thanks for the work @forrejam

@forrejam
Copy link
Contributor Author

forrejam commented Sep 16, 2024

Hi @carlmontanari no rush mate, let me know if you would rather do something more elegant like return a ScrapliTextFSM Exception or something like this. This was just the first cut of getting a solution that worked for me

@carlmontanari
Copy link
Owner

yeah, that's another good idea :) just pushed commit to raise the textfsm exception from the base scrapli exception. I think this should be fairly consistent scrapli-wide so we can always catch the base scrapli exception if you want to be lazy :D

if that looks good to ya we'll get it merged. thanks a bunch!

@forrejam
Copy link
Contributor Author

Looks good, thanks!

@carlmontanari carlmontanari merged commit 0f782b0 into carlmontanari:main Sep 16, 2024
12 checks passed
@forrejam forrejam deleted the raise_textfsm_err branch September 17, 2024 03:03
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.

2 participants