Platform Driver Development Framework (PDDF): 1) Changes to psu base class (1.0 APIs) to support PDD PSU CLI utils 2) Adding fan_base class to support PDDF fan CLI utils#57
Conversation
…class (1.0 APIs) to support PDDF CLI utils 2) Adding fan_base class to support PDDF fan CLI utils
| class FanBase(object): | ||
| __metaclass__ = abc.ABCMeta | ||
|
|
||
| @abc.abstractmethod |
There was a problem hiding this comment.
can you remove the "abstractmethod" restriction? understood that you are following the convention in other plugin base classes. But with this restriction, it requests all the vendors to implement these functions or at least write some stub functions if they don't want to implement them, otherwise, it will fail when loading the plugins on the vendor's platform. We are in the progress to replace plugins with new platform API, so it's highly possible that some vendors don't want to implement these new plugins, so please consider this.
And I suggest you think about using new platform API instead of adding new plugins, eventually we will use new platform API, put efforts on adding new plugins is kind of waste in my view.
There was a problem hiding this comment.
Okay. We will remove @AbstractMethod restriction.
We plan to move to 2.0 APIs eventually. However, we needed this change for short term to support the CLIs in some of our platforms.
There was a problem hiding this comment.
Agree with @keboliu -- plugins have been deprecated in favor of the new object-oriented platform 2.0 API. Developing new plugins seems unnecessary as they should no longer be used by platform developers.
| """ | ||
| return False | ||
|
|
||
| def get_model(self, idx): |
There was a problem hiding this comment.
you may align the method defied in new platform API, if eventually you still want to go with new plugins.
There was a problem hiding this comment.
I tried to align method names as far as possible, but the methods need index of the PSU.
|
keboliu, Please visit the PR: #62 |
can you close this PR since you created a new one? |
|
Please visit the PR: #62 |
All these changes are w.r.t 1.0 APIs. These will be migrated to 2.0 APIs in future.