Fature/apply review fixes #388
Conversation
Update recommendations and add Requirements in the corresponding Requirements class Make CWL depending on OGC Application Package for not having to add another conformance class such as Define the Requirement for w param in DRU directly to make it easier to extent, cf. opengeospatial#386 Move workflow-not-found exception Requirement to DRU Requirements class
Remove uneeded media type information in recommendations
There was a problem hiding this comment.
This should be removed from the core since it seems to a be a CWL-specific thing. The core should be nice and simple. The POST body contains the definition of a single process to be deployed. Period.
Also, the name w, dervied from "workflow" I suspect, is also problematic. All workflow-related concepts should be restricted to Part 3.
There was a problem hiding this comment.
In 3105532, the Requirement has been moved from the DRU to the OGC Application Package Requirements class.
- /req/deploy-replace-undeploy/deploy/w-param -> /req/ogcapppkg/deploy/w-param
As mentioned here about the Requirement /deploy/w-param for another Workflow Language:
Might be relevant to refer to a common parameter that can be reused across Workflow languages regardless of their specific implementation.
I agree with this. It will ease the integration of more Workflow Languages in the future.
So, the CWL Requirements class refers to the Requirement /req/ogcapppkg/deploy/w-param for the Deploy operation.
In the OGC Application Package schema, the executionUnit property is defined as a oneOf where array can be an option.
There was a problem hiding this comment.
Hmm ... it might be something about CWL that I don't understand but I think this should be remove from the standard altogether. The POST body for a deploy operation should contain the definition of a single process and that is it. Nice and simple. This means that a CWL body should contain a single process definition (or "workflow" in CWL speak I guess).
However, as I said, this might be an effect of something about CWL that I don't understand. If that is the case then please exaplain.
There was a problem hiding this comment.
In CWL, we can identify a process as an instance of the workflow class. This was clarified in 3105532.
A CWL file can contain multiple workflow definitions. The w parameter permits the client to select the one to deploy as a process from the Processes Part 2 implementation.
As illustrated in PR #386, it is a concept shared among Workflow Languages that can be used to deploy processes.
There was a problem hiding this comment.
In CWL, the contents can be defined under $graph which contains a list of multiple CWL "applications".
Each of those "applications" can themselves be sub-workflows or atomic command line tools that can be combined into a main workflow. The w parameter can help disambiguate which one of IDs is the targeted tool for deployment as OGC API Process.
There was a problem hiding this comment.
As discussed during the SWG meeting on January 8th, 2024, we moved the requirements w-param and exception-workflow-not-found back to the CWL Requirements class. They are not referenced from anywhere else.
…oy/deploy/w-param Requirement Move the Requirement deploy/w-param to the OGC Application Package Requirementss class to make it reusable from other Requirements classes, such as CWL
Modifications were required after the review was sent to the SWG. This PR should solve most reported issues and clarifications.
Changed from recommendation to Requirement, updated original recommendation:
Split recommendation into Requirement and recommendation:
Update Requirement references from the ATS consequently.