Implement python-dmidecode/dmidecode as alternative for kernel DMI #2479

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

dagwieers commented Mar 21, 2013

This implementation falls back to python-dmidecode (RHEL5.5+) if the kernel as no DMI support. Alternatively, if python-dmidecode is missing, we attempt to use the dmidecode binary (for RHEL5.4 and older) before giving up.

This fixes #376 and #1657 and also helps @lwade on RHEL5.5+.

Hi

Two typos: missing , at EOL line 462 and line 490. Also at least for Dell servers some of the DMI_DICT XML paths are different:

@@ -459,7 +459,7 @@
 
             DMI_DICT = dict(
                 bios_date       = '/sys/devices/virtual/dmi/id/bios_date',
-                bios_version    = '/sys/devices/virtual/dmi/id/bios_version'
+                bios_version    = '/sys/devices/virtual/dmi/id/bios_version',
                 form_factor     = '/sys/devices/virtual/dmi/id/chassis_type',
                 product_name    = '/sys/devices/virtual/dmi/id/product_name',
                 product_serial  = '/sys/devices/virtual/dmi/id/product_serial',
@@ -486,9 +486,9 @@
                 import dmidecode
 
                 DMI_DICT = dict(
-                    bios_date       = '/dmidecode/BiosInfo/ReleaseDate',
-                    bios_version    = '/dmidecode/BiosInfo/BiosRevision'
-                    form_factor     = '/dmidecode/ChassisInfo/Type',
+                    bios_date       = '/dmidecode/BIOSinfo/ReleaseDate',
+                    bios_version    = '/dmidecode/BIOSinfo/BIOSrevision',
+                    form_factor     = '/dmidecode/ChassisInfo/ChassisType',
                     product_name    = '/dmidecode/SystemInfo/ProductName',
                     product_serial  = '/dmidecode/SystemInfo/SerialNumber',
                     product_uuid    = '/dmidecode/SystemInfo/SystemUUID',
Contributor

dagwieers commented Mar 22, 2013

Thanks, I ordered the entries alphabetically before pushing late at night, and contrary to what I always do, the last line did not have a trailing comma.

Contributor

mpdehaan commented Mar 23, 2013

the unguarded try/except really should be

except ImportError

so it doesn't eat other exceptions.

Contributor

mpdehaan commented Mar 23, 2013

also question, are you intending to fix the FIXME?

FIXME: Fall back to using dmidecode, if available

Contributor

dagwieers commented Mar 23, 2013

I plan to implement dmidecode-binary mode at some point, can't say when though.

PS try/except fixed, thanks !

Contributor

mpdehaan commented Mar 24, 2013

Seems instead of NA it should use Python None too, and the try/except should guard only the import statement, and do something like HAS_DMIDECODE=True and then use that conditionally.

Imports should occur at the top of the file.

Contributor

dagwieers commented Mar 24, 2013

Yes, None makes more sense, however the original code returned NA. I will look into it.

Why should imports always occur at the top of the file ?

Contributor

mpdehaan commented Mar 26, 2013

It's just a python convention thing, that way you can look at the top and see what the module is likely to require (or if any modules aren't being used)

NA actually makes sense I think, or maybe just omitting the facts would be ok too (since they are not available, almost as if it were another platform)

Member

bcoca commented Mar 27, 2013

One reason is to always keep them on top is for easy spotting of
duplication.

Import is not handled conditionally, so it makes very little sense to put
inside specific code (aside from try/except ImportError).


Brian Coca

Contributor

mpdehaan commented Apr 5, 2013

Yep, breaking things up into some smaller functions would also be good... I understand this is not your problem for it already being like that, but it makes the code easier to understand/organize.

Contributor

mpdehaan commented Apr 23, 2013

@dagwieers

Any update on this one?

Code organization is not so important but imports at the top and guarding them should be done.

I also am not sure we want two ways to do it, so would we just require python-dmidecode to get DMI info?

Trying to control length/redundancy of the setup module, but perhaps it's not worth fighting.

Opinions welcome.

Contributor

dagwieers commented Apr 23, 2013

I'll look at it today. Prefer to see it merged at some point ;-)

Contributor

dagwieers commented Apr 24, 2013

Ok, found some time to improve the code. We now import at the top, but only conditionally (since we prefer the kernel DMI support over python-dmidecode and using subprocess/dmidecode as a last resort).

@dagwieers dagwieers Implement python-dmidecode/dmidecode as alternative for kernel DMI
This implementation falls back to python-dmidecode (RHEL5.5+) if the kernel as no DMI support. Alternatively, if python-dmidecode is missing, we attempt to use the dmidecode binary (for RHEL5.4 and older) before giving up.

This fixes #376 and #1657 and also helps @lwade on RHEL5.5+.
4135cff
Contributor

mpdehaan commented Apr 27, 2013

merged in and pushing shortly, thank you!

mpdehaan closed this Apr 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment