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

adding juniper_junos_show_system_processes_summary.textfsm support #1921

Conversation

jnicholson56
Copy link
Contributor

Adding support for Juniper Junos show system processes summary command.

@mjbear
Copy link
Contributor

mjbear commented Jan 3, 2025

@jnicholson56
You're probably getting back from a winter break. 😀
Heads up that there are a few suggestions on this junos show system processes summary template that are pending your review.

Thank you!

@jnicholson56
Copy link
Contributor Author

jnicholson56 commented Jan 3, 2025 via email

@jnicholson56
Copy link
Contributor Author

I updated the files based on the requested changes

@mjbear
Copy link
Contributor

mjbear commented Jan 4, 2025

I updated the files based on the requested changes

Thank you.
I'm going to focus on PR #1922 since it seems to be an overall simpler template.
We can swing back around to this one once that one is ready to go.

@jnicholson56
Copy link
Contributor Author

jnicholson56 commented Jan 5, 2025 via email

@mjbear
Copy link
Contributor

mjbear commented Jan 5, 2025

That works for me. I have a lot of templates to add eventually and doing them one at a time is probably for the best. I suspect they are all going to need work before they are accepted ☺️

There was a learning curve for me too where the group was asking me to change regexes.

You'll figure out the common suggestions and be able to proactively tweak templates. 😉

…mmary.textfsm

Co-authored-by: Michael Bear <38406045+mjbear@users.noreply.github.com>
@mjbear
Copy link
Contributor

mjbear commented Jan 7, 2025

Update: Disregard. Jacob created #1965

@jnicholson56
We need to add an entry for this new template to the index file. Please include that in this PR.

The below is the test system telling us it doesn't know about this template (because the index doesn't say the template exists).

E           ntc_templates.parse.ParsingException: Unable to parse command "show system processes summary"
on platform juniper_junos - No template found for attributes:
"{'Command': 'show system processes summary', 'Platform': 'juniper_junos'}"

ntc_templates/parse.py:82: ParsingException

@jmcgill298
Copy link
Contributor

thanks @jnicholson56 I forked this and updated the index file in order to get this across the finish line and clean up some of the backlog

@jmcgill298 jmcgill298 closed this Jan 7, 2025
@jnicholson56
Copy link
Contributor Author

jnicholson56 commented Jan 7, 2025 via email

@mjbear
Copy link
Contributor

mjbear commented Jan 7, 2025

That is unfortunate. The other PR had an active question that I needed an answer too so I could finish the both properly. I will open my question as an issue and resubmit them again later once I figure out the proper direction for them.

@jnicholson56
I've learned a bit about GitHub over time and the more I know the smoother it goes.
When you're opening PRs, they can be set as a Draft (I suggest doing that out of the gate if the PR isn't quite ready) and when it's ready to go then mark it as Ready for review.

We can link the new PR to the old one, no issue there. 🙂

GitHub docs example "You can mark a draft pull request as ready for review or convert a pull request to a draft."

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.

3 participants