Update scripts in sonic-buildimage from py-swsssdk to swsscommon#11215
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
src/sonic-bgpcfgd/bgpmon/bgpmon.py
Outdated
There was a problem hiding this comment.
Remove pipe because in flush_pipe(self, data) has following code:
if value is None:
# delete case
self.pipe.delete(key)
else:
# Add or Modify case
self.db.hmset(self.pipe.STATE_DB. key, value)
This is because swsscommon not using pyredis, and self.db.get_redis_client(self.db.STATE_DB) will return StrictRedis. so the StrictRedis.pipeline() method does not exist in swss-common, in swss-common get_redis_client will return DBConnector.
Also another reason is the DBConnector only has following 1 method in swsscommon.py:
def hmset(self, multiHash):
return _swsscommon.DBConnector_hmset(self, multiHash)
Also DBConnector using redisContext from lib hredis to handle all redis operation, which doesn't not provide hmemset and delete operation.
This comment was marked as duplicate.
This comment was marked as duplicate.
Fixed. |
|
This pull request fixes 3 alerts when merging ddf1c0b9b5f32b906d7be3bfda081c5a30fb24e6 into 1568667 - view on LGTM.com fixed alerts:
|
4e8ea6a to
cde3633
Compare
|
This pull request fixes 3 alerts when merging cde36331c6869fd5431fcf1895d6b6576bbc2004 into 8fb534e - view on LGTM.com fixed alerts:
|
There was a problem hiding this comment.
Can't move to dbsync, because currently snmp agent also using this:
https://github.com/Azure/sonic-snmpagent/blob/5f8f475b54fbe12b959258bb5567f71577aff0ea/src/sonic_ax_impl/__main__.py
This comment was marked as duplicate.
This comment was marked as duplicate.
We can move all the changes in platform/ and device/ paths into another PR. Those changes need to be tested with proper hardware, reviewed and approved by platform vendors. Other platform-independent changes are relative easy to test. Refers to: device/accton/x86_64-accton_minipack-r0/plugins/led_control.py:1 in 1bcf1fd. [](commit_id = 1bcf1fd7cd9aa83264e4c99182c0dc5287ce2f3d, deletion_comment = False) |
src/sonic-bgpcfgd/bgpmon/bgpmon.py
Outdated
There was a problem hiding this comment.
Not an exact replacement of redis-py pipe, but you may check if RedisPipeline satisfy the usage here.
There was a problem hiding this comment.
The RedisPipeline not have delete and hmset methods, to use RedisPipeline we need build a new RedisCommand command and run the command with RedisPipeline. that basically rewrite the delete and hmset method in DBConnector:
There was a problem hiding this comment.
delete should be straightforward. We need more discussion on hmset
There was a problem hiding this comment.
I will check if we can improve current hmset without using lua script.
There was a problem hiding this comment.
I found the hmset here actually using following API which is not lua script based:
template
void DBConnector::hmset(const std::string &key, InputIterator start, InputIterator stop)
{
RedisCommand shmset;
shmset.formatHMSET(key, start, stop);
RedisReply r(this, shmset, REDIS_REPLY_STATUS);
}
So maybe we can finish this PR first, and improve the lua based hmset with another PR.
There was a problem hiding this comment.
Could you postpone this change to another future PR? we can merge the other files in this PR first.
There was a problem hiding this comment.
Fixed, change in this file reverted.
There was a problem hiding this comment.
If these files are simply moved from another repo, let's separate them into a standalone PR, not to mix with other changes not of the same nature.
There was a problem hiding this comment.
Fixed, create a new PR here: #11347
Also, will update this PR to only contains sonic script change, and split vender HW related change to different PRs.
All hardware related change move to following PRs for different vender: |
1bcf1fd to
659e24f
Compare
There was a problem hiding this comment.
The port_util change split to another PR, this PR need wait for another PR merge first:
#11347
659e24f to
4089311
Compare
Related work items: sonic-net#284, sonic-net#288, sonic-net#624, sonic-net#630, sonic-net#640, sonic-net#2164, sonic-net#2206, sonic-net#2207, sonic-net#2233, sonic-net#2234, sonic-net#2238, sonic-net#8857, sonic-net#10034, sonic-net#10519, sonic-net#10685, sonic-net#10711, sonic-net#10987, sonic-net#10990, sonic-net#11047, sonic-net#11070, sonic-net#11117, sonic-net#11186, sonic-net#11207, sonic-net#11213, sonic-net#11215, sonic-net#11220, sonic-net#11221, sonic-net#11257, sonic-net#11291, sonic-net#11298, sonic-net#11301, sonic-net#11326, sonic-net#11333, sonic-net#11335, sonic-net#11341, sonic-net#11344, sonic-net#11347, sonic-net#11359, sonic-net#11366, sonic-net#11368, sonic-net#11370, sonic-net#11372, sonic-net#11375, sonic-net#11385, sonic-net#11386, sonic-net#11394, sonic-net#11397, sonic-net#11401, sonic-net#11402, sonic-net#11403, sonic-net#11405, sonic-net#11414
Why I did it
Update scripts in sonic-buildimage from py-swsssdk to swsscommon
How I did it
Change code to use swsscommon.
How to verify it
Pass all E2E test case
Which release branch to backport (provide reason below if selected)
Description for the changelog
Update scripts in sonic-buildimage from py-swsssdk to swsscommon
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)