-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Part 3 Deployable Workflows - Analysis and Proposals #431
Comments
Thanks for looking into this @fmigneault . In general, I think your understanding how this is proposed to work is correct. I was a bit confused at first when you talked about 2 different variants, but yes the intent was that:
I thought we already had an
This is required in the process description. Potentially if it is not there the server could automatically version it...
Which additional metadata are we missing at the input / output level?
Is that because
Only the derived fields / field selector modifiers (
I am confused. The mention about using No objection to using
The exec req suggests we could align this with the media type for POST to |
For what it's worth ... I prefer proposal 2 since it makes a workflow just another execution unit. Not special or different from any other execution unit. I am not sure I understand all the contortions about To that I would say that we make "schema" optional in the process description. If the schema of the inputs/outputs can be inferred from the execution unit then you don't need to include a |
That wouldn't work because the
In case the
Yes, that's the reason, ie:
If this is the preference over explicit ogcapi-processes/openapi/schemas/processes-core/schema.yaml Lines 3 to 4 in b972e74
Yes (field selectors), and no (not clear/easy). For example, if the Since field modifiers can completely redefine the output however they want, it is not that trivial to infer the resulting
Your understanding is correct. The only issue about ONLY using Omitting the qualified value representation with
I missed this type when reading the draft. It is acceptable as well. However, I believe |
The advantage (and main purpose) of Reusing
That would be a valid alternative, as long as it is only in the context of
This is valid as well. |
The Part 3: Deployable Workflows proposes an alternate deployment definition based on an execution body, trying to bridge Part 1/2/3.
I would like to validate my understanding of it, and propose adjustments to improve alignments (as applicable).
Since there are 2 variants for deployment, the 2 are analyzed separately, but using an equivalent workflow example.
Variant 1: Direct Deployment with Execution Body
Analysis
A new process named
DeployWorkflow
with inputwf-input
and outputwf-output
would be created.The
schema
definition ofwf-input
would be the same as the one ofarg
fromNestedProcess
, whereas theschema
of thewf-output
would be equivalent toout
ofMainProcess
.Proposals
Add an
id
field, which is not present inprocesses-workflows/execute-workflows.yaml
.id
was missing considering it is not required for execution only.However, some process ID is needed to perform the deployment.
Alternatively to
id
, reuse?w=<id>
query (https://github.com/opengeospatial/ogcapi-processes/blob/master/openapi/parameters/processes-dru/w-param.yaml)processes-workflows/execute-workflows.yaml
definition without any modification. However, debatable whether it is intuitive or not.Other required parameter
version
from Part 1 needs to be added.Since there is no equivalent query parameter, so it might be better to have the Part 3: Deployable Workflows schema be a
oneOf[ process-core/processSummary, processes-workflows/execute-workflows ]
This should be added to OpenAPI path
/processes
.Introduce
application/ogc-workflow+json
(or some equivalent) to distinguish from other deployment structures already supported (CWL, OGC App Pkg, etc.).This variant doesn't indicate how additional metadata for the resolved
wf-input
andwf-output
can be defined.Recommendations to had to the document, either:
arg
/out
defined, nothing more, nothing less.$input
/$output
to extend/override whatarg
/out
provide.Variant 2: Embedded Deployment of Execution Body in Execution Unit
Analysis
Proposals
Because
wf-input
/arg
andwf-output
/out
schemas should be aligned to be mapped correctly, redefininginputs
andoutputs
with schemas explicitly inprocessDescription
is redundant. However, this would not be disallowed according toprocesses-core/process.yaml
.Recommendations should be given in the standard document about this case.
More specifically,
processDescription.inputs
andprocessDescription.outputs
could be relevant to provide additional details, such asprocess-core/descriptionType.yaml
metadata properties.However, adding any
inputs
/outputs
there would fail validation if theschema
is omitted, since it is required in their definitions. Because of this, we end up going back to redundantschema
definitions mentioned above.Possible recommendations:
Use
And indicate that
schema
should be inferred by$input
in this deployment use case.Recommend to explicitly reference the schema:
If Part 3: Fields Modifiers are thrown in the mix of Deployable Workflows, notably for the
wf-input
andwf-output
, then theschema
mapping betweenwf-input
/arg
andwf-output
/out
could actually differ entirely.In this case, contrary to previous (1),
schema
underprocessDescription.inputs
andprocessDescription.outputs
could become mandatory. This is because, without any reference schema fromDeployWorkflow
(yet to be deployed), the workflow could be validated if they were omitted, since there would be no indication of the intended source and desired result for field-modifedwf-input
/wf-output
.Improve the description of Part 3: Deployable Workflows regarding media-type.
The requirement mentions using
application/ogcapppkg+json
, but this can easily be confused with the case whereprocesses-dru/executionUnit.yaml
is employed directly.When an embedded execution unit definition is used, it is preferable to employ the qualified value with
application/ogc-workflow+json
to avoid ambiguity about the package contents (or use Variant 1 directly instead).The text was updated successfully, but these errors were encountered: