-
Notifications
You must be signed in to change notification settings - Fork 661
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
[yang] extend ConfigMgmt constructor to pass YANG options #2118
Conversation
@@ -1002,7 +1003,7 @@ def get_manager() -> 'PackageManager': | |||
docker_api = DockerApi(docker.from_env(), ProgressManager()) | |||
registry_resolver = RegistryResolver() | |||
metadata_resolver = MetadataResolver(docker_api, registry_resolver) | |||
cfg_mgmt = config_mgmt.ConfigMgmt(source=INIT_CFG_JSON) |
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.
I did not understand this use.
So ConfigMgmt searches for yang models at a fixed location 'YANG_DIR = "/usr/local/yang-models"', then in what case it is an error ?.
Kindly elaborate, thx.
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.
Not exactly
ConfigMgmt searches for yang models in current working directory AND "/usr/local/yang-models"
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.
Since the expectation is to only read schema from /user/local/yang-models
let's disable reading schema from CWD (current working dir) in sonic-yang-mgmt directly, no need to create a flags object and pass it.
libyang ref: https://netopeer.liberouter.org/doc/libyang/master/html/group__contextoptions.html#ga08ef1a18adb7dac8e9f4a2c9522875ba
PR to add SonicYangOptions needs to be reverted: sonic-net/sonic-buildimage#10359
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.
@ghooo I have considered this option earlier, but after the discussion I chose the current approach. It looks more flexible to me and allows to pass different yang options to sonic-yang-mgmt
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.
I think this change will benefit all users of sonic_yang library. It would be better if we move the change to the library directly. Approving to get this unblocked.
/azpw run Azure.sonic-utilities |
/AzurePipelines run Azure.sonic-utilities |
Azure Pipelines successfully started running 1 pipeline(s). |
@praveen-li - Now that the comment was addressed, can you please approve this PR ? |
/azp run Azure.sonic-utilities |
Azure Pipelines successfully started running 1 pipeline(s). |
@praveen-li would you please help to review and approve if you are ok with the latest update? Thanks. |
2b65065
to
93a0254
Compare
@ghooo Could you help review this PR while @praveen-li is not available? |
@ghooo Could you please help to review changes? |
@ghooo - As the comments were addressed, can you please approve this PR ? |
@ghooo - Can you please approve this PR ? |
Update sonic-utilities submodule pointer to include the following: * [GCU] Handling type1 lists ([sonic-net#2171](sonic-net/sonic-utilities#2171)) * [yang] extend ConfigMgmt constructor to pass YANG options ([sonic-net#2118](sonic-net/sonic-utilities#2118)) * [dump] implement ACL modules ([sonic-net#2153](sonic-net/sonic-utilities#2153)) * show commands for SYSTEM READY ([sonic-net#1851](sonic-net/sonic-utilities#1851)) * [GCU] Handling non-compliant leaf-list with string values ([sonic-net#2174](sonic-net/sonic-utilities#2174)) * Add sonic-delayed.target to Application Extension .timer file generator ([sonic-net#2176](sonic-net/sonic-utilities#2176)) * [portconfig] Allow to configure interface mtu for physical ports ([#l](https://github.com/Azure/sonic-utilities/pull/l)) * Broadcast Unknown-multicast and Unknown-unicast Storm-control ([sonic-net#928](sonic-net/sonic-utilities#928)) * sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071)) * [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel ([sonic-net#2151](sonic-net/sonic-utilities#2151)) * Modify override testcase to cover PORT admin_status ([sonic-net#2165](sonic-net/sonic-utilities#2165)) * [GCU] Validate peer_group_range ip_range are correct ([sonic-net#2145](sonic-net/sonic-utilities#2145)) * [auto-ts] add memory check ([sonic-net#2116](sonic-net/sonic-utilities#2116)) * support new interface types CR8/SR8/KR8/LR8 which are brougnt by SAI V.1.10.2 ([sonic-net#2167](sonic-net/sonic-utilities#2167)) * [scripts/fast-reboot] Add option to include ssd-upgrader-part boot option with SONiC partition ([sonic-net#2150](sonic-net/sonic-utilities#2150)) * [config reload] Fix invalid rstrip. ([sonic-net#2157](sonic-net/sonic-utilities#2157)) * Accept 0 for queue and dscp ([sonic-net#2162](sonic-net/sonic-utilities#2162)) Signed-off-by: dprital <drorp@nvidia.com>
- What I did Pass YANG attribute that allow do not automatically search for schemas in current working directory, which is by default searched automatically (despite not recursively). - How I did it extend ConfigMgmt constructor - How to verify it Create some invalid link on switch : ln -s /usr/abc xxx run spm list --> There should not be these messages:
…nic-net#2118)" This reverts commit 2716ff2.
#### What I did Revert parameter from function that was mistakenly added in #2118 We don't need to pass 'sonic_yang_options' to static function get_module_name() #### How I did it remove 'sonic_yang_options' from SonicYang object creation in get_module_name()
#### What I did Revert parameter from function that was mistakenly added in #2118 We don't need to pass 'sonic_yang_options' to static function get_module_name() #### How I did it remove 'sonic_yang_options' from SonicYang object creation in get_module_name()
#### What I did Revert parameter from function that was mistakenly added in sonic-net/sonic-utilities#2118 We don't need to pass 'sonic_yang_options' to static function get_module_name() #### How I did it remove 'sonic_yang_options' from SonicYang object creation in get_module_name()
Should be merged after sonic-net/sonic-buildimage#10359 Used SonicYang with extended list of params to init object.
What I did
Pass YANG attribute that allow do not automatically search for schemas in current working directory, which is by default searched automatically (despite not recursively).
How I did it
extend ConfigMgmt constructor
How to verify it
ln -s /usr/abc xxx
--> There should not be these messages:
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)