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.

Sign up for GitHub

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

Jump to bottom

Fix libyang backlinks issue: #9545

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix libyang backlinks issue: #9545

wants to merge 2 commits into from

Conversation

li-pingmao
Copy link
Contributor

Fix libyang backlinks and sonic-yang-mgmt find_data_dependency issues:
1) changes of find_data_dependency():
- remove call to libyang backlinks()
- find data dependencies/backlinks based on schema and data tree python APIs
2) Add test of yang model with union of leafrefs

Why I did it

1.  libyang dropped python binding backlinks() API with newer version
2. old 1.0.173 python backlinks() API does not find the data dependencies/backlinks in case of union of leafrefs

How I did it

  • remove call to libyang backlinks()
  • changes of find_data_dependency():
    find data dependencies/backlinks based on schema and data tree python APIs

How to verify it

Add test cases of yang model with union of leafrefs

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@li-pingmao li-pingmao requested a review from lguohan as a code owner December 15, 2021 20:00
@lgtm-com
Copy link

lgtm-com bot commented Dec 15, 2021

This pull request introduces 1 alert when merging c1c7fb885022cb3f91e9f76c26940c3d8256a381 into 727a946 - view on LGTM.com

new alerts:

  • 1 for Testing equality to None

@qiluo-msft qiluo-msft added the Request for 202111 Branch For PRs being requested for 202111 branch label Dec 16, 2021
@zhangyanzhao zhangyanzhao added the YANG YANG model related changes label Jan 27, 2022
@zhangyanzhao
Copy link
Collaborator

Ping will follow-up with Mohamed and Praveen offline.

@ghooo ghooo mentioned this pull request Jan 27, 2022
Merged
ghooo added a commit to sonic-net/sonic-utilities that referenced this pull request Feb 18, 2022
@ghooo
[GCU] Adding unit-test where path and ref paths are under the same YA…
15670bf 
…NG container (#2047)

#### What I did
Did a debug session with Ping Mao and found this case is failing in her PR: sonic-net/sonic-buildimage#9545 I think it is an interesting case and I added it explicitly to GCU.

Adding unit-test where path and ref paths are under the same YANG container

[sonic-loopback-interface.yang](https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-loopback-interface.yang) has:
```yang
    container sonic-loopback-interface {
        container LOOPBACK_INTERFACE {
            list LOOPBACK_INTERFACE_LIST {
                ...
                leaf name{
                    type string;
                }
                ...
            list LOOPBACK_INTERFACE_IPPREFIX_LIST {
                leaf name{
                    ...
                    type leafref {
                        path "../../LOOPBACK_INTERFACE_LIST/name";
                    }
                }
               ...
```

#### How I did it
Unit-test added

#### How to verify it
unit-test

#### 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)
@liat-grozovik
Copy link
Collaborator

@li-pingmao can you please resolve conflict so we can move forward with the review?
this one is blocking the NVGRE PR to be un reverted due to this issue.

@li-pingmao
Copy link
Contributor Author

Sure, looking into it.

@lgtm-com
Copy link

lgtm-com bot commented Feb 23, 2022

This pull request introduces 1 alert when merging 3f9c59c1a10e1ccb507add09a2c54c0b80761703 into 55e7a14 - view on LGTM.com

new alerts:

  • 1 for Testing equality to None

@li-pingmao
Fix libyang backlinks issue:
94ac50b 
        - remove call of libyang backlinks() call
        - find data dependencies based on python schema and data tree APIs
        - Add test of yang model with union of leafrefs
@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@qiluo-msft
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft
Copy link
Collaborator

Seems sonic-utilities unit test is stuck.

2022-03-17T00:41:35.7419046Z tests/config_dpb_test.py::TestConfigDPB::test_get_breakout_options PASSED [  8%]
2022-03-17T00:41:35.7420084Z tests/config_dpb_test.py::TestConfigDPB::test_config_breakout_extra_table_warning PASSED [  8%]
2022-03-17T00:41:35.7421887Z [  FAIL LOG END  ] [ target/python-wheels/bullseye/sonic_utilities-1.2-py3-none-any.whl ]
2022-03-17T00:41:35.7486017Z make: *** [slave.mk:676: target/python-wheels/bullseye/sonic_utilities-1.2-py3-none-any.whl] Error 1
2022-03-17T00:41:35.7486717Z make: *** Waiting for unfinished jobs....

@zhangyanzhao
Copy link
Collaborator

This PR is failing sonic-utilities test and
test_find_ref_paths__path_and_ref_paths_are_under_same_yang_container__returns_ref_paths, need @li-pingmao help to fix.

@zhangyanzhao
Copy link
Collaborator

  1. option Update README.md #1: change ACL yang model as workaround
  2. option Update README.md #2: upgrade libyang to latest version but it has no binding function (delivered by separate package)
    @li-pingmao will raise PR for option Update README.md #1.

@zhangyanzhao
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zhangyanzhao
Copy link
Collaborator

Ping debugged with Mohamed, still need more investigation on new test cases.

@li-pingmao li-pingmao requested a review from a team as a code owner June 10, 2022 02:01
@ghooo ghooo mentioned this pull request Jul 25, 2022
Open
@zhangyanzhao
Copy link
Collaborator

@ganglyu

@zhangyanzhao
Copy link
Collaborator

@li-pingmao can you please join next YANG subgroup meeting and provide an update? Thanks.

@zhangyanzhao
Copy link
Collaborator

libyang owner proposed a new option and @ganglyu is working on it.

malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
@malletvapid23
[GCU] Adding unit-test where path and ref paths are under the same YA…
3c91342 
…NG container (#2047)

#### What I did
Did a debug session with Ping Mao and found this case is failing in her PR: sonic-net/sonic-buildimage#9545 I think it is an interesting case and I added it explicitly to GCU.

Adding unit-test where path and ref paths are under the same YANG container

[sonic-loopback-interface.yang](https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-loopback-interface.yang) has:
```yang
    container sonic-loopback-interface {
        container LOOPBACK_INTERFACE {
            list LOOPBACK_INTERFACE_LIST {
                ...
                leaf name{
                    type string;
                }
                ...
            list LOOPBACK_INTERFACE_IPPREFIX_LIST {
                leaf name{
                    ...
                    type leafref {
                        path "../../LOOPBACK_INTERFACE_LIST/name";
                    }
                }
               ...
```

#### How I did it
Unit-test added

#### How to verify it
unit-test

#### 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft left review comments

@lguohan lguohan Awaiting requested review from lguohan

@ghooo ghooo Awaiting requested review from ghooo

@praveen-li praveen-li Awaiting requested review from praveen-li

@dgsudharsan dgsudharsan Awaiting requested review from dgsudharsan

@rathnasabapathyv rathnasabapathyv Awaiting requested review from rathnasabapathyv

At least 1 approving review is required to merge this pull request.

Assignees

@ganglyu ganglyu

Labels
Request for 202111 Branch For PRs being requested for 202111 branch YANG YANG model related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@li-pingmao @zhangyanzhao @liat-grozovik @qiluo-msft @ganglyu