Closed uniqueg closed 1 year ago
Thanks @patmagee :) I would still like to prepare a PR for the solution proposed here, before reaching out to implementers to vote. But I'm afraid it's gonna take me another two weeks or so.
@uniqueg can you please rebase this on develop
@wleepang @cjllanwarne I wonder what your thoughts are on this modified PR?
I think tags
is more important than name
for this response. (see my comment above).
From a client perspective, runid
, (start|end)_time
, and status
are certainly must haves. I'd say tags
is required and it is strongly recommended that "run_name" and "workflow_name" tags are included.
I think
tags
is more important thanname
for this response. (see my comment above).From a client perspective,
runid
,(start|end)_time
, andstatus
are certainly must haves. I'd saytags
is required and it is strongly recommended that "run_name" and "workflow_name" tags are included.
I'm with you. It seems to me like we all agree and it's mostly a semantic argument. The main point is that we want to encourage implementers as much as possible to make this info available, without making their life unduly hard if, for whatever reason, they can't. So let it be in tags
:)
@uniqueg once you update this PR to remove name and make fields like start_time and end_time optional, I will go ahead and merge it!
@uniqueg once you update this PR to remove name and make fields like start_time and end_time optional, I will go ahead and merge it!
Done
Alternative to #172 Closes #165
This PR differs from #172 in the following ways:
RunSummary
is defined, to be used specifically for theruns
array items inRunListResponse
RunSummary
extendsRunStatus
(not modified!) withrequired
additional fieldsRunSummary
orRunStatus
(anyOf
) are acceptable responses forGET /runs
start_time
andend_time
, also workflowname
andtags
are included in the extended response;workflow_url
was not included, as it can point to a local object and its support would therefore require WES to serve objects (up for debate of course, following #172 and #165)Taken together, this proposal avoids or partly addresses the following concerns raised against #172:
RunStatus
schema remains untouched, thus retaining the behavior ofGET /runs/{run_id}/status
(which also usesRunStatus
).RunStatus
instead ofRunSummary
(or indeed any other response that has therun_id
andstate
properties) in run list responses for backward compatibility (anyOf
), this PR adds a couple of measures to more strongly encourage the use of the new extended schema. In particular,DEPRECIATION WARNING
is added to the description ofRunListResponse
, informing implementers that the use ofRunStatus
inRunListResponse
will be discontinued in favor ofRunSchema
only.RunSummary
, which - together with the depreciation warning - should tell every implementer clearly where the specs will be going in this respect; indeed, as soon as support forRunStatus
inRunListResponse
is removed (a simple change), any compliant implementation MUST provide these additional fields in their run list responses.It does NOT address the concern that "clients may not agree with selection of fields".