Skip to content

T7561: simplify op-mode-definitions XML cache generation #4562

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

Open
wants to merge 5 commits into
base: current
Choose a base branch
from

Conversation

jestabro
Copy link
Contributor

@jestabro jestabro commented Jun 17, 2025

Change summary

Simplify original implementation of the op-mode XML cache generation and provide interface methods for use in, e.g., documentation generation.

While the evolution of the op-mode schema is taking place, it may be of benefit to provide a more usable structure/interface to the op-mode XML; this improves on the original implementation as follows:

(1) Disambiguate paths by enriching with node type, resulting in the simple nested dict structure familiar from the interface-definitions case.
(2) Provide generator and lookup methods to access the resulting data structure.

The above allows, for example, simple tools for documentation generation, either in bulk or on the fly.

Note one fundamental difference with the previous structure of nested lists of dicts: there is no naive dump to JSON --- nor should there be, is the position here: using the JSON representation for generation of documentation or other uses is neither needed nor desired. In it's place, the appropriate interface method into the structure should be used.

Various JSON representations may be produced from the node_data structures by simple transformations, by inserting virtual tag nodes or placeholders (as included in the path field of the node_data class).

Simple examples:

>>> from vyos.xml_ref import walk_op_node_data
>>> for i, n in enumerate(walk_op_node_data()):
...     if i > 10:
...         break
...     print(n)
... 
NodeData(name='add', node_type='node', help_text=None, comp_help={}, command=None, path=['add'], children=[('container', 'node')])
NodeData(name='container', node_type='node', help_text='Add container image', comp_help={}, command=None, path=['add', 'container'], children=[('image', 'tagNode')])
NodeData(name='image', node_type='tagNode', help_text='Pull a new image for container', comp_help={}, command='/usr/libexec/vyos//op_mode/container.py add_image --name image-tag_value', path=['add', 'container', 'image', 'image-tag_value'], children=[])
NodeData(name='raid', node_type='tagNode', help_text='Add a RAID set element', comp_help={'script': ['/usr/libexec/vyos//completion/list_raidset.sh']}, command=None, path=['add', 'raid', 'raid-tag_value'], children=[('by-id', 'node'), ('member', 'tagNode')])
NodeData(name='by-id', node_type='node', help_text='Add a member by disk id to a RAID set', comp_help={}, command=None, path=['add', 'raid', 'raid-tag_value', 'by-id'], children=[('member', 'tagNode')])
NodeData(name='member', node_type='tagNode', help_text='Add a member to a RAID set', comp_help={}, command='/usr/libexec/vyos//op_mode/raid.py add --raid-set-name raid-tag_value --by-id --member member-tag_value', path=['add', 'raid', 'raid-tag_value', 'by-id', 'member', 'member-tag_value'], children=[])
NodeData(name='member', node_type='tagNode', help_text='Add a member to a RAID set', comp_help={}, command='/usr/libexec/vyos//op_mode/raid.py add --raid-set-name raid-tag_value --member member-tag_value', path=['add', 'raid', 'raid-tag_value', 'member', 'member-tag_value'], children=[])
NodeData(name='system', node_type='node', help_text='Add item to a system facility', comp_help={}, command=None, path=['add', 'system'], children=[('image', 'tagNode')])
NodeData(name='image', node_type='tagNode', help_text='Add a new image to the system', comp_help={'list': ['/path/to/vyos-image.iso "http://example.com/vyos-image.iso" latest']}, command='/usr/libexec/vyos//op_mode/image_installer.py --action add --image-path image-tag_value', path=['add', 'system', 'image', 'image-tag_value'], children=[('vrf', 'tagNode'), ('username', 'tagNode')])
NodeData(name='username', node_type='tagNode', help_text='Username for authentication', comp_help={}, command=None, path=['add', 'system', 'image', 'image-tag_value', 'username', 'username-tag_value'], children=[('password', 'tagNode')])
NodeData(name='password', node_type='tagNode', help_text='Password to use with authentication', comp_help={}, command='/usr/libexec/vyos//op_mode/image_installer.py --action add --image-path image-tag_value --username username-tag_value --password password-tag_value', path=['add', 'system', 'image', 'image-tag_value', 'username', 'username-tag_value', 'password', 'password-tag_value'], children=[])

Lookup:

>>> from vyos.xml_ref import lookup_op_node_data
>>> path = ['show', 'interfaces', 'ethernet']
>>> out = lookup_op_node_data(path)
>>> pprint(out)
[NodeData(name='ethernet',
          node_type='node',
          help_text='Show Ethernet interface information',
          comp_help={},
          command='/usr/libexec/vyos//op_mode/interfaces.py show_summary '
                  '--intf-type=ethernet',
          path=['show', 'interfaces', 'ethernet'],
          children=[('detail', 'leafNode')]),
 NodeData(name='ethernet',
          node_type='tagNode',
          help_text='Show specified Ethernet interface information',
          comp_help={'path': ['interfaces ethernet']},
          command='/usr/libexec/vyos//op_mode/interfaces.py show '
                  '--intf-name=ethernet-tag_value --intf-type=ethernet',
          path=['show', 'interfaces', 'ethernet', 'ethernet-tag_value'],
          children=[('brief', 'leafNode'),
                    ('identify', 'leafNode'),
                    ('physical', 'node'),
                    ('statistics', 'leafNode'),
                    ('transceiver', 'leafNode'),
                    ('vif', 'tagNode'),
                    ('event-log', 'node')])]

