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

[202012][multi-asic][sonic-config-engine]: Get PORT table from namespace config db #10475

Merged
merged 7 commits into from
May 27, 2022

Conversation

SuvarnaMeenakshi
Copy link
Contributor

Why I did it
Cherry-pick of: #7632
portconfig.py gets PORT table from config_db if it is present. If not, port_config.ini files are parsed.
For multi-asic platform, if namespace is passed to get_port_config(), config_db connection was done to host namespace always and not to asic specific namespace.
Provides fix for: #7161

How I did it
Modify db connection function to connect to namespace config_db.

How to verify it

Unit-test passed.
Verified on multi-asic VS platform.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

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

…ig db (sonic-net#7632)

Why I did it
portconfig.py gets PORT table from config_db if it is present. If not, port_config.ini files are parsed.
For multi-asic platform, if namespace is passed to get_port_config(), config_db connection was done to host namespace always and not to asic specific namespace.
Provides fix for: sonic-net#7161

How I did it
Modify db connection function to connect to namespace config_db.

(cherry picked from commit 2e305c9)
@wenyiz2021
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

Modify autoneg value to be 1 instead of 'on'.

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
@SuvarnaMeenakshi
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@SuvarnaMeenakshi
Copy link
Contributor Author

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

@qiluo-msft
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

if namespace is not None:
swsscommon.SonicDBConfig.load_sonic_global_db_config(namespace=namespace)
config_db = swsscommon.ConfigDBConnector(use_unix_socket_path=True, namespace=namespace)
except Exception as e:
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 11, 2022

Choose a reason for hiding this comment

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

Exception

What is the type of exception you are expecting? The general exception type will hide bugs and should be limited. #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was added to handle exception if thrown by load_sonic_global_db_config().

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like load_sonic_global_db_config() calls SonicDBConfig::initializeGlobalConfig() which already handled exceptions, and it should be good enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created a PR : #10581 to remove this, I am working on unit-test failure, will merge that PR in master branch first and then take it in to 202012

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qiluo-msft #10581 is merged in master branch to fix this comment. Can we merge this PR to 202012 branch and cherry-pick the other PR to 202012.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#10960 - PR raised to fix this comment.

@SuvarnaMeenakshi SuvarnaMeenakshi merged commit ec9732a into sonic-net:202012 May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants