-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[sonic-yang-mgmt]: sonic-yang-mgmt package for configuration validation. #3861
Conversation
32fc509
to
657555a
Compare
retest this please |
3267568
to
9c3dc09
Compare
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.
This is where the SONiC Yang files are to be checked in.
Curious, why there is none,
This PR has basic build steps, Code and YANG files comes with PR 3730, 3784 & 3891. |
Build started for merge commit. |
ff9e266
to
bf6f677
Compare
bf6f677
to
2037148
Compare
+ Renuka
Yes, I think, I need to do that. I made it to PY on review comments from Renuka. But pip doesn't let it installed on the sonic switch, if it doesn't have a proper version number. I will give a try to PY3 first, But due to the version of sonic utilities as PY2 I may have to fall back on the PY2.
Thanks,
Regards
Praveen
________________________________
From: vishnushetty <notifications@github.com>
Sent: Thursday, April 2, 2020 5:52 PM
To: Azure/sonic-buildimage <sonic-buildimage@noreply.github.com>
Cc: Praveen Chaudhary <pchaudhary@linkedin.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/sonic-buildimage] [sonic-yang-mgmt]: a.)Dev code, b.) build time test code and c.) wheel package Makefiles for sonic-yang-mgmt package. (#3861)
@vishnushetty commented on this pull request.
________________________________
In rules/sonic-yang-mgmt-py2.mk<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgh.neting.cc%2FAzure%2Fsonic-buildimage%2Fpull%2F3861%23discussion_r402677562&data=02%7C01%7Cpchaudhary%40linkedin.com%7Cd413e8ca48014edb347408d7d7693f5a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637214719344268742&sdata=zdFfJPslxZvt1fEf6crPolWRKOKECOOdtlbfc0nz%2FUk%3D&reserved=0>:
@@ -0,0 +1,8 @@
+# sonic-yang-mgmt python wheel
+
+SONIC_YANG_MGMT_PY = sonic_yang_mgmt-1.0-py-none-any.whl
do you want to change PY(py) -> PY2(py2) to be consistent with version
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgh.neting.cc%2FAzure%2Fsonic-buildimage%2Fpull%2F3861%23pullrequestreview-386872556&data=02%7C01%7Cpchaudhary%40linkedin.com%7Cd413e8ca48014edb347408d7d7693f5a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637214719344268742&sdata=FQsw1yzLzHRUNmrOc5NP241Z%2BAHd8TeZjXU2BF15Sik%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgh.neting.cc%2Fnotifications%2Funsubscribe-auth%2FAJHZHPPXPWTTYCWOYXQBHZTRKUXLTANCNFSM4JYRKTEQ&data=02%7C01%7Cpchaudhary%40linkedin.com%7Cd413e8ca48014edb347408d7d7693f5a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637214719344278738&sdata=%2B%2Bl%2FW32ML0DCZnQduyyDlkqhBXVNqAdcNScwNg%2FGZek%3D&reserved=0>.
|
5853723
to
2eaba1c
Compare
This pull request introduces 17 alerts when merging 2eaba1c8fd2fd70e93f5dccac20572cc12d43acf into 711445c - view on LGTM.com new alerts:
|
This pull request introduces 17 alerts when merging 1246ce4e7d24a070b8b7a9c7df7ff34753d9fb25 into de5a04a - view on LGTM.com new alerts:
|
1246ce4
to
a065d2a
Compare
This pull request introduces 1 alert when merging a065d2a6c5a05557a79f049ce5737909bd4a47cb into 7405f8c - view on LGTM.com new alerts:
|
a065d2a
to
1b8977d
Compare
This pull request introduces 1 alert when merging 1b8977d910f60ff561179b511e8a5a62cb3e71b6 into 7405f8c - view on LGTM.com new alerts:
|
67f0028
to
fb26bce
Compare
Changes: 1.) prefix test_ for sample yang models. 2.) name properly with data_node or schema_node. 3.) Update function calls after libyang PLY APIs updates. 4.) Fix test cases after sample yang files name changes.
…ig for the test. Changes: 1.) All the YANG models are not merged yet, so test config has tables with no YANG, allow this scenario for tests as of now.
0a7d729
to
57db247
Compare
…oving reference to sonic. Changes: 1.) To avoid confusion, Changing yang models to generic and removing reference to sonic 2.) Additing Relative path whenever needed.
9d45052
to
f82637a
Compare
retest vsimage please |
1 similar comment
retest vsimage please |
05/11/2020 Review: |
…user warning. Changes: 1.) Store Tables with YANG models seperately, so that a warning to user can be issued with enough information. 2.) print the tables without YANG models properly in cropConfigDB(). Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
…ly for user warning. Changes: 1.) Added config for extra tables in yangTest.json. 2.) Added new test case to find tables without yang models. 3.) Move common part in a helper function. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com
@renukamanavalan, @lguohan, @zhenggen-xu Latest commit to address below review comments:
Kindly let me know if any other concerns. Thanks a lot for the review. |
This pull request introduces 1 alert when merging d450aec234ab6f47b30b3b4e61b23e3448f1d9e8 into 0542afb - view on LGTM.com new alerts:
|
d450aec
to
0466e82
Compare
This pull request introduces 1 alert when merging 0466e8272b0a3ab853e4b8d8484499a8dc8e4f5c into 0542afb - view on LGTM.com new alerts:
|
Changes: 1.) Categories public and private functions. 2.) Minor spaces related changes. 3.) Defining class SonicYangException and using it in all public functions. 4.) Changing Class names to PascalCase. 5.) Keeping functions, which deals with Libyang, in lower_underscore_case, rest functions names are in camelCase.
0466e82
to
84beafb
Compare
retest vsimage please |
@renukamanavalan, @lguohan, @zhenggen-xu All tests passed, Kindly review. Thanks a lot. |
Overall concern -- based on our discussion on this PR. This PR adds additional validation for load & reload. With neither update, nor save guarded by the same validation, there is a good probable window to get a saved .JSON, which can't be loaded back. This can lead to inconsistent state and difficulties ahead. As we have SWSS sdk, as the center point for all updates, we have a single point for validation. Request: Please make this as a priority to address in future PRs, ASAP. |
@renukamanavalan @lguohan @zhenggen-xu Note: As of now, I had to remove config validation while config load\reload, because I notice multiple ASIC files change. So I would like to know first, whether we need validation for each file or not ?, Please let me know if you can help me here. |
This package includes python yang libraries which will be used with sonic utilities
package to validate the config. This python libraries are written on top of libyang
and also provides functionality to translate the config from SONiC ConfigDB to SONiC
YANG and vice-versa.
NOTE: This PR Depends on PR 3730
This PR has changes done under PR 3874 and 3891 as well. So those can be closed once this is merged.
- What I did
wheel package Makefiles
libyang Python APIs:
Extension of libyang Python APIs:
-- Cropping input config based on Yang Model.
-- Translate input config based on Yang Model.
-- rev Translate input config based on Yang Model.
-- Find xpath of port, portleaf and a yang list.
-- Find if node is key of a list while deletion if yes, then delete the parent.
- How I did it
wheel package Makefiles
libyang Python APIs:
Extension of libyang Python APIs:
From the json format of yang models, a map is created from config DB tables
to container in yang model. Input Config is cropped on based of this map.
Input Config is also translated based of this map.
Similarly from yang data tree, output is reverse translated to config DB.
This PR also includes:
-- Find xpath of port, portleaf and a yang list.
-- Find if node is key of a list while deletion if yes, then delete the parent.
Yang model can be represented in JSON as below:
This is used to create a map between Config DB Table and YANG Model Container.
Note: As per guideline https://github.com/Azure/SONiC/blob/master/doc/mgmt/SONiC_YANG_Model_Guidelines.md. Table maps to only container in YANG.
- How to verify it
wheel package Makefiles
Built this package separately.
libyang Python APIs:
Extension of libyang Python APIs:
Added PyTest:
- Description for the changelog
This package includes python yang libraries which will be used with sonic utilities
pacakge to validate the config. This python libraries are written on top of libyang
and also provides functionality to translate the config from SONiC ConfigDB to SONiC
YANG and vice-versa.
- A picture of a cute animal (not mandatory but encouraged)
I can put Microsoft logo :). Thx