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

Fix to get all port related attributes from config_db #362

Merged
merged 5 commits into from
Nov 13, 2018

Conversation

paavaanan
Copy link
Contributor

@paavaanan paavaanan commented Oct 31, 2018

- What I did

  • interface_naming_mode functions fetch the alias_interface_name via sonic-cfggen. But, with the recent change in config_db removes all port related entries when a sepecific interface is down. This affects the existing implementation of fetching port details to following fucntions [ interface_alias_to_name(), interface_name_to_allias(), InterfaceAliasConverter() ] and throws key error.

- How I did it

  • To fix this, harnessed the existing port_config.ini file to fetch the interface details and populated a port_dict and this fetch the alias interface names quickly.

NOTE: config commands are entirely modified from linux commands to db format. This fix does not cover interface validation for those commands.

@paavaanan paavaanan changed the title interface_nameing_mode alias errors KeyError: 'alias' when we try to use "show" command after shut / unshut the interface using config interface [InterfaceName] [shutdown | startup] Oct 31, 2018
@paavaanan
Copy link
Contributor Author

paavaanan commented Oct 31, 2018

This fix address the issue
sonic-net/SONiC#268

config/main.py Outdated Show resolved Hide resolved
show/main.py Outdated Show resolved Hide resolved
show/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
@stcheng
Copy link
Contributor

stcheng commented Nov 1, 2018

@paavaanan thanks for addresing the commits. could you also change the title of this pull request into a brief one? the current one contains some details that shall be moved into the commit message.

@paavaanan paavaanan changed the title KeyError: 'alias' when we try to use "show" command after shut / unshut the interface using config interface [InterfaceName] [shutdown | startup] [sonic-cfggen]: Optimizing config.py to use config_db to fetch port related information and remove sonic-cfggen support from thes script. Nov 2, 2018
@paavaanan paavaanan changed the title [sonic-cfggen]: Optimizing config.py to use config_db to fetch port related information and remove sonic-cfggen support from thes script. [sonic-cfggen]: Optimizing config/main.py to use config_db to fetch port related information and remove sonic-cfggen support from thes script. Nov 2, 2018
@paavaanan paavaanan changed the title [sonic-cfggen]: Optimizing config/main.py to use config_db to fetch port related information and remove sonic-cfggen support from thes script. [sonic-cfggen]: Optimizing config/main.py to use config_db to fetch port related information and remove sonic-cfggen from the script. Nov 2, 2018
@stcheng
Copy link
Contributor

stcheng commented Nov 4, 2018

@paavaanan this is a nice article about the git commit message: https://medium.com/@preslavrachev/what-s-with-the-50-72-rule-8a906f61f09c

config/main.py Show resolved Hide resolved
config/main.py Show resolved Hide resolved
show/main.py Show resolved Hide resolved
@paavaanan
Copy link
Contributor Author

@paavaanan this is a nice article about the git commit message: https://medium.com/@preslavrachev/what-s-with-the-50-72-rule-8a906f61f09c

Thanks shoutian

config/main.py Outdated
PLATFORM_ROOT_PATH = '/usr/share/sonic/device'
SONIC_CFGGEN_PATH = '/usr/local/bin/sonic-cfggen'
HWSKU_KEY = 'DEVICE_METADATA.localhost.hwsku'
PLATFORM_KEY = 'DEVICE_METADATA.localhost.platform'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add definitions for PLATFORM_ROOT_PATH, HWSKU_KEY and PLATFORM_KEY? They are not used anywhere in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Joe, it is added as part of the initiall pull request. This is not necessary. Will remove the lines

config/main.py Outdated
@@ -12,17 +12,20 @@
from swsssdk import ConfigDBConnector
from natsort import natsorted
from minigraph import parse_device_desc_xml
from portconfig import get_port_config
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add this import if you are not using it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed in the new commit

show/main.py Outdated
PLATFORM_ROOT_PATH = '/usr/share/sonic/device'
SONIC_CFGGEN_PATH = '/usr/local/bin/sonic-cfggen'
HWSKU_KEY = 'DEVICE_METADATA.localhost.hwsku'
PLATFORM_KEY = 'DEVICE_METADATA.localhost.platform'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add these four constants? They are not used anywhere in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other than SONIC_CFGGEN_PATH removed unused constants

show/main.py Outdated
@@ -15,9 +15,15 @@
import sonic_platform
from swsssdk import ConfigDBConnector
from swsssdk import SonicV2Connector
from portconfig import get_port_config
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add this import if you are not using it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed in new commit

Copy link
Contributor

Choose a reason for hiding this comment

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

? This was not removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in new commit!

Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

As comments. Also, as Shuotian has requested, please abbreviate your PR title. It is a bit long. One place to start is to remove the opening [sonic-cfggen]: because this PR doesn't directly affect sonic-cfggen, which resides in the sonic-buildimage repo.

@paavaanan paavaanan changed the title [sonic-cfggen]: Optimizing config/main.py to use config_db to fetch port related information and remove sonic-cfggen from the script. Fix to get all port related attributes from config_db Nov 10, 2018
@stcheng
Copy link
Contributor

stcheng commented Nov 12, 2018

thanks for fixing that!

@jleveque jleveque merged commit ac67208 into sonic-net:master Nov 13, 2018
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 28, 2022
 Add support for attribute capability query in lua script (sonic-net#362)
mihirpat1 pushed a commit to mihirpat1/sonic-utilities that referenced this pull request Sep 15, 2023
EmmcUtil class implements SsdBase API and can be used by ssdutil instead
of the default SsdUtil class on platforms that have eMMC.
The class is copied from Arista platform.

Signed-off-by: Yakiv Huryk <[email protected]>
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.

3 participants