Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc Generator: PropertyRequirements for Status property don't appear to work #271

Closed
barbara9 opened this issue Feb 5, 2020 · 11 comments
Closed

Comments

@barbara9
Copy link

barbara9 commented Feb 5, 2020

I am trying to get the doc generator to list the properties within the Status object in my document. I am using the same code as in the sample input. I have similar issues with other properties that are objects. EthernetInterface.IPv4Addresses, EthernetInterface.IPv6Addresses, where I just want to document the Address property.

"Status": {
"PropertyRequirements": {
"Health": {},
"State": {}
}
},

@barbara9
Copy link
Author

barbara9 commented Feb 5, 2020

What shows up in the html for the above is:

Status { } | object |   | The status and health of the Resource and its subordinate or dependent Resources.See the Resource schema for details on this property.

@akf
Copy link
Contributor

akf commented Feb 5, 2020

I've confirmed that this is what happens when I run the doc generator against the json-schema files in the Redfish repo, if I don't specify that I want "common objects" pulled into a single section. (If I do specify "common objects," things like Status are output in one section and you'd get a line similar to what you're seeing, but with a link to "the Status object" in the same document.

At this moment I'm fuzzy on whether what's happening now is a bug, but there is a workaround to force objects like this to be output "in place." If you use a config file, you can include an object_reference_disposition entry. In your case, you would specify the objects that you want fully expanded where they appear in other objects in the "include" list, and leave the "common_object" list empty.

"object_reference_disposition": {
    "common_object": [
    ],
    "include": [
        "http://redfish.dmtf.org/schemas/v1/PCIeDevice.json#/definitions/PCIeInterface",
        "http://redfish.dmtf.org/schemas/v1/Resource.json#/definitions/Status"
    ]
}

@akf
Copy link
Contributor

akf commented Feb 5, 2020

See https://github.com/DMTF/Redfish-Tools/blob/master/doc-generator/README_config_files.md for details on config files, if you're not already using that option.

@barbara9
Copy link
Author

barbara9 commented Feb 6, 2020

Yes, I am using a config.json file. Adding the above worked for expanding Status. However, now I have the problem where all the properties in the Status object are included. The information in the document json file listing PropertyRequirements, referenced above" doesn't work to only list certain properties. I even tried adding properties to the config.json file as excluded (see below), and that didn't work either.

"excluded_properties": [
"Oem",
"HealthRollup"
]

@barbara9
Copy link
Author

barbara9 commented Mar 6, 2020

This issue seems to have stalled. Are you waiting for more information from me?

@akf
Copy link
Contributor

akf commented Mar 6, 2020

It has stalled, I apologize. I need to figure out whether this is somehow intended behavior, but it's more that I've been busy with other things.

I have to puzzle a little for good test cases, and in that you could help me:
It would help to have copies of your config file and supplemental markdown file, and the command line arguments you're using. Also, are you working against the json-schema files in the Redfish repo, or are there additional files?

@barbara9
Copy link
Author

barbara9 commented Mar 6, 2020

No problem. I was busy doing other stuff too ;-). Wanted to be sure you weren't waiting on me for something. config and markdown file attached, along with the generated html. I'm using the straight Redfish schema, no additional files. The doc generator and the schema are up one directory level from the config and markdown files.
RedfishTranslationLayer.zip

The command line I used to generate the html is:
python ..\doc-generator\doc_generator.py --config .\config.json

@barbara9
Copy link
Author

Any update on this issue? Do you need additional information or files from me?

@akf
Copy link
Contributor

akf commented Sep 1, 2020

@jautor this is the issue I wanted to loop you in on, and potentially include in the next batch of work on the doc generator.

I finally took a bit of time today to poke at this one, and I don't have a full understanding (it's complicated), but I think that I simply didn't account for the combination of expanding-in-place an object from a different schema, and applying a profile to get a subset of the object, at the same time. @barbara9 wants to include "Status," "IPv4Addresses," and "IPv6Addresses" in a subset, and she wants to specify a limited set of properties in each of those objects. The zip file she attached to this issue includes a profile, "RedfishTranslationLayer.json."

I can get the output document to include only a subset of properties on some of the instances of "Status", if I specify that subset as part of the profile under the "Resource" schema -- so, defining the subset based on where the property is defined rather than where it is used. (When "Status" is nested a couple of levels deep, this doesn't work ... so there's something else awry there.)

I believe what we would want is to be able to specify a set of property requirements for "Status" in each place where "Status" is included though -- as in the RedfishTranslationLayer.json subset, which calls out different properties for Status in different places. Right? So the subset should be applied based on where the object is being output in the document, rather than where it came from.

The IP Address objects seem to be an example of a different problem as well. I suspect it may have to do with the fact that those objects are within lists of items.

@jautor
Copy link
Member

jautor commented Sep 2, 2020

Yes, we should always evaluate the inline inclusion based on the profile for the current (including) schema - as "where I copied the object from" will have no impact on its usage. It would be a mistake for someone to make a profile requirement on the 'Resource" or "IPAddresses" schemas, as they're supporting files that don't map to an actual resource in the tree. That may be something we need to add to the Redfish Interoperability Profile Specification - to state that you can't make requirements on non-resource schemas (it's not a long list). For clarity in the doc-generator, perhaps we should have it flag/ignore/error on a profile that makes requirements on Resource, IPAddresses, Redundancy or Settings (there may be a few others but those seem unlikely.

A couple of ways to solve this:

  • rely on the include/common_object settings to get the $ref objects expanded inline. This will require profile writers to know that detail so that their requirements aren't missing. I think we can document that, but i'd prefer option 2:
  • Always expand a $ref object inline if there is a profile mention for that resource. This has the benefits of being both 'automatic', and probably also helps profile authors to clarify their intentions. Marking an object "Mandatory" with no further property requirements would result in an "empty object requirement" - certainly not what the profile author intended.

akf added a commit to lucidmeetings/Redfish-Tools that referenced this issue Oct 29, 2020
* Ensure profile/subset requirements are extracted from the input profile where an object is used, not where it is defined. (For example, "Status" is defined in the "Resource" schema, but it should be included in a profile within the object in which it is referenced.)
* Always expand a $ref inline if there is a profile mention for that resource.
* Warn when a profile specifies requirements on the IPAddresses, Redundancy, Resource, or Settings schema.
@akf akf closed this as completed in 146bb37 Oct 29, 2020
@akf
Copy link
Contributor

akf commented Oct 29, 2020

@barbara9 at long last, here are some updates that address your issue. I've been testing with a subset of the profile example you provided. If you have a chance to pull the latest code from this branch and try again, this would be a great time to identify whether this really solves the issue for you.

@akf akf reopened this Oct 29, 2020
@akf akf closed this as completed in 65f2e73 Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants