openEHR / specifications-ITS-REST

openEHR REST API Specifications
https://specifications.openehr.org/releases/ITS-REST/latest
Apache License 2.0
18 stars 15 forks source link

Remove version support for template endpoint for adl 1.4 #82

Closed serefarikan closed 5 years ago

serefarikan commented 5 years ago

The post operation to upload a new opt to the server has an optional version parameter.

In the context of adl 1.4 based implementations, this parameter causes a number of issues.

but that is it. In Ocean's implementation, whatever the opt with that template id is, it will be used for validation. I suspect it may be the same for other vendors for adl 1.4 implementations and deployments.

Based on the arguments above, I cannot see any benefit for the version parameter here. I'd like to hear the original motivation/thinking for introducing this parameter.

freshehr commented 5 years ago

So our policy in freshehr was to embed a major version number in the templateid and bubble up any version changes in undeying archetypes. Versioning should be formalised for templates and actually be in the. Opt

freshehr commented 5 years ago

However I would also like to explore distributing a maven or npm type manifest alongside templates as an alternative to embedding the dependencies directly. I'm not too bothered about the current additional version parameter as long as it is optional. It may help some implementers but it's probably not a good long term approach.

serefarikan commented 5 years ago

@freshehr I cannot see the use for the additional version parameter. From an implementer point of view, I need to know how that parameter affects behaviour. Why would a rest client add it? What should they expect from the server? There is a semantic gap here in regards to what versioning of opts supposed to do. I am therefore bothered with an implementer's hat on :)

sebastian-iancu commented 5 years ago

For .opt: we also include a manually assigned version number in the .opt template name (a.k.a "BloodPressure v1"), but we made an extra extension on that template_id to allow a uid being appended to the name, uid which internally is liked to a specific version. If I'm not mistaken, there is no formal definition of TEMPLATE_ID in specs.

Back to your question, if I remember correctly, the reasoning was that: We should allow an upload to indicate (and/or overrule) specific version of a templates; it is perhaps only a label associated with content, but it could be just a placeholder for an implementation feature. Why we need it? Think about what will happen if you upload again a template with same name - should you forbid that? or just completely overwrite previous template? what if previous template is already in use? etc. When you get a template only by id (name) than you get always the 'latest', unless you specify one of the desired version.

sebastian-iancu commented 5 years ago

support for {version] on definitions/template/adl1.4 is removed - specs needs to be reworded/redesigned there (see slack discussions)

bjornna commented 5 years ago

I don't see why we should remove the optional semver {version} parameter. Some systems might not support versioning on templates. In those systems the semver version will always be 1.0.0. Other systems might support content based versioning on templates. Those systems will benefit from this parameter.

Some running openEHR backends support uploading of new templates. Sometimes the changes are major and other times only minor or patch. The openEHR REST API should be able to support this pattern and leave it up to the system owner if he/she wants to support this kind of versioning.

In our system we use NuGet packages to carry additional metadata for each definition (artifact). Metadata is a) the semver version and b) the semver based transitive dependencies. Based on this we want to be able to know excactly which file version of a template is used in a specific system.

In sum: I am not in favour of removing the optional semver {version} paramter.

wolandscat commented 5 years ago

@bjornna I think the argument is that if versioning is properly supported by templates (as it is for example in ADL 2 templates), the version id will always be in the template content or template id, and will never have to be supplied separately.

freshehr commented 5 years ago

The further argument is that while we do need to solve this problem, passing a version parameter via the API is probably the worst approach, and we don't want to encourage people down this road if it needs unpicked soon after by a better solution. However if an implementer is already using this approach, I am happy to leave it in.

serefarikan commented 5 years ago

@bjornna This is not about supporting versioning or not. My point is that this is a bad way of supporting versioning. Technically this is similar to sending the archetype id of a composition along with it as a parameter instead of putting it in the composition. Versions of templates should be specified by modelling tools and processed by back ends. Allowing the app developer to set the version leads to an error prone methodology.

bjornna commented 5 years ago

In the initial design of the REST API we ended up with this optional parameter since several vendors already supported upload of the same template id multiple times. None of them had any versioning, just simple replacement. The SEC group wanted to make a proposal that was a fraction better by providing the optional possibility for clients (beeing a tool or an application)to define the versioning.

Our company is currently building a generic definition store for all the artifacts a running application might need. The storage is content agnostic. We dont't want to add a parser for any kind of content, since it would not scale. For this service there will be clients (might be a tool or a server) who is responsible for their content type. The clients will be responsible for housekeeping the versioning.

I see that you all have the opinion that the endpoints should be able to parse and analyze the content and based on this process be able to define the correct version for the uploaded artifact.

If that's the expected feature/functionality for an openEHR REST API I will not argue any more on this topic.

wolandscat commented 5 years ago

@bjornna I think it's probably the best route for now i.e. to remove the optional version in that particular function. If we really do decide later that it is the best way to provide such information, then we can add it in, but for now, it doesn't correspond to how archetypes and templates work, so I think it is judicious to get rid of it now.

freshehr commented 5 years ago

@bjornna - I think you are on exactly the correct approach with the idea of developing some sort of pan-artefact versioning/dependency mgt. system. Any mature deployment is going to have multiple dependencies - archetypes, templates, FHIR profiles, terminology subsets/valuesets, mapping artefacts.

This is, of course, an extension of the Release Sets idea, but I would love to explore how we might make use if industry-standard approaches like NPM, Maven, NuGet as many of the really tricky issues (required/preferred versions, clashes) have largely been resolved (or at least as far as possible). I can absolutely see us adding back a parameter which will take a package.json payload (or something similar) but I feel some work and thinking needs to be done first.