Skip to content

Add a new API connect_host to SonicV2Connector #1003

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Add a new API connect_host to SonicV2Connector #1003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
778707c
Add a new API connect_host to SonicV2Connector
vivekverma-arista Apr 9, 2025
7a6b54d
Update test_redis_ut.py
vivekverma-arista Apr 10, 2025
1a2e241
Merge branch 'master' into agg-voq
vivekverma-arista Apr 13, 2025
2aee6a5
Merge branch 'master' into agg-voq
vivekverma-arista Apr 16, 2025
bae2ab4
Merge branch 'sonic-net:master' into agg-voq
vivekverma-arista Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions common/sonicv2connector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ void SonicV2Connector_Native::connect(const std::string& db_name, bool retry_on)
m_dbintf.connect(db_id, db_name, retry_on);
}

void SonicV2Connector_Native::connect_host(const std::string& db_name, const std::string& host, bool retry_on)
{
m_dbintf.set_redis_kwargs("", host, get_db_port(db_name));
int db_id = get_dbid(db_name);
m_dbintf.connect(db_id, db_name, retry_on);
}

void SonicV2Connector_Native::close(const std::string& db_name)
{
m_dbintf.close(db_name);
Expand Down
2 changes: 2 additions & 0 deletions common/sonicv2connector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class SonicV2Connector_Native

void connect(const std::string& db_name, bool retry_on = true);

void connect_host(const std::string& db_name, const std::string& host, bool retry_on = true);

void close(const std::string& db_name);

void close();
Expand Down
2 changes: 2 additions & 0 deletions tests/test_redis_ut.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,9 @@
db = SonicV2Connector(host='127.0.0.1')
db = SonicV2Connector(use_unix_socket_path=True, namespace='', decode_responses=True)
db = SonicV2Connector(use_unix_socket_path=False, decode_responses=True)
db = SonicV2Connector(host="127.0.0.1", decode_responses=True)

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning test

This assignment to 'db' is unnecessary as it is
redefined
Loading
before this value is used.
db = SonicV2Connector()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

connect_host

Class SonicV2Connector is for historic compatible purpose. For your use case, I believe DBConnector is what your need, providing your all the flexibility to connect to a host and port. Could you explore?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like in the queuestat: https://github.com/sonic-net/sonic-swss-common/blob/master/common/dbconnector.cpp#L655-L664 can be used. But it will need significant adjustments in the unit test in our case.

The infra for mimicking SonicV2Connector is already in place, it may not be easy to do that with DBConnector

https://github.com/sonic-net/sonic-utilities/blob/master/tests/mock_tables/dbconnector.py#L231-L240

Copy link
Contributor

@qiluo-msft qiluo-msft Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should first discuss which design fit your needs more properly. The effort of test adjustments and how to mimicking should not revert the design decision. Sorry if you already spend time on testcode, but we need to be careful when extending interfaces in swss-common repo.

db.connect_host(db_name="TEST_DB", host='127.0.0.1')
except:
assert False, 'Unexpected exception raised'

Expand Down
Loading