-
Notifications
You must be signed in to change notification settings - Fork 335
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
cli: T6740: add a converter from set commands to config #4124
cli: T6740: add a converter from set commands to config #4124
Conversation
👍 |
✅ No issues found in unused-imports check.. Please refer the workflow run |
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.
The following concerns may be purely cosmetic, but it is a timely reminder that we are still relying on the legacy loader and parser to provide:
(1) setting tag data
(2) ordering of nodes
We have explicit mechanisms for both, and if one wants the result of the converter to provide a stand alone config rendered with respect to those constraints, the following patch illustrates the idea:
088fa9a
The approach to (1) there is reasonable and provides a utility that may be otherwise useful to set all tag data long a path; the approach to (2) is a hack by simply reading and re-rendering the result --- we currently order nodes when parsing, not rendering, as we haven't had need of the latter (other than an optional ordering of values for unittests).
It is the author's discretion as to whether we should bother with cosmetic parity for the output.
Signed-off-by: Daniil Baturin <[email protected]>
CI integration ❌ failed! Details
|
Change Summary
Add new utility
vyos-commands-to-config
that convertsset
commands to configs suitable for loading.Whether we want a CLI completion for it is an open question.
Types of changes
Related Task(s)
Related PR(s)
Component(s) name
Proposed changes
How to test
Smoketest result
Checklist: