DMTF / Redfish-Tacklebox

Python based utilities for performing common management operations with Redfish
Other
51 stars 25 forks source link

enhancement sensor list #122

Closed PeterKchen2 closed 1 year ago

PeterKchen2 commented 1 year ago
  1. Older power/thermal models's Redundancy name style should same with the Newler

  2. if Name exist use : as sensor name else as sensor name

mraineri commented 1 year ago

I'll need to run this against a few systems; I have a bit of concern that showing the Id property and the Name property will produce a lot of repetition in some places. I like the idea though, but we might need to be careful about when we concatenate the properties into a single field.

PeterKchen2 commented 1 year ago

any update about this issue??

mraineri commented 1 year ago

Comparing the before and afters with a few different services I have some concerns about some of the name changes.

Some examples:

And the list goes on like this...

Today, the tool does rely on the "Name" property to be intelligible in responses. Best practice from Redfish is to use "Name" as something human friendly to describe the resource. So, a name like "Sensor" really doesn't help the user. Are you able to give an example of what you're seeing on the system you have? Maybe we need to have a control flag to specify a naming mode for sensors in case the service isn't using "Name" properly.

PeterKchen2 commented 1 year ago

I used redfish server is implment like this Id is number string (for example: 1), and name is name for example: "SYS FAN 1"

PeterKchen2 commented 1 year ago

so Id maybe is not a number string, maybe is a name ???

PeterKchen2 commented 1 year ago

is Id only required is The unique identifier string, not required is the number string or name

mraineri commented 1 year ago

"Id" is a unique identifier. It's always a string, but could just be a number, a GUID, a camel-cased identifier... It really depends on the service's design for how they track their identifiers internally. I've seen several cases where "Id" is almost like "Name", but special characters are removed to meet Redfish Spec requirements with URI rules and other constructs.

"Name" is the name of the resource and is intended to be human readable. If you're seeing "SYS FAN 1" as the "Name", that's pretty reasonable to me already.

PeterKchen2 commented 1 year ago

ok, I should show with Name if Name exist, else use Id, thank you

PeterKchen2 commented 1 year ago

I commit this

mraineri commented 1 year ago

My only nit so far is I think it would read better if there's a space after the : characters (I'll ask others to see if they agree with me). Other than that I think this is much better; I'll run it against some systems to confirm.

PeterKchen2 commented 1 year ago

any update about this issue

mraineri commented 1 year ago

I've been going back and forth about this with some others. Unfortunately I'm not seeing good use of "Name" in many cases outside of Sensor resources. Some services do a good job at making "Name" useful for people for a tool like this, and others don't. I may need to bring this up internally to see if there's any guidance we can give on this topic, because existing documentation from DMTF is pretty open-ended.

Prepending ":" to "Name" looks a bit redundant too; if "Name" is a "good" value, it should be self explanatory already.

One of the thoughts that came up during our call was we could introduce a control flag to affect how the name is constructed at the user's request. Essentially:

def get_sensors( context, use_id = False ):

And later, have code blocks that look something like this:

if use_id:
    power_supply_name = "Power Supply " + power_supply.dict["Id"]
else:
    power_supply_name = power_supply.dict["Name"]
PeterKchen2 commented 1 year ago

discide to show sensors's name using "Id" or "Name" by option

mraineri commented 1 year ago

Looks good; approved