fish
Guppy
Posts: 65
|
Post by fish on Jun 12, 2019 8:22:54 GMT
Hi,
Task property EndTime seems to have Nullable="false". This seems strange for an ongoing Task which should not have an EndTime yet.
EndTime property of Job has Nullable="true", which I assume should be similar application of EndTime. Is this a fault in the Task CSDL? I would propose to change to Nullable="true" also for EndTime in Task in such case.
(I think also that the word "last" is a bit confusing in the Description of StartTime and EndTime. How could a Task be restarted? I would assume that each execution of an asynchronous operation should result in creation of new Task resource. OK?)
BR
/Magnus
|
|
|
|
Post by mraineri on Jun 13, 2019 17:49:13 GMT
In the case where a Task is currently ongoing, I would expect "EndTime" to simply not be returned in the response payload. This is much like the case where you might have a DIMM not populated in a system, so properties like PartNumber, SerialNumber, and CapacityMiB are all not returned since the "State" of the resource is "Absent". Using null for the property value traditionally means that the property should be filled in by the service, but the service is unable to ascertain the value for the property.
|
|
|
|
Post by mraineri on Jun 13, 2019 17:50:13 GMT
And I agree; using "last" in those descriptions is odd. I think it would be best to remove that word since a given Task is not meant to be "restarted".
|
|
fish
Guppy
Posts: 65
|
Post by fish on Jun 20, 2019 9:04:50 GMT
In the case where a Task is currently ongoing, I would expect "EndTime" to simply not be returned in the response payload. This is much like the case where you might have a DIMM not populated in a system, so properties like PartNumber, SerialNumber, and CapacityMiB are all not returned since the "State" of the resource is "Absent". Using null for the property value traditionally means that the property should be filled in by the service, but the service is unable to ascertain the value for the property. Hi, Unfortunately I dont really understand your arguments here. Too me the case of EndTime for an onging Task would be typical case of when a service cannot acertain a "normal" value and should instead provide null for this property. I think this behavior would also be consistent with how the Redfish specification describes how to handle such cases (some quotes below). Specification 1.7.0 chapter 9.5 "Properties" -Properties not returned from a GET operation indicate that the property is not supported by the
implementation.
-If an implementation supports a property, it shall always provide a value for that property. If a
value is unknown, then the value of null is an acceptable value if supported by the schema
definition. Specification 1.6.1 chapter 7.5.6.5 "Required Properties" If an implementation supports a property, it shall always provide a value for that property. If a value is
unknown, then null is an acceptable values in most cases. Properties not returned from a GET operation
shall indicate that the property is not currently supported by the implementation.I think that your example with SerialNumber/PartNumber for not populated DIMM sounds also like typical case for exposing null in these properties. (I may have commented on this in similar forum discussion before also.) I would like you to add support for null in Task EndTime for these reasons, in order to allow Redfish implementations to adhere to such behavior if preferred. (I think adding null to the schema should be fully backward compatible change.) And it would be more consistent with EndTime of Job also. |
|
|
|
Post by jautor on Jun 21, 2019 0:30:46 GMT
Unfortunately I dont really understand your arguments here. Too me the case of EndTime for an onging Task would be typical case of when a service cannot acertain a "normal" value and should instead provide null for this property. I think this behavior would also be consistent with how the Redfish specification describes how to handle such cases (some quotes below). Specification 1.7.0 chapter 9.5 "Properties" -Properties not returned from a GET operation indicate that the property is not supported by the
implementation.
-If an implementation supports a property, it shall always provide a value for that property. If a
value is unknown, then the value of null is an acceptable value if supported by the schema
definition. That statement in the specification could use some clarification. The intent was to make two statements - first, if a property doesn't show up in a GET response, you should not expect a PATCH/POST operation containing that property to be supported. And second (more importantly), every property in the payload should have a "real" value - not something made up for properties you don't support (or aren't valid or useful in that resource instance). What we didn't want was for implementations to "fill out" every property with garbage data just because "it's in the specification". And since modern languages can deal with absent properties in a JSON payload very, very easily - you simply don't populate properties that don't make sense. No, we reserve the null value for error cases or other "temporarily unavailable" properties - and those should be exceptions. If there's no DIMM in a socket, no SerialNumber or PartNumber is supported. That's not an error condition - it's simply "not supported". If you receive a null value, the expectation is that you *will* get an actual value at some later time. EndTime is a case that could be argued (and was) for null, but as we wanted to stick with the error condition / temporarily unavailable, we chose to omit the property until it provides "valid data and meaning". And that usage in this case is clearly documented in the schema definition for that property. The rationale was also one to help avoid developer traps that cause interoperability problems - meaning someone populating EndTime with 00:00:00, some random time in the future, or other made-up data that would absolutely confuse client software. Hope that helps, Jeff |
|
fish
Guppy
Posts: 65
|
Post by fish on Jun 24, 2019 8:42:00 GMT
Im actually somewhat surprised by your opinion and argument on this subject. To me the specification text is really specific about this subject, and I think that the specification includes a good description of a good principle. And I get really worried if you are considering to change the specification in some significant way here. I think also that many service implementations that use typical software libraries for json response serialization will want to keep such behavior where a resource will always return the same set of properties, as this should simplify implementation using the serializer. (Only that some of the properties are set to value null, when temporarily unavailable.)
Unfortunately I think that your proposed handling actually violates this part of the specification: "Properties not returned from a GET operation indicate that the property is not supported by the implementation.". If a service does not at all return EndTime for an ongoing Task, the client should then expect that the service will never respond with EndTime, even for completed Task. And I assume that this is not your intention. So such behavior will likely cause more problems for clients than returning null value in EndTime for ongoing Tasks.
Also I think that not having possibility to provide null in EndTime property is what will more likely force developer to provide strange EndTime values such as 00:00:00 or similar, as they should also want to comply to the other parts of the specification ("If an implementation supports a property, it shall always provide a value for that property."). Im not saying that properties that an implementation/service does not support should be populated with null values. Such not supported properties should never be exposed by the service even if they are in the schema. But in this case of EndTime we will want to expose it with with proper value for completes Tasks so we would need to expose it also for ongoing Tasks. (As for Jobs.)
I still think the EndTime for ongoing task and serialNumber for removed DIMM is typical scenario for "temporary unavailable" as you describe, and for this reason null value should be very suitable in these cases.
I really appreciate that you have provided null option for other properties where needed in all other properties in Redfish schemas where I have seen such need. So I hope that you will continue this in the future also.
I would once again want to urge you to add null option for EndTime in Task for previously stated reasons, as this should most likely not even be of any disadvantage for implementations that dont want to use it. But it would be of great help for us that think we need it for reasons of consistency and actually also for reasons of being compliant to the current specification. Otherwise I think that we may need to consider to add an OEM defined property in Task for EndTime including null option, which would be a bit odd.
I would also really like to hear opinions from other forum members on this subject.
|
|
