Skip to content

[Mellanox] Refactor Mellanox platform API to support dynamic port configuration#8422

Merged
liat-grozovik merged 13 commits intosonic-net:masterfrom
Junchao-Mellanox:modular-chassis
Oct 25, 2021
Merged

[Mellanox] Refactor Mellanox platform API to support dynamic port configuration#8422
liat-grozovik merged 13 commits intosonic-net:masterfrom
Junchao-Mellanox:modular-chassis

Conversation

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator

@Junchao-Mellanox Junchao-Mellanox commented Aug 11, 2021 •
edited by liat-grozovik
Loading

Why I did it

  1. To support systems with dynamic port configuration
  2. Apply lazy initialization to faster the speed of loading platform API

How I did it

  1. Add module.py to implement dynamic port configuration (aka line card model)
  2. Adjust chassis.py, platform.py, thermal.py, sfp.py to support dynamic port configuration
  3. Optimize existing code

How to verify it

  1. Platform regression on MSN4700, MSN3800 and MSN2700, 100% pass
  2. Unit test covers all new changes.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

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

@Junchao-Mellanox Junchao-Mellanox mentioned this pull request Aug 11, 2021
Merged
keboliu
keboliu previously approved these changes Aug 11, 2021
View reviewed changes
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Aug 11, 2021

This pull request introduces 8 alerts and fixes 16 when merging d70896e into d6433d1 - view on LGTM.com

new alerts:

  • 6 for Unused import
  • 1 for Module is imported more than once
  • 1 for First argument to super() is not enclosing class

fixed alerts:

  • 6 for Unused import
  • 5 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 1 for Except block handles 'BaseException'

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Aug 12, 2021

This pull request fixes 16 alerts when merging b163ae0 into 9417fe9 - view on LGTM.com

fixed alerts:

  • 6 for Unused import
  • 5 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 1 for Except block handles 'BaseException'

@liat-grozovik liat-grozovik changed the title [Mellanox] Refactor Mellanox platform API to support modular chassis [Mellanox] Refactor Mellanox platform API to support dynamic port configuration Aug 16, 2021
@liat-grozovik
Copy link
Copy Markdown
Collaborator

liat-grozovik commented Aug 16, 2021

@keboliu please review
@lguohan please assign someone to review

keboliu
keboliu previously approved these changes Aug 27, 2021
View reviewed changes
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Aug 27, 2021

This pull request fixes 16 alerts when merging 02fd3ae into 48ba459 - view on LGTM.com

fixed alerts:

  • 6 for Unused import
  • 5 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 1 for Except block handles 'BaseException'

@keboliu keboliu requested review from prgeor and sujinmkang September 9, 2021 01:04
@liat-grozovik liat-grozovik removed the request for review from lguohan September 13, 2021 09:25
keboliu
keboliu previously approved these changes Sep 14, 2021
View reviewed changes
sujinmkang
sujinmkang previously approved these changes Sep 15, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@sujinmkang sujinmkang left a comment

Choose a reason for hiding this comment

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

LGTM

@liat-grozovik
Copy link
Copy Markdown
Collaborator

liat-grozovik commented Sep 29, 2021

@Junchao-Mellanox please note

liat-grozovik
liat-grozovik requested changes Sep 29, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@liat-grozovik liat-grozovik left a comment

Choose a reason for hiding this comment

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

see comment

@Junchao-Mellanox Junchao-Mellanox dismissed stale reviews from sujinmkang and keboliu via 0111475 September 30, 2021 00:56
@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Junchao-Mellanox commented Sep 30, 2021

To avoid conflict with #8799, I will add copyright to newly added files of this PR.

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Junchao-Mellanox commented Oct 13, 2021 

E           Failed: Processes "['check_other_vms--ARISTA11T2']" had failures. Please check the logs

Looks like some deploy issue.

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Junchao-Mellanox commented Oct 13, 2021

/azpw run

@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Oct 13, 2021

/AzurePipelines run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Oct 13, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 18, 2021

This pull request fixes 16 alerts when merging fa4a6d7 into 5356244 - view on LGTM.com

fixed alerts:

  • 6 for Unused import
  • 5 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 1 for Except block handles 'BaseException'

qiluo-msft pushed a commit to sonic-net/sonic-platform-daemons that referenced this pull request Oct 18, 2021
@Junchao-Mellanox
[thermalctld] Update line card thermal sensor status to DB (#211)
1565a23 
Update line card thermal sensor status to DB, includes PSU thermal sensors and SFP thermal sensors on line card. Depends on sonic-net/sonic-buildimage#8422.

#### Description

In thermal update function, update PSU, SFP and direct thermal of line card

#### Motivation and Context

To support modular chassis

#### How Has This Been Tested?

1. Full platform regression, 100% passed
2. Unit test passed
keboliu
keboliu previously approved these changes Oct 19, 2021
View reviewed changes
prgeor
prgeor previously approved these changes Oct 20, 2021
View reviewed changes
liat-grozovik
liat-grozovik reviewed Oct 20, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@liat-grozovik liat-grozovik left a comment

Choose a reason for hiding this comment

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

please update mellanox files with Nvidia copyright

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 20, 2021

This pull request fixes 16 alerts when merging ceef999 into 9527cbe - view on LGTM.com

fixed alerts:

  • 6 for Unused import
  • 5 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 1 for Except block handles 'BaseException'

@Junchao-Mellanox Junchao-Mellanox mentioned this pull request Oct 22, 2021
Merged
5 tasks
@Junchao-Mellanox Junchao-Mellanox dismissed stale reviews from prgeor and keboliu via 7325a5a October 22, 2021 02:57
keboliu
keboliu previously approved these changes Oct 22, 2021
View reviewed changes
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 22, 2021

This pull request introduces 1 alert and fixes 16 when merging 7325a5a into 5b5f3f0 - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 6 for Unused import
  • 5 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 1 for Except block handles 'BaseException'

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 22, 2021

This pull request fixes 16 alerts when merging 91ddc95 into 5b5f3f0 - view on LGTM.com

fixed alerts:

  • 6 for Unused import
  • 5 for Wrong number of arguments in a class instantiation
  • 4 for Unused local variable
  • 1 for Except block handles 'BaseException'

@liat-grozovik liat-grozovik merged commit e8b4c2a into sonic-net:master Oct 25, 2021
@Junchao-Mellanox Junchao-Mellanox deleted the modular-chassis branch October 29, 2021 01:42
@tomer-israel tomer-israel mentioned this pull request Nov 11, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liat-grozovik liat-grozovik liat-grozovik left review comments

@keboliu keboliu keboliu approved these changes

@prgeor prgeor prgeor approved these changes

+1 more reviewer

@sujinmkang sujinmkang sujinmkang left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Platform: Mellanox

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Junchao-Mellanox @liat-grozovik @mssonicbld @keboliu @prgeor @sujinmkang