search

truenas / py-SMART

Wrapper for smartctl (smartmontools)
GNU Lesser General Public License v2.1
76 stars 35 forks source link

Rationale for defaulting `SMARTCTL_PATH` to `/usr/local/bin/smartctl` and not just `smartctl`? #10

Closed intelfx closed 4 years ago

intelfx commented 7 years ago

What is the rationale for having

SMARTCTL_PATH = '/usr/local/sbin/smartctl'

and not just 'smartctl', given that subprocess.Popen() performs PATH lookup?

surajrav commented 7 years ago

I am not sure why it did not work, but when I tried it sure did not.

It could be because of the fact that I am importing and using pySMART module in a heavily gevent patched python process (and gevent does monkey patch subprocess too)

intelfx commented 7 years ago

@surajrav Maybe /usr/local/sbin is not in your PATH?

surajrav commented 7 years ago

Yes could very well be, but I prefer to keep the path this way. I cannot guarantee that /usr/local/sbin will be in the path or not and to rely on that for my code to work seemed fragile. May I ask an honest question, why does it matter so much? Maybe if I see the reasoning behind your complaint to this we can solve the problem together much more effectively

intelfx commented 7 years ago

Initially, I wanted to package pySMART for Arch Linux. I contacted original author of pySMART about python 3 support and he told me that he stopped maintaining his project, instead pointing me to this project as the most complete fork so far.

Now, as Arch Linux has a different (more standards-compliant) file system hierarchy, this code doesn't work there as-is. So I decided to ask whether there are reasons to hardcode non-standard pathes instead of writing universal code that works everywhere.

Also please look at pull-request #11.

intelfx commented 7 years ago

@surajrav Are you going to review the PR #11?

surajrav commented 7 years ago

Yes I will try to get to it this week, a lot on my plate though.

I do not like the idea of relying on the path (since in my product i.e. FreeNAS that is a problem). However, let me see what I can do about it

ghost commented 4 years ago

Closed by #21