Load Yang model default value to DefaultValueProvider#682
Load Yang model default value to DefaultValueProvider#682liuh-80 merged 14 commits intosonic-net:masterfrom
Conversation
| @@ -20,6 +20,10 @@ parameters: | |||
| - name: sonic_slave | |||
| type: string | |||
|
|
|||
There was a problem hiding this comment.
All azure pipeline change is from another PR: #681
That PR need merge first.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
You removed the whole content and changed file attribute. Is it intended? In reply to: 1242820539 In reply to: 1242820539 Refers to: common/dbconnector.cpp:7 in 00935a3. [](commit_id = 00935a3, deletion_comment = True) |
| ~DefaultValueProvider(); | ||
|
|
||
| // libyang context | ||
| struct ly_ctx *m_context = nullptr; |
There was a problem hiding this comment.
It's true, there will only one ly_ctx, it's created by our code and this is a singleton.
Libyang only need 1 context, there is no scenario that we need multiple context.
There was a problem hiding this comment.
Today the answer is yes. I am think relax the limitation may be future proof.
In future, ApplDB may have separate yang models, structure events may also has separate yang models. And these yang models are independent with each other. So I guess there will be use case that clients may use multiple ly_ctx..
The lazy initialization is still useful since the client of swsscommon may not use yang features at all.
There was a problem hiding this comment.
Fixed, now defaultvalue provider is not a singleton, so we can use multiple default value provider to load default value for different DB.
| #include "table.h" | ||
| #include "json.h" | ||
|
|
||
| #if defined(__arm__) || defined(__aarch64__) |
There was a problem hiding this comment.
New UT cover the warning code and passed, so I think it's safe.
Also I check this issue online and found there some discussion suggest ignore this warning is safe.
common/defaultvalueprovider.cpp
Outdated
| #if defined(__arm__) || defined(__aarch64__) | ||
| #define WARNINGS_NO_CAST_ALIGN \ | ||
| _Pragma ("GCC diagnostic push") \ | ||
| _Pragma ("GCC diagnostic ignored \"-Wcast-align\"") |
There was a problem hiding this comment.
[](http://example.com/codeflow?start=0&length=5)
Indent 4 spaces? 5 is weird. #Closed
I see non-critical comment change and still file attribute change. If it is intended change, suggest move to a standalone PR. This PR is complex and should be focused. In reply to: 1242820539 Refers to: common/dbconnector.cpp:7 in 00935a3. [](commit_id = 00935a3, deletion_comment = True) |
Fixed, reverted change in this file. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Currently, there still 10 repos need update their azure pipeline to install libyang. 10 PRs already created. |
2 participants
Why I did it
Support sonic-swss-common read Yang model defaut value.
How I did it
Load Yang model default value to DefaultValueProvider.
How to verify it
Add new UT to cover DefaultValueProvider code.
Pass all UT.
Which release branch to backport (provide reason below if selected)
Description for the changelog
Load Yang model default value to DefaultValueProvider.
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)