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

[ycabled] add more coverage to ycabled; add minor name change for vendor API CLI return key-values pairs #338

Merged
merged 5 commits into from
Feb 10, 2023

Conversation

vdahiya12
Copy link
Contributor

@vdahiya12 vdahiya12 commented Feb 7, 2023

The main motivation of this PR is improve coverage of ycabled.
While fixing unit test cases found an improvement to the name key-values returned to CLI, changed those names for keys as well

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description

Motivation and Context

How Has This Been Tested?

Unit-Tests and running the changes on testbed.

Additional Information (Optional)

fail sceanrio's

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 changed the title [ycabled] add more coverage to ycabled; add minor logic for vendor API failure cases [ycabled] add more coverage to ycabled; add minor name change for vendor API CLI return key-values pairs Feb 8, 2023
@@ -2665,7 +2666,7 @@ def handle_show_ber_cmd_arg_tbl_notification(fvp, xcvrd_show_ber_cmd_arg_tbl, xc
helper_logger.log_warning("Failed to execute cli cmd get_alive_status API for port {} due to {}".format(physical_port,repr(e)))
if res is not None:
fvs_log = swsscommon.FieldValuePairs(
[("cable", str(res))])
Copy link
Collaborator

Choose a reason for hiding this comment

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

without this fix what was the impact?

Copy link
Contributor Author

@vdahiya12 vdahiya12 Feb 10, 2023

Choose a reason for hiding this comment

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

This will just result in executing the show CLI
$show mux cablealivestatus as

PORT       ATTR    HEALTH
---------  ------  --------
 Ethernet0  cable True

I think preferred way would be

PORT       ATTR    HEALTH
---------  ------  --------
Ethernet0 cable_alive True

@prgeor prgeor added Y-Cable and removed Y-Cable labels Feb 10, 2023
@vdahiya12 vdahiya12 merged commit f132d12 into sonic-net:master Feb 10, 2023
qiluo-msft pushed a commit that referenced this pull request Feb 14, 2023
…dor API CLI return key-values pairs (#338)

The main motivation of this PR is improve coverage of ycabled.
While fixing unit test cases found an improvement to the name key-values returned to CLI, changed those names for keys as well

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description
Motivation and Context
How Has This Been Tested?
Unit-Tests and running the changes on testbed.

Additional Information (Optional)
yxieca pushed a commit that referenced this pull request Feb 21, 2023
…dor API CLI return key-values pairs (#338)

The main motivation of this PR is improve coverage of ycabled.
While fixing unit test cases found an improvement to the name key-values returned to CLI, changed those names for keys as well

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description
Motivation and Context
How Has This Been Tested?
Unit-Tests and running the changes on testbed.

Additional Information (Optional)
AntonHryshchuk added a commit to AntonHryshchuk/sonic-buildimage that referenced this pull request Feb 22, 2023
Update sonic-platform-daemons submodule pointer to include the following:
* f132d12 [ycabled] add more coverage to ycabled; add minor name change for vendor API CLI return key-values pairs ([sonic-net#338](sonic-net/sonic-platform-daemons#338))

Signed-off-by: AntonHryshchuk <antonh@nvidia.com>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Feb 23, 2023
Update sonic-platform-daemons submodule pointer to include the following:
* 05dd3bd Update CMIS module types for 2x100G AOC support ([sonic-net#339](sonic-net/sonic-platform-daemons#339))
* f132d12 [ycabled] add more coverage to ycabled; add minor name change for vendor API CLI return key-values pairs ([sonic-net#338](sonic-net/sonic-platform-daemons#338))

Signed-off-by: dprital <drorp@nvidia.com>
liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Feb 23, 2023
Update sonic-platform-daemons submodule pointer to include the following:
* 05dd3bd Update CMIS module types for 2x100G AOC support ([#339](sonic-net/sonic-platform-daemons#339))
* f132d12 [ycabled] add more coverage to ycabled; add minor name change for vendor API CLI return key-values pairs ([#338](sonic-net/sonic-platform-daemons#338))

Signed-off-by: dprital <drorp@nvidia.com>
lguohan pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Feb 24, 2023
Update sonic-platform-daemons submodule head to include:

05dd3bd mihirpat1 Wed Feb 22 09:19:13 2023 -0800 Update CMIS module types for 2x100G AOC support (sonic-net/sonic-platform-daemons#339)
f132d12 vdahiya12 Thu Feb 9 18:01:38 2023 -0800 [ycabled] add more coverage to ycabled; add minor name change for vendor API CLI return key-values pairs (sonic-net/sonic-platform-daemons#338)

Update sonic-platform-common submodule head to include:
85c20cd mihirpat1 Wed Feb 22 09:18:20 2023 -0800 Update host electrical interface for 2x100G AOC (sonic-net/sonic-platform-common#346)

Signed-off-by: Mihir Patel <patelmi@microsoft.com>
StormLiangMS pushed a commit that referenced this pull request Mar 7, 2023
…dor API CLI return key-values pairs (#338)

The main motivation of this PR is improve coverage of ycabled.
While fixing unit test cases found an improvement to the name key-values returned to CLI, changed those names for keys as well

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description
Motivation and Context
How Has This Been Tested?
Unit-Tests and running the changes on testbed.

Additional Information (Optional)
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.

5 participants