Version check for deprecated

[aheynig]

From the schema PowerDistribution_v1.xml, the property Version in

              <Record>
                <PropertyValue Property="Kind" EnumMember="Redfish.RevisionKind/Deprecated"/>
                <PropertyValue Property="Version" String="v1_3_0"/>
                <PropertyValue Property="Description" String="This property has been deprecated in favor of the PowerSupplyRedundancy property in the Chassis resource."/>
              </Record>

does not seem to be used to check if the actual reported version is affected.

I assume the version would have to be read and compared in redfish_service_validator/validateRedfish.py:

    if prop.Type.Revisions is not None:
        for tag_item in prop.Type.Revisions:
            revision_tag = tag_item.find('PropertyValue', attrs={ 'EnumMember': 'Redfish.RevisionKind/Deprecated', 'Property': 'Kind'})
            if revision_tag and not prop.Type.IsMandatory:
                desc_tag = tag_item.find('PropertyValue', attrs={'Property': 'Description'})
                deprecatedPass = False
                counts['warnDeprecated'] += 1
                if desc_tag:
                    my_logger.warning('{}: The given property is deprecated: {}'.format(prop_name, desc_tag.attrs.get('String', '')))
                else:
                    my_logger.warning('{}: The given property is deprecated'.format(prop_name))

[jautor]

Do you mean that your system is using/reporting PowerDistribution.v1_2_0 (or any version prior to v1.3), and that property was deprecated in v1.3?

Yes, that is the expected behavior, because this is just a "warning", not a failure, so we want to report those properties that were deprecated in newer versions of schema.

[mraineri]

@aheynig do you have an example payload and report to better understand what you're highlighting? As @jautor is pointing out, the tool is intended to report warnings when properties are deprecated.

[mraineri]

And the version it's deprecated is more for documentation purposes; even if your resource version is older, we still want to highlight that a deprecated property is in use to encourage implementers to update their service to the newer method. It is just a warning though, so it doesn't indicate non-conformance.

[aheynig]

To be alerted to deprecated properties on manual review, the current behavior is perfect.

However, when automatically evaluating the csv files, it is not possible to get a single result that means "complete test passed and no deprecated properties used" when "Deprecated" can mean that the property is deprecated in the currently used/reported version, or will be deprecated in future versions. An update of the schema files will then lead to a change of the test result.

For this, it would be very helpful to be able to differentiate between these two types of "Deprecated".

[mraineri]

I'm not sure I understand the distinction; if a property is deprecated, it's deprecated. The version doesn't matter since the guidance from the Redfish Forum is to use the newer method.

[jautor]

I thought we had a config option to suppress the warnings on deprecations so as not to alarm users that are strictly checking for conformance (as opposed to development) - but I don't see that and it appears that was a move to make those "warnings" instead of "errors".

Suggest we add a config flag to "warn on deprecated properties" to better align with CI strict checking. Because regardless of schema versions, implementations will eventually encounter deprecated properties for backwards compatibility purposes...

[aheynig]

This is an example scenario: A project has an automatic test based on the Service Validator. This test can fail or succeed. As long as the test is successful, no human looks at the detailed results and logs.

The test has two ways to handle "Deprecated" in the csv:

1) The test fails

  Only PowerDistribution.v1_2_0 is offered in the project for a certain period of time. As long as the schema file has version v1_2_0, the test is successful.

  Now the schema files are updated, trusting that they are backward compatible. The test now fails.

2) The test succeeds

  There is a change to offer PowerDistribution.v1_3_0 instead of PowerDistribution.v1_2_0. The schema files are also updated. However, a depracated property has not been removed.

  The test is successful. This is bad.

I think it should be possible to differentiate in the csv whether the deprecated warning affects the used/reported version (and has always affected it) - or was added later (and did not generate a deprecated warning before) and may be ignored due to backwards compatibility.

[jautor]

If you're doing some processing on the CSV - what if we added the "deprecated version" as a column (populated only for deprecated properties). You could compare version>=deprecated version to filter those out. Just thinking about ways to present this that may be more useful for a broader user base...

But, how will you handle deprecated properties that will remain (for compatibility) in future versions? For example, IndicatorLED is a commonly used, but deprecated, property. Comparing schema versions won't filter that out - someone would have to say "we expect that one to be present". Do you handle that as part of your tool (exception list, etc.)?

[aheynig]

Thank you for this great suggestion! This additional information in the CSV would help me a lot, I could easily compare it with the reported version number I find in the same CSV line.

For deprecated properties that should be preserved permanently, I would then create exception rules in the CSV processing tool. I think this is a good and clear solution. This CSV processing tool already uses rule lists to ensure that specific "optional" properties are present. And also rules to check values.

[mraineri]

PR merged