Update smartctl.py

[gthvn1]

It superseded #46

[stormi]

This change in behaviour requires:

• An updated README
• A commit message with a bit more details about the changes (answering questions such as "why" et "why this solution"): https://docs.xcp-ng.org/project/development-process/commit-message-conventions/

Also, since the plugin now supports new cases, would it be appropriate to add a new test case that checks explicitly the new cases?

[stormi]

(see previous comments)

[gthvn1]

(see previous comments)

Yes I saw them... I'm waiting for an xcpng 8.3 with megaraid to be able to test it for real and I will update the tests. I will also ping XO team.

[gthvn1]

I pinged the XO team about the modification: vatesfr/xen-orchestra#8272

[gthvn1]

@stormi & @benjamreis about adding new tests for megaraid. The problem I have is that currently the plugin is looping through devices and run the command for each of them. Like:

        for device in devices:
            cmd = run_command(["smartctl", "-j", "-a", "-d", device['type'], device['name']], check=False)
            results[device['name'] + ":" + device['type']] = json.loads(cmd['stdout'])

As the command cannot be run for unit test I mocked it. It is fine because I currently have only one device. But how can I mock the run_command to return a different output for each devices ? If I add megaraid devices I will have three different outputs to return.
I guess there should be kind of yield but I don't know the syntax and how to use it for now. I will look in the pytest doc but if you already know :)

EDITED: it looks like we can have iterable side_effect... let's try that :)

[gthvn1]

Note: I squashed all commit from @1234Erwan into one commit to cleanup the tree. So I think it is more readable and this allows the work that has done to be preserved.

I updated the Readme and the tests so I think it is ok now

[gthvn1]

@stormi @benjamreis no hurry but it is ok to review it now since XO team verified that there was no impact on their side 👍

[stormi]

It looks good to me. I only had a quick overview of the code and mostly focused on the described functional changes. I'm counting on @benjamreis (or someone else if that's not his responsibility anymore) to review the code in detail if needed.

[benjamreis]

It looks good to me. I only had a quick overview of the code and mostly focused on the described functional changes. I'm counting on @benjamreis (or someone else if that's not his responsibility anymore) to review the code in detail if needed.

I'll review it this afternoon - since I already took a look I think its okay for me to re-review it 👍