search

POETSII / Orchestrator

The Orchestrator is the configuration and run-time management system for POETS platforms.
1 stars 1 forks source link

Changed spelling of Metadata plus extended locations #222

Closed m8pple closed 3 years ago

m8pple commented 3 years ago

In the v4 spec the element name is Metadata, and that was used in the example xml given to Andrew. But in the orchestrator it is given as MetaData. Depending on how much xml is floating around with the MetaData form, we could just change the spec to say that either form is correct?

I also notice the grammar allows the meta-data to be attached to lots of different elements, which was true of v3 and earlier, but in the v4 discussion it was required that it only be attached to GraphType or GraphInstance, and nowhere else.

I'm happy to open it up, as it is a non-breaking change, and I didn't really get the reasoning for restricting it in the first place.

m8pple commented 3 years ago

An actual bug is that in the v4 grammar the elements have to be in a strict order, and it was specified the MetaData comes before Properties:

https://github.com/POETSII/poets_improvement_proposals/blob/2a355282f3aa3912bdacf4ec50bf25199bdf310d/proposed/PIP-0020/virtual-graph-schema-v4.rnc#L238-L255

In the orchestrator grammar they are switched:

https://github.com/POETSII/Orchestrator/blob/b5615c33a5476c4ed1d1170cbc1abaa4201ad007/Config/V4Grammar3.ocfg#L4-L6

m8pple commented 3 years ago

The orchestrator grammar also doesn't include the key and value attributes, so rather than:

<Metadata Metadata="[]"/>

it should be:

<Metadata Metadata="[],[1](key,value)"/>
m8pple commented 3 years ago

Fixed by this commit: 74e27478dee3e7031cda1c608e8abb1080a83756

heliosfa commented 3 years ago

We are happy to revert to the specified case ("Metadata") as we don't currently do anything with it.

m8pple commented 3 years ago

I've rebased the proposed patch from last month into #264 , as it is still a blocking issue.

heliosfa commented 3 years ago

Resolved in #264