sonic-py-swsssdk: Changes to support SONiC Gearbox Manager #79
Conversation
* add databases that will be used to represent second switch (gearbox phy) in REDIS HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md Signed-off-by: [email protected]
|
@qiluo-msft requesting a review, thanks. If there is a better way to request that, please let me know. |
| @@ -51,6 +51,26 @@ | |||
| "id" : 7, | |||
| "separator": "|", | |||
| "instance" : "redis" | |||
| }, | |||
| "ASIC_DB2" : { | |||
There was a problem hiding this comment.
ASIC_DB2 [](start = 9, length = 8)
How many ASIC DBs in long future? We have only 16 databases in Redis.
There was a problem hiding this comment.
We have no plans immediately for increasing the count any time soon. However, there may be an issue with this - sairedis now supports multiple switches, and what we may need in the future to deal with the redis limitation is multiple redis databases, one per switch/phy. There is discussion along these lines occurring, but to answer the question, this implementation for VS platforms requires only these new tables, no more, and no more are planned for.
There was a problem hiding this comment.
The use case "for VS platforms" is quite limited and you should change /var/run/redis/sonic-db/database_config.json in that environment.
The design of the config file is to maintain backward-compatible with previous single redis and hardcoded databse mapping if there is no /var/run/redis/sonic-db/database_config.json. So we have no plan to extend it to support new feature.
In reply to: 434791844 [](ancestors = 434791844)
There was a problem hiding this comment.
We are planning to submit changes to the database-docker (sonic-buildimage) to create these tables (via database_config.json.j2 in vs and other build targets).
But for now, the unit tests in sonic-utilities are failing based on what will be an incompatibility with REDIS database as it will be once we complete pushing all gearbox changes through SONiC. And those unit tests failing at gate are dependent on only the db config offered by SonicV2Connector. Not sure I see a way out of this, unless there are APIs in SonicV2Connector that maybe we can extend the list of databases by appending them in some unit test setup function (I looked at the SonicV2Connector methods, see getters and setters but nothing that can be used to extend the list of databases). Maybe the design doesn't allow for that, and not even sure it is the right answer.
There was a problem hiding this comment.
Okay, you are talking the use case "the unit tests in sonic-utilities". You need to modify /var/run/redis/sonic-db/database_config.json in that environment.
Otherwise if you want more flexibility, such as use different config for different unit test cases. You are expected to mock the classes. ref: https://github.com/Azure/sonic-snmpagent/blob/master/tests/mock_tables/dbconnector.py
In reply to: 434874842 [](ancestors = 434874842,434791844)
There was a problem hiding this comment.
We are testing an alternate implementation which does not impact this project, if it works, will drop this pull request. Thanks for the good review comments, very helpful.
|
Abandoned. We made changes to sonic-utilities that result in this pull no longer being required. |
HLD is located at https://github.com/Azure/SONiC/blob/b817a12fd89520d3fd26bbc5897487928e7f6de7/doc/gearbox/gearbox_mgr_design.md
Signed-off-by: [email protected]