Skip to content

Conversation

@jlmcgraw
Copy link
Contributor

@jlmcgraw jlmcgraw commented Oct 24, 2025

Description

Handle NTP servers configured to use a key

eg:
ntp server vrf Mgmt 10.2.2.2 key 2
ntp server vrf Mgmt 10.3.3.3 key 3 prefer

Motivation and Context

NTP servers that have a key configured are not properly parsed by the current regex

Impact (If any)

NTP servers that have a key configured are properly parsed

Screenshots:

Checklist:

  • I have updated the changelog.
  • I have updated the documentation (If applicable).
  • I have added tests to cover my changes (If applicable).
  • All new and existing tests passed.
  • All new code passed compilation.

@jlmcgraw jlmcgraw requested a review from a team as a code owner October 24, 2025 17:47
Copy link
Collaborator

@ThomasJRyan ThomasJRyan left a comment

Choose a reason for hiding this comment

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

This is a fine change, just needs some maintenance.
Along with the requested changes, it will need a changelog file and unittests.
You can look to close PRs to get a sense of what they should be, but for a quick example

Changelogs are .rst files placed into genieparser/changelog/undistributed with the format

---------------------------------
            Fix
---------------------------------
* <OS>
    * <Parser Class Name>
        * <A brief description of what was changed>

And unittests consist of two files inside of the tests/<name_of_parser_class> folder. A <name>_output.txt file with the raw output of the command, and a <name>_expected.py file that contains a single variable called expected_output which should be equal to the resulting parsed dictionary.

Include all that and this is good to go! (I know it's a bit of a process, but I promise you it gets easier with time)

@jlmcgraw jlmcgraw requested a review from ThomasJRyan October 28, 2025 15:26
Copy link
Collaborator

@ThomasJRyan ThomasJRyan left a comment

Choose a reason for hiding this comment

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

One small change to the changelog and all is good to go

@omehrabi omehrabi self-requested a review October 28, 2025 16:22
@omehrabi
Copy link
Contributor

the pipeline seems to fail

@jlmcgraw
Copy link
Contributor Author

jlmcgraw commented Oct 28, 2025

the pipeline seems to fail

My output was the result of using this parser as part of a 'learn("ntp")', it seems that output differs from a straight parsing of just "show ntp config". I will update

@omehrabi omehrabi merged commit 933ac99 into CiscoTestAutomation:main Oct 28, 2025
5 checks passed
@jlmcgraw jlmcgraw deleted the 2025-10-24_ios_xe_show_ntp branch October 29, 2025 15:45
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