Skip to content
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

Introduce fixed service info type #143

Merged
merged 1 commit into from
Jan 12, 2021

Conversation

aniewielska
Copy link
Contributor

This PR sets the value of ServiceType artifact to tes. Looks decently in ReDoc and in OpenAPI generator, but I am not sure, if this is the best possible option in OpenAPI.
Work inspired by htsget, as there seems to be no good examples in Cloud Workstream.

@kellrott
Copy link
Member

Are we trying to get this in before 1.0?

@aniewielska
Copy link
Contributor Author

It should not change the payload of the spec, other than by making tes the only option for the service type (artifact). But I would like to get some attention of TASC, as the solution does not look particularly elegant (> 13 lines of code just to say, that only one option is possible for one field) and would need to be repeated in all other APIs implementing Service Info.

Copy link
Member

@jb-adams jb-adams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, essentially the same as it appears in htsget. I agree the solution doesn't look particularly elegant, but it seems to be the only way to do it in OpenAPI.

@aniewielska
Copy link
Contributor Author

I will merge it now then, as it essentially does not change the wire format (implementations were supposed to be returning tes anyway).

@aniewielska aniewielska merged commit a9e1d6b into ga4gh:master Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants