search

POETSII / Orchestrator

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

Dealing with attribute `appname` for 1.0.0-alpha v4 parser #199

Open m8pple opened 3 years ago

m8pple commented 3 years ago

When parsing the v4 file one-dev.xml from the PEP20 repository, it complains about an attribute appname which is supposed to be present.

  1. (  23,  9) : Element /mnt/e/dt10_all/POETS/poets_improvement_proposals/proposed/PIP-0020/xml/ic/tests/valid/L3-compilation/one-dev.xml.Graphs 
                  contains 0 instance(s) of attribute "appname": this (0) should be exactly 1

This attribute wasn't present in any of the v4 xml examples provided by either UoS or IC, and it's never appeared in the released version of the Orchestrator documentation that I am aware of.

Based on the 1.0.0-alpha documentation this seems to be there to provide the unique id for an application in the orchestrator, which I think previously came out of the GraphInstance/@id attribute in the development/classic branch. There now also seems to be support for multiple GraphInstance elements per Graphs element.

The v3 grammar allowed multiple GraphInstances, each with an id, but this was explicitly changed to be at most 1 GraphInstance per Graphs element, mostly to make parsing easier for the Orchestrator. (https://github.com/POETSII/poets_improvement_proposals/blob/2a355282f3aa3912bdacf4ec50bf25199bdf310d/proposed/PIP-0020/virtual-graph-schema-v4.rnc#L72, plus the change from * to ? in the grammar. )

Overall it doesn't really matter that much, except it is a backwards incompatible change because it is a required attribute, so all valid v4 XML will no longer be valid v4 according to the Orchestrator. This is not too much of a deal with XML coming from me/Johnny/Mahyar, but there are a number of students who have been writing v4 XML over the last two years, and I'm not sure that their XML generators still work. it's debatable how useful their apps still are, but their repos are still floating around, and at least one might come back to it.

So I see two ways of dealing with this:

  1. Make the appname attribute optional, and have it default to Graphs/GraphInstance/@id. This would just be an incremental change on the v4 schema, e.g. to v4.1
  2. Keep the required appname attribute and bump the xmlns version number to v5 to reflect that it is backwards incompatible.

Option 1 provides the most compatibility with existing v4 XML in the wild coming from IC, and the v4 XML provided by UoS in the v4 discussions. Option 2 is going to have no effect on the orchestrator, but requires someone from IC to change the graph_schema parser/emitter and regenerate all test xml.

So practically speaking option 2 is going to get us orchestrating faster, and can be mostly automated apart from the parser/emitter changes.

m8pple commented 3 years ago

A suggested fix is in 4c758cb3d4ba5232bb5418ae36333129b8570638 + 524341c1e2a94f5628c94ed2c485e88ba15f2c08

heliosfa commented 3 years ago

Make the appname attribute optional, and have it default to Graphs/GraphInstance/@id. This would just be an incremental change on the v4 schema, e.g. to v4.1

"OK, I can live with that"