[sonic_platform_base] Proper use of class and instance attributes #173
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This was referenced Feb 23, 2021
jleveque
changed the title
|
This pull request fixes 1 alert when merging 2110c3a into c6b642b - view on LGTM.com fixed alerts:
|
| self._fan_list = [] | ||
|
|
||
| # List of ThermalBase-derived objects representing all thermals | ||
| # available on the PSU | ||
| self._thermal_list = [] | ||
|
|
||
| self.psu_master_led_color = self.STATUS_LED_COLOR_OFF | ||
| PsuBase._psu_master_led_color = self.STATUS_LED_COLOR_OFF |
There was a problem hiding this comment.
@jleveque Thanks for addressing this!
|
@jleveque thanks for the heads up. We'll make the changes. |
This was referenced Feb 24, 2021
jleveque
pushed a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Feb 24, 2021
- Fix parent `__init__` call for platform API, based on sonic-net/sonic-platform-common#173 - Implementation for more platform API methods - Daily storage information reporting in `arista daemon` - Enhancements to reboot cause reporting, now multi-sourcing reboot cause information - Miscellaneous fixes
lguohan
pushed a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Feb 25, 2021
…alizer (#6854) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
jleveque
added a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Feb 25, 2021
…lizer (#6853) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
jleveque
added a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Feb 25, 2021
…ializer (#6852) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
yxieca
pushed a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Mar 4, 2021
…ializer (#6852) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
yxieca
pushed a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Mar 4, 2021
…lizer (#6853) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
yxieca
pushed a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Mar 4, 2021
…alizer (#6854) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
carl-nokia
pushed a commit
to carl-nokia/sonic-buildimage
that referenced
this pull request
Aug 7, 2021
- Fix parent `__init__` call for platform API, based on sonic-net/sonic-platform-common#173 - Implementation for more platform API methods - Daily storage information reporting in `arista daemon` - Enhancements to reboot cause reporting, now multi-sourcing reboot cause information - Miscellaneous fixes
carl-nokia
pushed a commit
to carl-nokia/sonic-buildimage
that referenced
this pull request
Aug 7, 2021
…alizer (sonic-net#6854) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
carl-nokia
pushed a commit
to carl-nokia/sonic-buildimage
that referenced
this pull request
Aug 7, 2021
…lizer (sonic-net#6853) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
carl-nokia
pushed a commit
to carl-nokia/sonic-buildimage
that referenced
this pull request
Aug 7, 2021
…ializer (sonic-net#6852) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
lolyu
pushed a commit
to lolyu/sonic-buildimage
that referenced
this pull request
Sep 13, 2021
…ializer (sonic-net#6852) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
lolyu
pushed a commit
to lolyu/sonic-buildimage
that referenced
this pull request
Sep 13, 2021
…lizer (sonic-net#6853) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
lolyu
pushed a commit
to lolyu/sonic-buildimage
that referenced
this pull request
Sep 13, 2021
…alizer (sonic-net#6854) In preparation for the merging of sonic-net/sonic-platform-common#173, which properly defines class and instance members in the Platform API base classes. It is proper object-oriented methodology to call the base class initializer, even if it is only the default initializer. This also future-proofs the potential addition of custom initializers in the base classes down the road.
oleksandrivantsiv
pushed a commit
to oleksandrivantsiv/sonic-platform-common
that referenced
this pull request
Oct 25, 2024
Selectable object can return multiple fields, however current code processes only the first field and wait till the next selectable is triggered. This PR consumes all fields that were returned by a selectable object. This results in reducing processing time from order of seconds to order of milliseconds. signed-off-by: Tamer Ahmed <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
__init__.pyMotivation and Context
Proper object-oriented programming. This could prevent issues where attributes are shared between instances when they were not meant to be.
This also fixes a bug with
PsuBase.psu_master_led_color, where it is defined as both a class attribute and an instance attribute.With this change, some vendors' APIs can be cleaned up, because they redefined class attributes as instance attributes. That code can be removed.
However, note that vendor APIs MUST call the base class initializer in their initializer methods, or the instance attributes will not be available and will raise exceptions.
How Has This Been Tested?
Tested on one vendor's platform