Above one sees an example of path ambiguity.
One can provide hints by either including indicative tag-node values:

>>> path = ['show', 'interfaces', 'ethernet', 'some-tag-value']
>>> out = lookup_op_node_data(path, tag_values=True)
>>> pprint(out)
[NodeData(name='ethernet',
          node_type='tagNode',
          help_text='Show specified Ethernet interface information',
          comp_help={'path': ['interfaces ethernet']},
          command='/usr/libexec/vyos//op_mode/interfaces.py show '
                  '--intf-name=ethernet-tag_value --intf-type=ethernet',
          path=['show', 'interfaces', 'ethernet', 'ethernet-tag_value'],
          children=[('brief', 'leafNode'),
                    ('identify', 'leafNode'),
                    ('physical', 'node'),
                    ('statistics', 'leafNode'),
                    ('transceiver', 'leafNode'),
                    ('vif', 'tagNode'),
                    ('event-log', 'node')])]

or by specifying the node type of the final node:

>>> path = ['show', 'interfaces', 'ethernet']
>>> out = lookup_op_node_data(path, last_node_type='node')
>>> pprint(out)
[NodeData(name='ethernet',
          node_type='node',
          help_text='Show Ethernet interface information',
          comp_help={},
          command='/usr/libexec/vyos//op_mode/interfaces.py show_summary '
                  '--intf-type=ethernet',
          path=['show', 'interfaces', 'ethernet'],
          children=[('detail', 'leafNode')])]

Finally, as with the original implementation, the generate_op_cache.py script is able to check for inconsistencies in the XML itself during cache generation. cf. the inline comments regarding generate_op_cache.py --check-xml-consistency.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

How to test / Smoketest result

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@jestabro jestabro self-assigned this Jun 17, 2025
Copy link

github-actions bot commented Jun 17, 2025

👍
No issues in PR Title / Commit Title

@jestabro jestabro force-pushed the op-mode-data branch 4 times, most recently from 7ce53c5 to 03280f7 Compare June 18, 2025 04:09
Copy link

✅ No issues found in unused-imports check.. Please refer the workflow run

@jestabro jestabro force-pushed the op-mode-data branch 2 times, most recently from 1f3951d to 533115a Compare June 19, 2025 01:06
@jestabro jestabro marked this pull request as ready for review June 19, 2025 01:43
@jestabro jestabro marked this pull request as draft June 19, 2025 15:09
@jestabro
Copy link
Contributor Author

Move to draft to update for recent op-mode changes.

jestabro added 3 commits June 20, 2025 09:58
The original implementation of the op-mode XML cache generation resulted
in a structure that was difficult to use, for example, in documentation
generation. The source of complication is that, unlike the XML of
interface-definitions, path names are not unique: the same path may
occur as both a regular node and as a tag node. Here we simplify the
underlying structure by enriching path names with type information, thus
disambiguating paths. An interface to the cache is provided by explicit
generator and lookup functions.
@jestabro
Copy link
Contributor Author

jestabro commented Jun 20, 2025

Added the following:
(1) refined the --check-xml-consistency test to avoid false positives after re-adding children and file fields
(2) added a --check-path-ambiguity test to detect duplicate paths
(3) if there are no duplicate paths in the collection of XML files, collapse the nested dict to one that is indexed by str, not tuple, and dump to json. A subset of XML files may be chosen with, for example, --select 'poweroff | multicast-group | ...'

@jestabro jestabro force-pushed the op-mode-data branch 2 times, most recently from 8d0f03f to 8abc42c Compare June 21, 2025 00:44
Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

I think it's ready to take out of the draft status and merge now. When all command definitions are reworked to make every path unique, we can make the ambiguity check mandatory at a later point.

@jestabro jestabro marked this pull request as ready for review June 21, 2025 12:06
Copy link

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@sever-sever sever-sever requested a review from Copilot June 21, 2025 17:12
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies the op-mode XML cache generation by refactoring the data structure to use dataclasses and adding new utility methods for traversing, looking up, and sorting op-mode definitions, which in turn facilitates documentation generation.

  • Refactors NodeData from a TypedDict to a dataclass with additional fields.
  • Introduces new functions for walking, sorting, and collapsing XML definition data in op_definition.py and generate_op_cache.py.
  • Updates init.py to expose new API functions for accessing op-mode data.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
python/vyos/xml_ref/op_definition.py Refactored NodeData implementation and added lookup/walk utilities; duplicate function issue noted.
python/vyos/xml_ref/generate_op_cache.py Enhanced XML cache generation with sorting, consistency checks, and updated node insertion logic.
python/vyos/xml_ref/init.py Updated module exports and added new functions for walking and looking up op-mode data.
Comments suppressed due to low confidence (1)

python/vyos/xml_ref/op_definition.py:56

  • The function 'keys_of_name' is defined twice with different parameters; the second definition does not return the filtered result. Consider removing or renaming the duplicate definition and ensure that it returns the expected list of keys.
    filter(lambda t: t[0] == s, l)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants