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

[DellEMC]: Platform modules Python3 compliance and other changes #5609

Merged
merged 3 commits into from
Oct 17, 2020

Conversation

ArunSaravananBalachandran
Copy link
Contributor

@ArunSaravananBalachandran ArunSaravananBalachandran commented Oct 13, 2020

- Why I did it

  • Make DellEMC platform modules Python3 compliant.
  • Change return type of PSU Platform APIs in DellEMC Z9264, S5232 and Thermal Platform APIs in S5232 to 'float'.
  • Remove multiple copies of pcisysfs.py.
  • PEP8 style changes for utility scripts.
  • Build and install Python3 version of sonic_platform package.
  • Fix minor Platform API issues.

- How I did it

  • Used 2to3 and manual assessment for Python3 compliant changes.
  • Made changes in thermal.py, psu.py to return 'float' value.
  • Replaced multiple copies of pcisysfs.py with a single one in common directory.

- How to verify it

  • Verified the values read/write by utility scripts.
  • Installed Python3 version of 'sonic_platform_base' and used a python script to load Chassis class and then call the APIs accordingly and verify the return type.

UT Logs:
S6100_py2to3_logs.txt
S6000_py2to3_logs.txt
S5232_py2to3_logs.txt
Z9264_py2to3_logs.txt

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

  • 201811
  • 201911
  • 202006

- Description for the changelog

[DellEMC]: Platform modules Python3 compliance, Platform API fixes and other changes.

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

@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2020

This pull request fixes 19 alerts when merging 86ee138 into bba5df0 - view on LGTM.com

fixed alerts:

  • 16 for Unused import
  • 2 for Unused local variable
  • 1 for Result of integer division may be truncated

@jleveque
Copy link
Contributor

@ArunSaravananBalachandran: Do you intend to build a Python 3 version of your sonic_platform package in a separate PR?

@ArunSaravananBalachandran
Copy link
Contributor Author

@ArunSaravananBalachandran: Do you intend to build a Python 3 version of your sonic_platform package in a separate PR?

Have added a new commit to build and install Python3 version of sonic_platform package.

@lgtm-com
Copy link

lgtm-com bot commented Oct 14, 2020

This pull request fixes 19 alerts when merging 2045a5d into 812e1a3 - view on LGTM.com

fixed alerts:

  • 16 for Unused import
  • 2 for Unused local variable
  • 1 for Result of integer division may be truncated

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.

FYI, I recently removed the get_serial_number() method from chassis_base.py here, since it is redundant with the get_serial() method inherited from DeviceBase. Please go ahead and remove the corresponding method definitions from your chassis.py files.

@ArunSaravananBalachandran
Copy link
Contributor Author

FYI, I recently removed the get_serial_number() method from chassis_base.py here, since it is redundant with the get_serial() method inherited from DeviceBase. Please go ahead and remove the corresponding method definitions from your chassis.py files.

Thanks for the note. Updated the PR with the suggested changes.

@lgtm-com
Copy link

lgtm-com bot commented Oct 15, 2020

This pull request fixes 21 alerts when merging 1390e0f into 4c8b1c4 - view on LGTM.com

fixed alerts:

  • 18 for Unused import
  • 2 for Unused local variable
  • 1 for Result of integer division may be truncated

@jleveque
Copy link
Contributor

Retest this please

@jleveque
Copy link
Contributor

jleveque commented Oct 17, 2020

Check builds are not running. Will close and reopen PR in an attempt to fix ...

@jleveque jleveque closed this Oct 17, 2020
@jleveque jleveque reopened this Oct 17, 2020
@jleveque
Copy link
Contributor

Close and reopen PR appears to have worked :)

@jleveque
Copy link
Contributor

Retest broadcom please

@jleveque jleveque merged commit bcc6c64 into sonic-net:master Oct 17, 2020
jleveque added a commit that referenced this pull request Oct 18, 2020
… in Chassis class (#5649)

The `get_serial_number()` method in the ChassisBase and ModuleBase classes was redundant, as the `get_serial()` method is inherited from the DeviceBase class. This method was removed from the base classes in sonic-platform-common and the submodule was updated in #5625.

This PR aligns the existing vendor platform API implementations to remove the `get_serial_number()` methods and ensure the `get_serial()` methods are implemented, if they weren't previously.

Note that this PR does not modify the Dell platform API implementations, as this will be handled as part of #5609
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
…ic-net#5609)

- Make DellEMC platform modules Python3 compliant.
- Change return type of PSU Platform APIs in DellEMC Z9264, S5232 and Thermal Platform APIs in S5232 to 'float'.
- Remove multiple copies of pcisysfs.py.
- PEP8 style changes for utility scripts.
- Build and install Python3 version of sonic_platform package.
- Fix minor Platform API issues.
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
… in Chassis class (sonic-net#5649)

The `get_serial_number()` method in the ChassisBase and ModuleBase classes was redundant, as the `get_serial()` method is inherited from the DeviceBase class. This method was removed from the base classes in sonic-platform-common and the submodule was updated in sonic-net#5625.

This PR aligns the existing vendor platform API implementations to remove the `get_serial_number()` methods and ensure the `get_serial()` methods are implemented, if they weren't previously.

Note that this PR does not modify the Dell platform API implementations, as this will be handled as part of sonic-net#5609
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.

2 participants