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

[Celestica] Fix platform module to be compatible with Python3 #10051

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lukasstockner
Copy link
Contributor

@lukasstockner lukasstockner commented Feb 21, 2022

Why I did it

Running a current build of master or any of the recent release branches on a Seastone2 switch (and presumably other Celestica switches, since the code in question is shared) results in lots of errors in the logs and many platform-level commands not working as expected since the underlying platform module is not updated for Python 3 yet.

How I did it

By checking through the logs and trying out platform-related commands (e.g. showing SFP EEPROM info), I've identified some issues in the code. Most are related to Python 3 compatibility (includes and bytestring handling).

Together with #9770, this solves all platform issues that I found.

Therefore, this also fixes #8212.

How to verify it

Build the current master image, run it on a Seastone2, observe lots of platform-module-related errors in the syslog.
Build an image containing #9770 and this PR, run it on a Seastone2, check the syslog to see that the errors are gone.

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

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

These three releases are affected by #8212, so they should receive this fix.
Note that when backporting, vendor_rev should be changed to hardware_rev since #9447 is not part of the release branches.

Description for the changelog

Fix Celestica platform module to be compatible with Python3.

Link to config_db schema for YANG module changes

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

1782000_10200889079988336_2136309882_n

@ghost
Copy link

ghost commented Feb 21, 2022

CLA assistant check
All CLA requirements met.

@lukasstockner lukasstockner force-pushed the seastone-2-platform-fix branch 2 times, most recently from c68057d to a749dea Compare May 13, 2022 00:56
Signed-off-by: Lukas Stockner <lstockner@genesiscloud.com>
Signed-off-by: Lukas Stockner <lstockner@genesiscloud.com>
Signed-off-by: Lukas Stockner <lstockner@genesiscloud.com>
@lgtm-com
Copy link

lgtm-com bot commented Nov 10, 2022

This pull request introduces 4 alerts when merging c5bb357 into 10f36d6 - view on LGTM.com

new alerts:

  • 4 for Wrong number of arguments in a class instantiation

@lukasstockner
Copy link
Contributor Author

This pull request introduces 4 alerts when merging c5bb357 into 10f36d6 - view on LGTM.com

new alerts:

  • 4 for Wrong number of arguments in a class instantiation

False positive, it's confusing two different sonic_platforms.

@lguohan lguohan added the device label Sep 23, 2023
Copy link

@grische grische left a comment

Choose a reason for hiding this comment

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

Thanks!

bluecmd added a commit to kamelnetworks/sonic-buildimage that referenced this pull request Feb 3, 2024
bluecmd added a commit to kamelnetworks/sonic-buildimage that referenced this pull request Feb 19, 2024
bluecmd added a commit to kamelnetworks/sonic-buildimage that referenced this pull request Feb 19, 2024
bluecmd added a commit to kamelnetworks/sonic-buildimage that referenced this pull request Mar 3, 2024
bluecmd added a commit to kamelnetworks/sonic-buildimage that referenced this pull request Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Celestica] Seastone 2 DX030 missing sonic_platform
3 participants