search

DMTF / YANG-to-Redfish-Converter

This tool converts a YANG model file to the corresponding Redfish schema, specified in OData CSDL, in accordance with the YANG-to-CSDL Mapping Specification.
Other
4 stars 5 forks source link

Collection element not recognized by Travis #41

Closed jcleung5549 closed 5 years ago

jcleung5549 commented 5 years ago

The converter generates a element structures in the CSDL. However, element is not recognized by Travis.

    metadata/openconfig_lacp_orig_v1.xml
      ✗ is valid syntax
      Error: Unknown element name Collection
      at Schema.defaultElementParse (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\ParserCommon.js:87:11)
      at parseEntity (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\ParserCommon.js:26:11)
      at Schema.initEntity (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\ParserCommon.js:14:5)
      at new Schema (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\Schema.js:23:3)
      at Metadata.parseDataServiceElement (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\Metadata.js:129:25)
      at Object.parseEntity (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\ParserCommon.js:26:11)
      at Metadata.parseDataServices (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\Metadata.js:116:18)
      at Metadata.parseElement (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\Metadata.js:105:12)
      at Object.parseEntity (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\ParserCommon.js:26:11)
      at Metadata.parse (C:\Users\jcleung\Documents\Github external\branch-networktf\Redfish\node_modules\CSDLParser\lib\Metadata.js:57:18)
jcleung5549 commented 5 years ago

Travis expects the element to be within another element (like ). This is a problem for some of the offending elements.

(Currently, commenting out the elements in the CSDL to make Travis happy.)

tomasg2012 commented 5 years ago

With both this and #42, I believe that's how we decided to handle it, but with the CSDL tests for Travis modified to accommodate. Without this change, it will then have a problem with multiple of the same Annotation (i.e multiple RedfishYang.identity in the same object).

How should multiple of the same Annotation work? perhaps combine them if possible?

mraineri commented 5 years ago

The following syntax is likely not valid in CSDL terms (but it's hard to tell from the CSDL spec):

<Schema ...>
    <Collection>
    </Collection>
</Schema>

The <Collection> tags likely need to be within a given <Annotation>.

@jcleung5549 we might need to review the translation rules with the TF to see if we need to revise how this gets populated.

mraineri commented 5 years ago

For example:

<Schema ...>
    <Collection>
        <Record>
            <Annotation Term="RedfishYang.uses" String="interface-phys-holdtime-top"/>
        </Record>
        <Record>
            <Annotation Term="RedfishYang.uses" String="subinterfaces-top"/>
        </Record>
    </Collection>
</Schema>

might need to turn into this:

<Schema ...>
    <Annotation Term="RedfishYang.uses">
        <Collection>
            <String>interface-phys-holdtime-top</String>
            <String>subinterfaces-top</String>
        </Collection>
    </Annotation>
</Schema>
jcleung5549 commented 5 years ago

I think you are right. The YANG to Redfish mapping specification does not specify the mapping. We can agree at the next Network Infrastructure TF meeting.

jcleung5549 commented 5 years ago

The following YANG statements do not result in an element.

tomasg2012 commented 5 years ago

Since a Collection will not exist at the base of a Schema, the new approach would require creating Collection in a different form. Mike has pointed out that an Annotation with a specific name that can be defined more than once would require to be defined that way in the YangExtensions file.

<Term Name="revision" Type="Edm.String">
  <Annotation Term="OData.Description" String="The yang revision name."/>
</Term>

needs to evolve to

<Term Name="revision" Type="Collection(RedfishYang.revision_type)">
  <Annotation Term="OData.Description" String="The yang revision name."/>
</Term>
<ComplexType Name="revision_type">
  <Property Name="revision_date" Type="Edm.String"/>
  <Property Name="reference" Type="Edm.String"/>
</ComplexType>

or something similar, as suggested by Mike. Not sure if Property should be defined, perhaps there a way to make a more benign collection that can just follow a simpler annotation structure.

jcleung5549 commented 5 years ago

Adding the current CSDL structure (for the discussion purposes)

<Collection>
    <Record>
        <Annotation Term="RedfishYang.revision" String="2018-04-24">
            <Annotation Term="OData.Description" String="Clarified behavior of last-change state leaf."/>
            <Annotation Term="OData.LongDescription" String="Clarified behavior of last-change state leaf."/>
            <Annotation Term="RedfishYang.reference" String="2.3.1"/>
        </Annotation>
    </Record>
    <Record>
        <Annotation Term="RedfishYang.revision" String="2018-01-05">
                <Annotation Term="OData.Description" String="Add logical loopback to interface."/>
               <Annotation Term="OData.LongDescription" String="Add logical loopback to interface."/>
               <Annotation Term="RedfishYang.reference" String="2.3.0"/>
            </Annotation>
         </Record>
    <Record>
        <Annotation Term="RedfishYang.revision" String="2017-12-22">
            <Annotation Term="OData.Description" String="Add IPv4 proxy ARP configuration."/>
            <Annotation Term="OData.LongDescription" String="Add IPv4 proxy ARP configuration."/>
            <Annotation Term="RedfishYang.reference" String="2.2.0"/>
        </Annotation>
    </Record>
</Collection>
tomasg2012 commented 5 years ago

This is the resulting CSDL that Mike had suggested

<Annotation Term="RedfishYang.revision">
  <Collection>
    <Record>
      <PropertyValue Property="revision_date" String="2018-04-24"/>
      <PropertyValue Property="reference" String="1.0.1"/>
      <Annotation Term="OData.Description" String="Clarified order of ACL evaluation."/>
      <Annotation Term="OData.LongDescription" String="Clarified order of ACL evaluation."/>
    </Record>
    <Record>
...
    </Record>
  </Collection>
</Annotation>

Or a possible variation.

<Annotation Term="RedfishYang.revision">
  <Collection>
    <Record>
      <Annotation Term="RedfishYang.revision_date" String="2018-04-24"/>
      <Annotation Term="RedfishYang.reference" String="1.0.1"/>
      <Annotation Term="OData.Description" String="Clarified order of ACL evaluation."/>
      <Annotation Term="OData.LongDescription" String="Clarified order of ACL evaluation."/>
    </Record>
    <Record>
...
    </Record>
  </Collection>
</Annotation>
jcleung5549 commented 5 years ago

My preference would be for the second structure, since it uses the RedfishYang defintitions. Joe?

jcleung5549 commented 5 years ago

May 10 - TF agreed to use option 1.

tomasg2012 commented 5 years ago 
<Record>
    <Annotation Term="RedfishYang.revision" String="2018-04-24">
        <Annotation Term="OData.Description" String="Clarified order of ACL evaluation."/>
        <Annotation Term="OData.LongDescription" String="Clarified order of ACL evaluation."/>
        <PropertyValue Property="reference" String="1.0.1"/>
    </Annotation>
</Record>

Is how the current code in yang-2019 is expected to output a collection of revisions. This will likely have issue with the testing framework as RedfishYangExtensions requires an update for each property that might have expected tags embedded in them, and embedded annotations.

Additionally, in above comment (https://github.com/DMTF/YANG-to-Redfish-Converter/issues/41#issuecomment-489151867), these should no longer produce output.

mraineri commented 5 years ago

We'll need to update RedfishYangExtensions_v1.xml accordingly.

jcleung5549 commented 5 years ago

No longer seeing with YANG-2019. Waiting for a clean Travis run, before closing this issue.

jcleung5549 commented 5 years ago

Travis is no longer generating this error.