HewlettPackard / python-ilorest-library

Python library for interacting with devices which support a Redfish Service
Apache License 2.0
189 stars 92 forks source link

Highlight hpilo kernel module requirement #164

Closed tacerus closed 1 month ago

tacerus commented 3 months ago

The ilorest_chif.so library seems to handle a lack of the "hpilo" kernel module the same way as a lack of iLO devices, returning "No hpilo devices found" with status 19. This being converted to a "chif" error causes a misleading "Chif driver not found" message if the application is executed on a Linux system having the Chif driver installed, but not having the "hpilo" module loaded. At the point the message is printed, the library was already loaded, indicating it must be correctly present, otherwise it would not have been able to return an error code. Improve user guidance by instead setting an error message similar to the one returned by the library itself, but also pointing out the need for the hpilo kernel module if not executed on Windows. Additionally, highlight this requirement in the README.

-----Synopsis of Commits Above-----

Please fill out the following when submitting the PR

Status

Additional High Level Description

A few sentences describing the overall goals of the pull request's commits.

Dependent PRs

List any PRs that must be merged together. (Tool dependent PRs SHOULD NOT occur.)

Before Status can be set to READY I have completed the following:

tacerus commented 3 months ago

The "Pylint static analysis tests are passing above 9.0/10.0" checklist entry is odd, because already the version in the master branch seems to not pass this requirement. Should a specific version of Pylint be used?

tacerus commented 3 months ago

Tested:

# rmmod hpilo
# ilorest
iLORest : RESTful Interface Tool version 5.1.0.0
Copyright (c) 2014-2024 Hewlett Packard Enterprise Development LP
--------------------------------------------------------------------------------------------------------------------------------------------------------------
ERROR   : No devices were found. Ensure the hpilo kernel module is loaded.
rajeevkallur commented 1 month ago

This will be taken care in the next release 5.3 Thanks for contribution.

tacerus commented 1 month ago

Thanks, but I'm a bit confused how the Git history is handled in this repository. Instead of merging the patches to the master branch and tagging releases from the master branch when ready, it seems all changes are copied into "version X" commits? That somewhat defeats the benefit of Git where you want to have individual feature or fix specific commits in the Git history which then allow for easy diffing of changes and which would contain explanations about the individual patches in their respective commit messages.