search

ga4gh / task-execution-schemas

Apache License 2.0
80 stars 27 forks source link

Making file type an optional argument #155

Closed kellrott closed 1 year ago

aniewielska commented 2 years ago

If input/output types are not mandatory, should they have a default value then? How does a default behave for an optional parameter? I will check a code generator for this, as I expect problems.

uniqueg commented 1 year ago

Any feedback on your tests, @aniewielska? Intuitively I would think that handling defaults should be pretty standard for any validator and code generator based on JSON Schema...

aniewielska commented 1 year ago

I think at the moment the type is still mandatory, if we leave defaults with optional. I think the spec was wrong previously in combining defaults and required. But the behaviour we want is no default. When no type of in/output is provided the implementation needs to figure it out itself and not assume it is FILE.

uniqueg commented 1 year ago

Given that the accepted values are defined in an enum, the value passed for tesFileType has to be a value matching one of the enumerated values. If there's only FILE and DIRECTORY in the enum, one couldn't send a valid request omitting this if there's no default set. I would propose adding UNSET or UNSPECIFIED or something to that effect to the enum and then use that as default.

EDIT: I kinda lost track of the point here - that tesFileType is now optional. Not sure how that works with a default value, but I would assume that the expectation is that the default is still set in every request if no other value is supplied - which is not what we want. Removing the default may fix that. But perhaps adding UNSPECIFIED or similar to the enum and then setting that as default may make the intention clearer.

aniewielska commented 1 year ago

Right. So, if we cannot have null enums, I vote for adding UNSPECIFIED and making it default as it nicely described th intention here that the caller would not like to specify the type.