Skip to content
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.

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

Namespace support for the python classes. #63

Merged
merged 12 commits into from
Apr 23, 2020

Conversation

judyjoseph
Copy link
Contributor

@judyjoseph judyjoseph commented Mar 5, 2020

  • What I did

This PR is the changes needed to support multiple namespaces for the Multi-ASIC devices. Multi-DB namespace PR --> (sonic-net/SONiC#567)

  • How I did it

The changes are mainly to these classes

  1. SonicDBConfig
  2. SonicV2Connector/ConfigDBConnector

A new parameter "namespace" is added to the SonicV2Connector class init , to pass the namespace name. The default value of is kept as None which is representing case when user didn't give a namespace name.

class SonicV2Connector(DBInterface):
def init(self, use_unix_socket_path=False, namespace=None, **kwargs):

If the user don't explicitly set this parameter, namespace takes '' as value, and connects to the db_name in the local context. When I mean local context, it refers to the database docker running in the current namespace wherever you are running this application/script …. ( It could be either Linux host or any specific network namespace ). In this way it is backward compatible and the existing behavior of APIs in these utility classes are maintained.

In the SonicDBConfig, a new API is introduced load_sonic_global_db_config() which loads the /var/run/redis/sonic-db/database_global.json file if present. This file has the mapping between the namespace name and the corresponding database_config.json file.

  • How to verify it
    Verified the dockers come up in the Multi-ASIC platform and the config commands too were verified to an extend.

  • Description for the changelog

  • A picture of a cute animal (not mandatory but encouraged)

@judyjoseph judyjoseph requested a review from qiluo-msft March 10, 2020 06:08
@qiluo-msft
Copy link
Contributor

qiluo-msft commented Mar 13, 2020

import swsssdk

Suggest using argparse instead of reinvent the wheel. #Closed


Refers to: src/swsssdk/scripts/sonic-db-cli:3 in e61c5e6. [](commit_id = e61c5e6, deletion_comment = False)

@judyjoseph
Copy link
Contributor Author

judyjoseph commented Mar 17, 2020

import swsssdk

Suggest using argparse instead of reinvent the wheel.

Refers to: src/swsssdk/scripts/sonic-db-cli:3 in e61c5e6. [](commit_id = e61c5e6, deletion_comment = False)

Updated the sonic-db-cli implementation using argparse.

@judyjoseph judyjoseph requested a review from qiluo-msft March 17, 2020 05:57
@judyjoseph judyjoseph requested a review from qiluo-msft March 24, 2020 19:02
@sonic-net sonic-net deleted a comment from judyjoseph Mar 26, 2020
Error out if the user don't explicitly call SonicDBConfig.load_sonic_global_db_config in his application.

Also add a new parameter "namespace" to SonicDBConfig.load_sonic_global_db_config()
it is an optimized version and just loads a single json file for that namespace i.s.o loading all json files present.
@judyjoseph judyjoseph requested a review from qiluo-msft April 20, 2020 19:42
@lgtm-com
Copy link

lgtm-com bot commented Apr 20, 2020

This pull request introduces 1 alert when merging 56e057f into 154f381 - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

qiluo-msft
qiluo-msft previously approved these changes Apr 20, 2020
@lguohan lguohan merged commit b9cee36 into sonic-net:master Apr 23, 2020
@rlhui
Copy link

rlhui commented May 4, 2020

@judyjoseph , there's conflict cherry-picking. Please raise a separate PR for 201911 branch for this. Thanks.

@judyjoseph
Copy link
Contributor Author

@judyjoseph , there's conflict cherry-picking. Please raise a separate PR for 201911 branch for this. Thanks.

This PR has dependencies with the changes done in the commits just before this (eg: d1a1512) , so cannot cherrypick this one alone... Is it possible to advance the sonic-py-swssdk submodule in 201911 ? I can raise a PR for advancing the submodule itself in 201911.

abdosi pushed a commit that referenced this pull request May 5, 2020
This PR is the changes needed to support multiple namespaces for the Multi-ASIC devices. Multi-DB namespace PR --> (sonic-net/SONiC#567)

The changes are mainly to these classes

SonicDBConfig
SonicV2Connector/ConfigDBConnector

A new parameter "namespace" is added to the SonicV2Connector class init , to pass the namespace name. The default value is None representing empty namespace.

class SonicV2Connector(DBInterface):
def init(self, use_unix_socket_path=False, namespace=None, **kwargs):

If the user don't explicitly set this parameter, namespace takes None as value, and connects to the db_name in the local context, which refers to the database docker running in the current namespace wherever you are running this application/script …. (It could be either Linux host or any specific network namespace ). In this way it is backward compatible and the existing behavior of APIs in these utility classes are maintained.

In the SonicDBConfig, a new API is introduced load_sonic_global_db_config() which loads the /var/run/redis/sonic-db/database_global.json file if present. This file has the mapping between the namespace name and the corresponding database_config.json file.
self.assertEqual(ns_input, ns_list)

# This is the test to check if the global config file and get the correct DB in a namespace
def test__namespace_list(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicated function name

if SonicDBConfig._sonic_db_global_config_init == True:
return

if os.path.isfile(global_db_file_path) == True:
Copy link
Contributor

Choose a reason for hiding this comment

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

global_db_file_path [](start = 26, length = 19)

If global_db_file_path does not exist, why set _sonic_db_global_config_init to True?

Also can you return _sonic_db_global_config_init value to caller?

praveen-li pushed a commit to praveen-li/sonic-py-swsssdk that referenced this pull request May 4, 2021
This PR is the changes needed to support multiple namespaces for the Multi-ASIC devices. Multi-DB namespace PR --> (sonic-net/SONiC#567)

The changes are mainly to these classes

SonicDBConfig
SonicV2Connector/ConfigDBConnector

A new parameter "namespace" is added to the SonicV2Connector class init , to pass the namespace name. The default value is None representing empty namespace.

class SonicV2Connector(DBInterface):
def init(self, use_unix_socket_path=False, namespace=None, **kwargs):

If the user don't explicitly set this parameter, namespace takes None as value, and connects to the db_name in the local context, which refers to the database docker running in the current namespace wherever you are running this application/script …. (It could be either Linux host or any specific network namespace ). In this way it is backward compatible and the existing behavior of APIs in these utility classes are maintained.

In the SonicDBConfig, a new API is introduced load_sonic_global_db_config() which loads the /var/run/redis/sonic-db/database_global.json file if present. This file has the mapping between the namespace name and the corresponding database_config.json file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants