Mux cable show config command Added prober_type and fixed one format#4013
Mux cable show config command Added prober_type and fixed one format#4013qiluo-msft merged 1 commit intosonic-net:masterfrom
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
harjotsinghpawra
force-pushed
the
mux_cable
branch
from
222cf8c to
c4b08d7
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
harjotsinghpawra
force-pushed
the
mux_cable
branch
from
c4b08d7 to
b62e08f
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @qiluo-msft - please help merge. |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds the prober_type field to the mux cable show config command and fixes formatting issues to ensure consistent column display regardless of whether optional fields like soc_ipv4 are present.
- Adds
prober_typefield support to both tabular and JSON output formats - Updates test data to include expected
prober_typevalues in output - Fixes column alignment by ensuring all columns are consistently displayed with empty strings for missing values
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/muxcable_test.py | Updates expected test output to include new prober_type column and adds new test cases for prober type configuration |
| tests/mock_tables/config_db.json | Adds prober_type field to mock database entries for Ethernet4 and Ethernet8 |
| show/muxcable.py | Implements prober_type display functionality and fixes formatting to ensure consistent column display |
| config/muxcable.py | Adds new probertype command and updates existing functions to preserve prober_type field |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
|
|
||
| def create_table_dump_per_port_config(db ,print_data, per_npu_configdb, asic_id, port, is_dualtor_active_active): | ||
| def create_table_dump_per_port_config(db, print_data, per_npu_configdb, asic_id, port): |
There was a problem hiding this comment.
The function signature has been simplified by removing the is_dualtor_active_active parameter, but this change should be documented or the function should have a docstring explaining the new behavior and parameter expectations.
| else: | ||
| port_list.append("") |
There was a problem hiding this comment.
[nitpick] The consistent handling of optional fields by appending empty strings is good for formatting, but consider using a constant for the empty string value to make the intention clearer and easier to maintain.
| port_status_dict[port_name] = 'OK' | ||
|
|
||
| def update_configdb_pck_loss_data(config_db, port, val): | ||
| fvs = {} |
There was a problem hiding this comment.
The variable name fvs is unclear. Consider using a more descriptive name like field_values or config_fields to improve code readability.
|
|
||
|
|
||
| #'muxcable' command ("config muxcable packetloss reset <port|all>") | ||
| # 'muxcable' command ("config muxcable packetloss reset <port|all>") |
There was a problem hiding this comment.
The comment formatting is inconsistent. There should be a space after the '#' character to match the formatting style used elsewhere in the file.
…onic-net#4013) Orignal PR was reverted because of pre-test failure as part of following issue sonic-net#3978 orignal PR sonic-net#3884 What I did How I did it Fixed show mux config to one format no matter if soc_ipv4 is given or not How to verify it Ran the test_pretest as part of sonic_mgmt Previous command output (if the output of a command-line utility has changed) port state ipv4 ipv6 cable_type soc_ipv4 soc_ipv6 Ethernet8 auto 192.168.0.2/32 fc02:1000::2/128 active-active 192.168.0.3/32 Ethernet16 auto 192.168.0.4/32 fc02:1000::4/128 active-active 192.168.0.5/32 New command output (if the output of a command-line utility has changed) port state ipv4 ipv6 cable_type soc_ipv4 soc_ipv6 prober_type Ethernet8 auto 192.168.0.2/32 fc02:1000::2/128 active-active 192.168.0.3/32 software Ethernet16 auto 192.168.0.4/32 fc02:1000::4/128 active-active 192.168.0.5/32 software
Orignal PR was reverted because of pre-test failure as part of following issue
#3978
orignal PR #3884
What I did
How I did it
Fixed show mux config to one format no matter if soc_ipv4 is given or not
How to verify it
Ran the test_pretest as part of sonic_mgmt
Previous command output (if the output of a command-line utility has changed)
port state ipv4 ipv6 cable_type soc_ipv4 soc_ipv6
Ethernet8 auto 192.168.0.2/32 fc02:1000::2/128 active-active 192.168.0.3/32
Ethernet16 auto 192.168.0.4/32 fc02:1000::4/128 active-active 192.168.0.5/32
New command output (if the output of a command-line utility has changed)
port state ipv4 ipv6 cable_type soc_ipv4 soc_ipv6 prober_type
Ethernet8 auto 192.168.0.2/32 fc02:1000::2/128 active-active 192.168.0.3/32 software
Ethernet16 auto 192.168.0.4/32 fc02:1000::4/128 active-active 192.168.0.5/32 software