-
Notifications
You must be signed in to change notification settings - Fork 689
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
Support "show cli" for Multi Npu platforms. #889
Conversation
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
This pull request introduces 4 alerts when merging 32568af into fc719ad - view on LGTM.com new alerts:
|
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@arlakshm: FYI, whenever adding/removing/modifying subcommands in the user-facing CLI ( |
def db_connect_configdb(): | ||
|
||
def db_config_unload(): | ||
swsssdk.SonicDBConfig._sonic_db_global_config_init = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_variable_names are intended to be used as private variables. Suggest writing setter decorator.
sorted_table = natsorted(table) | ||
print tabulate(sorted_table, header_stat if not sub_intf_only else header_stat_sub_intf, tablefmt="simple", stralign='right') | ||
|
||
|
||
def __init__(self, intf_name): | ||
def __init__(self, intf_name,display_opt, namespace=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intf_name, display_opt
import sonic_device_util | ||
from show.multi_npu import multi_npu_process_options | ||
from show.multi_npu import skip_intf_display | ||
from show.multi_npu import DISPLAY_ALL as DISPLAY_ALL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you intend to use a different name for DISPLAY_ALL
@arlakshm Wondering are there any changes required for this PR, since there's also a PR for Multi-ASIC object. There's some common functions between Multi-ASIC and show/multi_npu.py. Are we fine with using 'npu', I remember we wanted to use 'asic'. |
BACKPLANE_INTERFACE_PREFIX = "Ethernet-BP" | ||
DISPLAY_ALL = 'all' | ||
DISPLAY_EXTERNAL = 'frontend' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be moved to 'constants.py' that you created.
Closing this PR. The changes have been spilit into multiple PRs
|
Closing this PR |
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan arlakshm@microsoft.com
- What I did
Support the following show commands for Multi NPU platforms.
To each of the above mentioned commands 2 new options have added
[-n, --namespace] to allow user to display the information for given namespaces
If this option is not present the information from all the namespaces will be displayed
[-d, --display] to allow user to display information related both internal and external interfaces
If this option is not present only external interfaces/neighbors will be display
One single NPU platform, this options are not valid, so the behavior remains unchanged
- How I did it
The following changes are done in this PR.
Two new arguments are added intfutil script
"-n" and "-d" for support the multi npu options described above
Two new arguments are added intfutil script
"-n" and "-d" for support the multi npu options described above
- How to verify it
Sample output
Help menu on single Npu platforms
Help menu on multi Npu platforms
Sample Interface Status
Sample output for specific ASIC
Sample output of specific asic display internal and external interfaces
- Previous command output (if the output of a command-line utility has changed)
- New command output (if the output of a command-line utility has changed)