Support read default value from yang model and read profile from PROFILE_DB.#625
Support read default value from yang model and read profile from PROFILE_DB.#625liuh-80 wants to merge 37 commits intosonic-net:masterfrom
Conversation
| class YangDefaultValueTableDecorator : public Table { | ||
| public: | ||
| YangDefaultValueTableDecorator(Table* table); | ||
| ~YangDefaultValueTableDecorator() override; |
There was a problem hiding this comment.
should this be marked as virtual?
There was a problem hiding this comment.
Thanks, fixed, and the 'override' declarator actually will makesure this is a virtual method.
|
My move dump() to TableEntryEnumerable interface, we can revert all Table class change and remove YangDefaultValueTableDecorator |
|
|
||
| namespace swss { | ||
|
|
||
| class YangDefaultValuePoppableDecorator : public TableEntryPoppable { |
There was a problem hiding this comment.
From design point of view, there are several requirements:
- SubscribeStateTable
- Yang Default + SubscribeStateTable (from bottom layer to up layer)
- Table + SubscribeStateTable (for example, image default profile with user override)
- Yang Default + Table + SubscribeStateTable
The outside interface is SubscribeStateTable
There was a problem hiding this comment.
My concern is: if we only decorate the TableEntryPoppable interface, then we can only decorate 2 method:
/* Pop an action (set or del) on the table */
void pop(KeyOpFieldsValuesTuple &kco, const std::string &prefix = EMPTY_PREFIX) override;
/* Get multiple pop elements */
void pops(std::deque<KeyOpFieldsValuesTuple> &vkco, const std::string &prefix = EMPTY_PREFIX) override;
But if we decorate SubscriberStateTable , then there are too much methods we need to decorate and the are not related with default value, for example these methods:
uint64_t readData() override;
bool hasData() override;
bool hasCachedData() override;
There was a problem hiding this comment.
Improved by create OverlayConfigTable and OverlaySubscriberStateTable.
From design point of view, there are several requirements:
The outside interface is Table In reply to: 1152829037 Refers to: common/table.h:165 in f452126. [](commit_id = f452126, deletion_comment = False) |
common/consumertablebase.h
Outdated
|
|
||
| const DBConnector* getDbConnector() const; | ||
|
|
||
| std::string getNamespace() const; |
There was a problem hiding this comment.
namespace is not property of consumertablebase, its property of dbconnector, and since we have method getDbConnectort() should you use that and then call getnamespace?
There was a problem hiding this comment.
Thanks, I will remove this method and improve code to use getDbConnector.
common/configdb.h
Outdated
| ret.setdefault(table_name, {})[self.deserialize_key(row)] = entry | ||
| return ret | ||
|
|
||
| class LayeredConfigDBConnectorDecorator(ConfigDBConnector): |
There was a problem hiding this comment.
WIll improve this decorator with re-write ConfigDBConnector API with reflection.
There was a problem hiding this comment.
Fixed, improve this decorator according to isabel's poc code.
common/configdb.h
Outdated
| config_db_connector.ori_get_config = config_db_connector.get_config | ||
| def _append_static_config(table, key, data): | ||
| serialized_key = config_db_connector.serialize_key(key) | ||
| ns = config_db_connector.getNamespace() |
There was a problem hiding this comment.
config_db_connector.getDbConnector().newConnector() will create a new connector to config DB, but in our case we need connect to a different database: static_config_db.
I will check if we can save static config to config DB with a different table name, for example static_*, then we can reuse current connector.
There was a problem hiding this comment.
Fixed, now use getDbConnector() for better performance.
common/schema.h
Outdated
| #define CHASSIS_APP_DB 12 | ||
| #define CHASSIS_STATE_DB 13 | ||
| #define APPL_STATE_DB 14 | ||
| #define STATIC_CONFIG_DB 15 |
common/defaultvalueprovider.cpp
Outdated
| } | ||
| } | ||
|
|
||
| std::map<std::string, std::string> DefaultValueProvider::GetDefaultValues(const std::string &table, const std::string &row) |
common/defaultvalueprovider.cpp
Outdated
| { | ||
| case 0: | ||
| { | ||
| tableInfoPtr = new TableInfoDict(fieldInfoMapping); |
liuh-80
force-pushed
the
dev/liuh/default_value_decorator
branch
2 times, most recently
from
0db1a97 to
1a00040
Compare
|
|
liuh-80
force-pushed
the
dev/liuh/default_value_decorator
branch
from
42da895 to
bb1108b
Compare
liuh-80
changed the title
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-swss-common |
|
Azure Pipelines failed to run 1 pipeline(s). |
liuh-80
changed the title
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Will validate all E2E test with this PR: sonic-net/sonic-buildimage#10575 |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Will split this PR to 3-4 small PRs for code review. |
|
Close this PR, will create splited PRs later. |
3 participants
Why I did it
Add default value and profile support to swss-swss-common.
How I did it
Add DefaultValueProvider and ProfileProvider.
How to verify it
Add new UT.
Pass all existing UT and E2E test.
Which release branch to backport (provide reason below if selected)
Description for the changelog
Add default value and profile support to swss-swss-common.
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)