ga4gh / task-execution-schemas

Apache License 2.0
80 stars 28 forks source link

Inconsistency between "GET /tasks" description and response definition #136

Open uniqueg opened 3 years ago

uniqueg commented 3 years ago

Description

There is a minor inconsistency in the definition of the GET /tasks endpoint: the responses section for a 200 response expects a tesListTasksResponse object, which is itself an array of tesTask objects. Now, tesTask objects require the property executors, an object of type tesExecutor. But the description of the view query parameter in the same endpoint definition states that for view type MINIMAL only the Task.id (should perhaps be tesTask.id) and Task.status (tesTask.status) are to be returned. This makes automatic schema-based validation of the response impossible for that endpoint.

Proposed solution

This is perhaps not quite easy to resolve, especially in Swagger 2.0, which does not allow defining different schemas for a response body. In OpenAPI 3.0, the anyOf and oneOf keywords can be used for that purpose (see here, here and here), although, unfortunately, not conditionally dependent of a request parameter (see here, bottom of page).

Perhaps the cleanest thing to do will be to define different schemas for POSTing and GETting tasks, then set different requirements for the latter (e.g., tesTaskResponse.id and tesTaskResponse.state, but not tesTaskResponse.executors). This would also allow the POST-specific schema to get rid of the id property, which is anyway pointless to include, as it is, according to its description, expected to be generated by the implementation. Setting id as required in the response will allow a client or automatic validation framework in the service to ensure that the service, in fact, did generate (and store!) a task ID and avoid potentially empty entries in a MINIMAL representation of the listed tasks (and elsewhere). Finally, once switching to OpenAPI 3.0, the tesTaskResponse schema could be decomposed into separate subschemas for the different views.

Other possible solutions:

uniqueg commented 3 years ago

I'm of course aware that all of the proposed solutions constitute breaking changes that would require any existing implementations to update their code, so I am not hoping for an immediate fix. But as I had to switch off automatic response validation in my framework for that reason (was too lazy to implement a custom validator), I thought I'd put it out there to be taken care with the next bunch of breaking changes.

uniqueg commented 2 years ago

Perhaps related: https://github.com/ga4gh/workflow-execution-service-schemas/pull/172