Open uniqueg opened 2 years ago
If we assume that a workflow can exist in multiple languages (which it arguably can, and we even have (toy/test) examples for this), then surely it could exist in multiple versions of the same language
Hmmm, this might be something we didn't consider. I understand/understood a workflow changing between DSL1
and DSL2
with newer versions. But we probably didn't anticipate a workflow supporting both in one {version_id}
in other words. If we were to add this, it would be nice for it to be optional.
using just content negotation
I think this is a good change to propose. If I recall correctly, I think some of this mess was just because we couldn't figure out how to represent some things in swagger 2.0 / associated code generator and never got removed. Now that we're on OpenAPI 3.0, it makes sense to me to clean this up especially if we are considering doing a next major revision.
Currently, multiple endpoints make use of the
{type}
path param to specify the descriptor type of a workflow resource (e.g.,CWL
,WDL
). As we are currently implementing a WES that accepts a (versioned) TRS URI forworkflow_url
, we were wondering whether TRS URIs should also contain the workflow type. However, in this context this seems unnecessary, as a WES request MUST contain aworkflow_type
(as well as aworkflow_type_version
) to be specified anyway.So here are a couple of thoughts:
{type}
but not{type_version
} as path params is inconsistent. If we assume that a workflow can exist in multiple languages (which it arguably can, and we even have (toy/test) examples for this), then surely it could exist in multiple versions of the same language (e.g., NextflowDSL1
andDSL2
). Having only{type}
is thus not sufficient to represent all possible representations of a - functionally identical - workflow under a resource with the same{id}
and{version_id}
.{type}
is just to account for thePLAIN_
types instead, this seems awfully complicated and we might be better off with getting rid of thePLAIN_
types altogether and using just content negotation (application/json
vstext/plain
) for this. I would anyway propose to do that, as usage ofPLAIN_
types is only specified in the description in prose, and thus the behavior cannot be enforced/validated programmatically. Besides, the description is ambiguous, as the current specs potentially present two ways of getting either the one or the other (by using thePLAIN_
types or by asking fortext/plain
, and vice versa), and it is unclear what to do if these two contradict each other (say, I specifyPLAIN_WDL
but then ask forapplication/json
in my header).GET /tools/{id}/versions/{version_id}/containerfile
does also NOT include{type}
even though there are multiple container types enumerated (currentlyDocker
,Singularity
andConda
). Again, this is not consistent with the behavior for workflows.For the next major revision of the TRS specs, I would thus propose to
PLAIN_
types in favor of the more formally specified content types (which are already specified for some endpoints, though possibly not quite consistently){type}
path param and dictate that functionally identical workflows in multiple languages or language versions should be registered under different version identifiers (e.g.,1.0.1-nfl-dsl1
,1.0.1-nfl-dsl2
,1.0.1-wdl
) OR add a{type_version
path parm to account for the possibility of having functionally identical workflows of multiple versions of the same language.../descriptor
and.../containerfile
are really the same endpoint if it weren't for{type}
) or getting rid of the distinction between workflows and tools altogether┆Issue is synchronized with this Jira Story ┆Project Name: Zzz-ARCHIVE GA4GH tool-registry-service ┆Issue Number: TRS-53