sonic-yang-mgmt: helpers to prevent dependents from calling into libyang #24414
Conversation
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bhouse-nexthop
changed the title
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
e81acff to
791ecf8
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
791ecf8 to
db1faee
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
f69d0c7 to
307e497
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
307e497 to
72383d1
Compare
|
/azp run Azure.sonic-buildimage |
|
another day, another spurious build failure unrelated to the patches. Rebased again ... |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@lguohan @vaibhavhd this PR passed all checks finally after a couple of force rebuilds (no changes on my end). Can you review and hopefully approve / merge. This is part of the libyang3 migration patch set. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds helper methods to sonic-yang-mgmt to prevent sonic-utilities from directly depending on libyang. The changes include a new method to load YANG modules from strings and enhancements to dependency-finding functions to support broader search scopes.
Key changes:
- Added
load_module_str_name()method to load YANG modules from string content and return module names - Enhanced
find_data_dependencies()andfind_schema_dependencies()to support recursive dependency lookups across all modules when given None/"/" xpath - Fixed existing typo in
_find_schema_node()method (schema_xapth → schema_xpath)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| src/sonic-yang-mgmt/sonic_yang.py | Added load_module_str_name() helper method; enhanced dependency-finding functions to support global lookups; fixed typo in schema node lookup |
| src/sonic-yang-mgmt/tests/libyang-python-tests/test_sonic_yang.py | Added test case for load_module_str_name() method |
| src/sonic-yang-mgmt/tests/libyang-python-tests/test_SonicYang.json | Added test data for dependency lookup tests including global scope test case |
Comments suppressed due to low confidence (1)
src/sonic-yang-mgmt/sonic_yang.py:682
- Variable nodes is not used.
nodes = []
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/sonic-yang-mgmt/tests/libyang-python-tests/test_sonic_yang.py
Outdated
Show resolved
Hide resolved
|
@bhouse-nexthop , i agree with the motivation of the pr. seems some copilot comments, we can get this merged once copilot comments are addressed. |
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
cf27d3f to
ddbfb13
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
ddbfb13 to
2decc6e
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
2decc6e to
5b5cd89
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Load a module based on the provided string and return the loaded module name. This is needed by sonic-utilities to prevent a direct dependency on libyang. By adding this helper, sonic-utilities will use only what is provided by sonic-yang-mgmt and not be dependant on the version of libyang installed.
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
5b5cd89 to
8921112
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The only caller to find_data_dependencies() is in sonic-utilities and it is called in a recursive manner by calling into libyang functions directly. This should be properly abstracted within sonic-yang-mgmt to ensure it is not dependent on any particular libyang version. Test cases have been updated to reflect the behavior while still maintaining compatibility with the existing sonic-utilities until it is updated.
bhouse-nexthop
force-pushed
the
parse-module-helper
branch
from
8921112 to
1a971a6
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@lguohan finally got past all the spurious build/test failures by issuing fake rebases. Anyhow, can you please review now and hopefully merge? |
sonic-utilities should never call directly into libyang. All functionality it needs should be provided in sonic-yang-mgmt. This resolves issues when libyang changes API/ABI such as happened between libyang1 and libyang2, and again between libyang2 and libyang3. Depends on sonic-net/sonic-buildimage#24414
sonic-utilities should never call directly into libyang. All functionality it needs should be provided in sonic-yang-mgmt. This resolves issues when libyang changes API/ABI such as happened between libyang1 and libyang2, and again between libyang2 and libyang3. Depends on sonic-net/sonic-buildimage#24414
3 participants
Why I did it
This is needed by sonic-utilities to prevent a direct dependency on libyang. By adding these helpers, sonic-utilities will use only what is provided by sonic-yang-mgmt and not be dependent on the version of libyang installed.
How I did it
Added helpers to:
How to verify it
See included test cases.
Which release branch to backport (provide reason below if selected)
N/A
Tested branch (Please provide the tested image version)
master as of 2025-11-03
Description for the changelog
sonic-yang-mgmt: helpers to prevent dependents from calling into libyang
Link to config_db schema for YANG module changes
N/A
A picture of a cute animal (not mandatory but encouraged)
Fixes #22385
Needed by sonic-net/sonic-utilities#4118