-
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
The execution unit schema is really confusing! #397
Comments
I think the intention was to follow the "by-reference"
This looks like it takes inspiration from docker compose services:
ndvi:
container_name: nvdvi
image: mydocker/ndvi:latest
environment:
FORWARDED_ALLOW_IPS: "*" I agree with the use of |
@fmigneault I still think there should be a consistent way to, at least, identify the type of execution unit. Right now we have three members for this: "type" (as a plain token), "type" as a media type, and "mediaType" as a mediaType. I would prefer that we have "type" as an enumerated token to identify type (each execution unit conformance class would expand the enumeration). Then the rest of the members of the Docker execution unit:
CWL by reference:
CWL inline
|
SWG meeting from April 29th: Check if there was any decision made in March. |
During the SWG meeting from March 4th, this was also discussed. It was agreed that Peter will create a PR for the SWG to review. You can find the recording here: (OGC member only) https://portal.ogc.org/files/?artifact_id=108046 Interesting part is around minute 32. |
@pvretano Also, the oneOf:
- allOf:
- type: object
properties:
type:
enum:
- docker
- oci
- oneOf:
- type: object
properties:
value:
$ref: executionUnit.yml # with the 'type' property removed
additionalProperties: false
- type: object
properties:
link:
$ref: link.yml
additionalProperties: false
- allOf:
- type: object
properties:
type:
enum:
- cwl
- oneOf:
- type: object
properties:
value:
$ref: "https://raw.githubusercontent.com/common-workflow-language/cwl-v1.2/main/json-schema/cwl.yaml"
additionalProperties: false
- type: object
properties:
link:
$ref: link.yml
additionalProperties: false
- allOf:
- type: object
properties:
type:
# if we want to be strict, here we should also include a 'not' of all other 'type'
type: string
- oneOf:
- type: object
properties:
value:
type: object
additionalProperties: true
- type: object
properties:
link:
$ref: link.yml
additionalProperties: false |
I have been reviewing the Part 2 and the schema for the execution unit is really disjoint and confusing.
If the execution unit is a container then the execution unit looks like this:
If, however, the execution unit is CWL, the execution unit looks like this:
or like this depending on whether the CWL is inline or referenced:
Why such varying schemas? Why, for example, does the CWL class have both "type" and "mediaType" when the examples are both media types? Why not have a single member,
type
, of type string and simply have a requirement in the CWL conformance class that it's value has to be a media type?Also, why does the docker conformance class have a member
image
? Why not call itvalue
to be consistent and simply include a requirement is the docker conformance class that the value ofvalue
is a Docker/OCI image reference? The dockerconformance class could further extend a "base" schema to add the
deployment
andconfig
members.If, for example, the base schema of the execution unit was:
The docker conformance class could extend this schema like this:
And the CWL conformance class could extend this schema like this:
... and, of course, a vendor-specific execution unit such as:
is also schema valid allowing experimention to flourish without too much trouble and perhaps lead to additional execution unit conformance classes.
The text was updated successfully, but these errors were encountered: