search

opengeospatial / ogcapi-routes

public repo for OGC API - Routes Standards Working Group
Other
10 stars 3 forks source link

Harmonization with OGC API Processes #27

Closed cportele closed 3 years ago

cportele commented 3 years ago

This pull request closes #15 and closes #17.

cportele commented 3 years ago

I cannot add you as a reviewer @jerstlouis, so I mention you in this comment: please have a look.

jerstlouis commented 3 years ago

Thanks @cportele I really have been struggling going back & forth in my mind about the need for this extra "value". Thinking that this will also be needed for a Maps extension allowing to specify custom inline styles object (profile of Processes, similar to Routes).

Then I read @fmigneault 's comments again and I think the most convincing arguments for the extra "value" (as opposed to trying to figure out a way to disembiguate pre-defined object schemas that can be substituted for it) is this comment, specifically:

Imagine JSON spec did the same, that they allowed you to write any schema, except you are not allowed to use a field named type because they use it internally to define their schema. Wouldn't it feel wrong?

The value adds that sub-object where we can safely tell the user: "do whatever you want in there, its your own schema".

and I think we took the right decision after all.

I had a look at the pull request and it looks good so far. After this is merged, I will have another go at updating the processes section in light of more recent changes to the process description (e.g. the switch from literalDataDomain/formats to JSON Schema for describing, and from id to key / values for, both inputs & outputs).

cportele commented 3 years ago

Thanks @jerstlouis.

For Routes the "value" and "inputs" wrappers do not add any value, they just add noise. This seems to be different for Processes, because of the resource design in that standard.

I still have doubts that it is the right decision to add the wrappers in Routes, too, because in general Routes and Processes are separate resources and there is a trivial mapping/conversion for those that want to implement Routing using a Processes backend, but I accept that we add them in order to move forward with both Routes and Processes.

jeffharrison commented 3 years ago

Please note - We will have this Issue as an Agenda Item for the SWG meeting.

jeffharrison commented 3 years ago

16 June 2021 SWG Meeting discussed this PR. There were comments and discussions as to whether this addition adds too much 'noise' to the API. Key points are that it does not affect Route Exchange Model (REM) and it also does not 'break' the API in the opinion of SWG members. Members requested a specific Use Case where this is used. Response was to support a client that supports only Routes API and a 'generic' client that support Processes API. Question brought up... Can this be added later? Answer is No. This is a 'Do now or never' decision. Observation made that we are not developing a Processes profile. Adding this elements closes an 'enormous gap' between Routes resource approach and Processes resource approach. Attendees noted the cost is small with respect to adding this, and there may be benefits.

Caveat 1 - Would it help if a note was added to explain why we have these elements? Yes. Very important to include this before or as we merge.

The Use Case is to allow both generic Processes client and Routes client to function with API.

NOTUC to Merging this PR.