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

[yang] extend ConfigMgmt constructor to pass YANG options #2118

Merged
merged 1 commit into from
May 25, 2022

Conversation

ayurkiv-nvda
Copy link
Contributor

@ayurkiv-nvda ayurkiv-nvda commented Mar 27, 2022

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

  1. Create some invalid link on switch :
    ln -s /usr/abc xxx
  2. run spm list
    --> There should not be these messages:
libyang[1]: Unable to get information about "xxx" file in "/tmp" when searching for (sub)modules (No such file or directory)
libyang[1]: Unable to get information about "xxx" file in "/tmp" when searching for (sub)modules (No such file or directory)
libyang[1]: Unable to get information about "xxx" file in "/tmp" when searching for (sub)modules (No such file or directory)
libyang[1]: Unable to get information about "xxx" file in "/tmp" when searching for (sub)modules (No such file or directory)

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)

@@ -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)

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.

Copy link
Contributor Author

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"

Copy link
Contributor

@ghooo ghooo May 16, 2022

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

Copy link
Contributor Author

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

Copy link
Contributor

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.

@ayurkiv-nvda
Copy link
Contributor Author

/azpw run Azure.sonic-utilities

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dprital
Copy link
Collaborator

dprital commented May 3, 2022

@praveen-li - Now that the comment was addressed, can you please approve this PR ?

@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zhangyanzhao
Copy link
Collaborator

@praveen-li would you please help to review and approve if you are ok with the latest update? Thanks.

@qiluo-msft
Copy link
Contributor

@ghooo Could you help review this PR while @praveen-li is not available?

@ayurkiv-nvda
Copy link
Contributor Author

@ghooo Could you please help to review changes?

@zhangyanzhao zhangyanzhao requested a review from ghooo May 12, 2022 17:16
@dprital
Copy link
Collaborator

dprital commented May 18, 2022

@ghooo - As the comments were addressed, can you please approve this PR ?

@dprital
Copy link
Collaborator

dprital commented May 24, 2022

@ghooo - Can you please approve this PR ?

@liat-grozovik liat-grozovik merged commit 4516179 into sonic-net:master May 25, 2022
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 25, 2022
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>
judyjoseph pushed a commit that referenced this pull request May 25, 2022
- 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:
dprital added a commit to dprital/sonic-utilities that referenced this pull request Jun 1, 2022
qiluo-msft pushed a commit that referenced this pull request Jun 9, 2022
#### 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()
judyjoseph pushed a commit that referenced this pull request Jun 13, 2022
#### 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()
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
#### 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()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants