Add htsget 1.2.0, OpenAPI v3.0.2 spec #385

brainstorm · 2019-02-21T02:40:25Z

As mentioned over twitter:

https://twitter.com/braincode/status/1097808553723670528

I've tried to comply with the official htsget spec, from the human-readable official spec:

https://samtools.github.io/hts-specs/htsget.html

But please let me know if there's something off. Things that concern me:

OpenAPI v3 only seems to have signed int32, while on the spec an unsigned int32 is required by the htsget spec. Granted, this is just for code scaffolding/stubs, but I hope nobody implements it wrong by accident.
I could write in the OAuth aspects in it, but the htsget spec explicitly keeps it out since it's "beyond
the scope of this [htsget] specification". I feel inclined to add it regardless to ease code generation, thoughts?. Just went ahead and added it.
I've added all fields as shown in the spec for BAM, but for VCF, I just picked up some (most used ones?) from the VCF4.2 spec. Would it make sense to list them in the hts-spec as well?

…eneration

…eads endpoint

brainstorm · 2019-02-21T04:11:55Z

Adding the Oauth2 bit might not have been the best idea since AWS API Gateway chokes on it:

delagoya · 2019-02-21T14:20:44Z

@brainstorm thanks a couple of notes on quick review:

unsigned int32 comes from trying to minimize the bytes necessary to store a genomic position in highly compress formats like BAM or CRAM. Since there are query parameters, I think it is OK to define it as int64 with a minimum of 0
Per your AWS API Gateway error, there are some OAS OAuth2 examples. But this seems specific to API Gateway and would not be a good thing to add to a official specification, but rather define a API Gateway specific schema in your own repo, import the schema components using $ref, and add in the custom-authorizer there.
No comment on this yet, will need to dig into the schema details.

jmarshall · 2019-02-21T14:44:02Z

Re (1), that OpenAPI documentation says format is free text so you would seem to be entirely justified in putting uint32 there.

(IMHO the spec saying "32-bit unsigned int" here is over-constraining things. It's a text format so just "unsigned int" or "int, minimum 0" says the same thing as far as the protocol is concerned. However "32-bit" has been there since the original Google Docs document and never come up in discussion as far as I know.)

Re (3), please raise this as a separate issue. Subsetting VCF fields via the fields parameter appears to have been overlooked when variants were added to htsget in PR #233.

Also

The contact information will need changing before merging. This could be the ga4gh-htsget mailing list; or the refget OpenAPI spec uses a GA4GH Security Notifications email address here.

daviesrob · 2019-02-21T15:21:43Z

I think it would be good to avoid 32 bit limitations where not strictly needed. SAM does support positions longer than that, and we're planning to add support for them in HTSlib (see samtools/htslib#709). While not necessary for human sequencing, it is for quite a few other organisms.

…ough. Change contact information to GA4GH

brainstorm · 2019-02-22T00:01:34Z

Thanks everyone for the prompt and valuable feedback, I just added the proposed changes and opened #386 as suggested.

jmarshall · 2019-05-15T14:25:32Z

pub/htsget-openapi.yaml

+    name: GA4GH
+    email: security-notification@ga4gh.org
+  license:
+    name: GPLv3


“The license information for the exposed API.” Probably best to omit license for the time being.

Yea, various parties may be allergic to GPL in particular -- omitting it is probably easier than selecting an alternative? 😉

pub/htsget-openapi.yaml

jmarshall · 2019-05-15T14:27:15Z

pub/htsget-openapi.yaml

+              - "SAMPLE"
+              - "FILTER"
+              - "FORMAT"
+              - "ALT"


Probably best to leave this out until #386 is resolved.

mlin · 2019-05-15T15:13:09Z

Sorry for the terrible belatedness of me adding a couple of comments here:

This is a really terrific companion to the spec, so first thank you!!
The example responses should probably illustrate HTTP URLs in addition to data: URIs, since the redirect is a core mechanic of the protocol, which servers designed for scalable operations would usually rely on. We could make an example for publicly accessible 1000 Genomes data or such.
The possible error responses could be fleshed out in more detail following the prose spec.

I would really like to help with these, but have bandwidth problems so would also welcome if somebody else is able to chew on them.

Thanks again this is great!

Thanks JMarshall Co-Authored-By: John Marshall <John.W.Marshall@glasgow.ac.uk>

…et-openapi

brainstorm · 2019-05-17T08:31:11Z

Thanks everyone for the feedback and code review, I'm refactoring and while I'm at it, I'll introduce 1.2.0 spec details as well (class), WIP here:

https://app.swaggerhub.com/apis/brainkod/htsget/1.2.0#/default/searchReadId

And some open questions for OpenAPI (just in case somebody here knows the solution), here:

https://twitter.com/braincode/status/1129301936828588038

@mlin

…k from @mlin (thanks), also add class as an extra attribute. /cc @ohofmann

brainstorm · 2019-06-26T05:56:40Z

Spec bumped to 1.2.0, thanks @mlin for the feedback and the heads up @ohofmann.

I hope there's nothing else missing/broken or out of sync... looks valid/ok on SwaggerHub:

https://app.swaggerhub.com/apis/brainkod/htsget/1.2.0

OT: Who is in charge of the CircleCI config on this repo?

brainstorm · 2019-07-17T00:11:57Z

👋

jmarshall · 2019-09-02T13:24:18Z

The example of a /variants/{id} response is a BAM stream. It needs to be VCF or BCF instead.

OT: Who is in charge of the CircleCI config on this repo?

You can figure that out by looking at the relevant PR (#328). However it's immaterial — if this PR is rebased to after PR #328 landed, Circle CI will be happy. As this PR does not involve PDFs, it doesn't matter either way.

brainstorm · 2019-09-02T22:42:27Z

Thanks @jmarshall, good catch, fixed now!

/cc @ohofmann

jmarshall · 2019-09-16T13:08:02Z

Agreed at the meeting that this is now ready for merging. Two remaining minor concerns:

The swaggerhub URL at the top is specific to brainkod:

servers:
    url: https://virtserver.swaggerhub.com/brainkod/htsget/1.2.0

Can this be replaced with a non-specific URL like the one in refget-openapi.yaml?

Last time I looked the treatment of error return codes was a bit haphazard, however we're happy for that to be tidied up at a later date.

@brainstorm: If you would like to address (1), I'll take care of rebasing and merging this. Thanks!

Includes barebones authorizationCode Oauth2 flow, which should aid/inform code generation. Uses int64 with minimum 0, unsure if that is really an uint64 though.

jmarshall · 2019-09-16T14:06:33Z

Merged as 766424f. Thanks!

* Update CRAM spec section on substitution matrix and codes. * Respond to review comments. * CRAM Slice and Container ref seq IDs must match (samtools#401) * Code review part 2. * Fix minor typo in the predecessors of BCF2 (samtools#427) * layed -> laid * Update MAINTAINERS.md (samtools#432) Proposal to add Rasko Leinonen as refget maintainer. * add jmmut to MAINTAINERS.md and move Cristina to "Past Members" * change order of maintainers * Clarify that INFO/END is used to form a CHROM:POS-END region (PR samtools#436) (samtools#436) INFO/END (when present) provides the size of the interval that the variant is located in, along with the CHROM and POS fields. This is also used when indexing VCF/BCF files, as can be gleaned from §6.3.1's description of BCF's rlen field. The implications of INFO/END have not previously been clear. In the absence of clear documentation, some SV tools have been using INFO/END fields for their own semi-related purposes (using INFO/CHR2:INFO/END as the other side's position in an interchromosomal rearrangement), leading to broken .csi indexes and region queries that don't work. Fixes samtools#425. * Allow C and Java native text spellings of NaN and infinities (samtools#409) C's printf doesn't output mixed case, while Java's Double.valueOf and Double.toString parse/output only `Infinity`, not `Inf`. Rather than requiring special-case code for both input and output in both languages, relax the VCF specification to allow NAN/INF/INFINITY case-insensitively. (Add "IEEE-754" to be specific and to improve the line breaks.) * Adding a note about <NON_REF> (samtools#380) * Update PDFs (CRAM and VCF additions; others cosmetic) CRAMv3 PRs samtools#401 and samtools#412. VCFv4.3 PRs samtools#380 (<NON_REF>), samtools#409 (infinity/NaN), and samtools#436 (INFO/END). All: Minor typo and whitespace formatting fixes. * Add htsget 1.2.0 OpenAPI v3.0.2 spec (PR samtools#385) Includes barebones authorizationCode Oauth2 flow, which should aid/inform code generation. Uses int64 with minimum 0, unsure if that is really an uint64 though. * Codify the existing policy of generally squashing PRs (PR samtools#444) * Permit AP_Delta in multi-ref slices. This means AP_delta can become negative. I have validated this decodes fine in both htsjdk and htslib. This is because AP is ITF8 and hence signed, like all other integers, so it would need explicit code to forbid this (which obviously isn't in the implementations). Hence the limitation is primarily one of an over-zealous specification. The impact of this is for position-sorted multi-ref slices AP can legally be stored efficiently. Also clarified fields in the container compression header when in multi-ref mode. Fixes samtools#431

brainstorm added 3 commits February 21, 2019 13:38

Add htsget 1.1.1, OpenAPI v3.0.0 spec

3b747e6

Add barebones authorizationCode Oauth2 flow, should aid/inform code g…

867eef9

…eneration

Add explicit URL parameter definition, fix name/description swap in r…

b8497d0

…eads endpoint

jmarshall added the htsget label Feb 21, 2019

Change to int64 with minimum 0, unsure if that is really an uint64 th…

09ee68b

…ough. Change contact information to GA4GH

brainstorm mentioned this pull request Feb 21, 2019

htsget VCF fields specification #386

Open

brainstorm mentioned this pull request Mar 14, 2019

OpenAPIv3 support to define routes, endpoints and stubs softprops/serverless-aws-rust-http#3

Closed

brainstorm mentioned this pull request Mar 27, 2019

__hts_idx_t uncompressed and compressed offset getters samtools/htslib#845

Open

jmarshall reviewed May 15, 2019

View reviewed changes

brainstorm and others added 3 commits May 17, 2019 12:39

Reference instead of VReference

7e70f56

Thanks JMarshall Co-Authored-By: John Marshall <John.W.Marshall@glasgow.ac.uk>

Adressing different concerns about the openapi spec

fc87823

Merge branch 'htsget-openapi' of github.com:umccr/hts-specs into htsg…

a8e4578

…et-openapi

brainstorm changed the title ~~Add htsget 1.1.1, OpenAPI v3.0.0 spec~~ Add htsget 1.2.0, OpenAPI v3.0.2 spec May 17, 2019

Bump to 1.2.0 htsget spec, flesh out error messages and other feedbac…

5e04acf

…k from @mlin (thanks), also add class as an extra attribute. /cc @ohofmann

Variants example should contain VCF, not BAM

0619ce6

Substitute URL to non-specific one, thanks @jmarshall

aba949c

jmarshall added the Merged For marking PRs that have been merged outwith GitHub and are shown as merely “Closed” label Sep 16, 2019

jmarshall closed this Sep 16, 2019

jmarshall mentioned this pull request Feb 20, 2020

Proposal: Clarify that the htsget specification text takes precedence over OpenAPI #472

Merged

jmarshall mentioned this pull request Apr 13, 2021

Define JSON Schema for htsget JSON payloads #254

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add htsget 1.2.0, OpenAPI v3.0.2 spec #385

Add htsget 1.2.0, OpenAPI v3.0.2 spec #385

brainstorm commented Feb 21, 2019 •

edited

Loading

brainstorm commented Feb 21, 2019

delagoya commented Feb 21, 2019

jmarshall commented Feb 21, 2019

daviesrob commented Feb 21, 2019

brainstorm commented Feb 22, 2019 •

edited

Loading

jmarshall May 15, 2019

mlin May 15, 2019

jmarshall May 15, 2019

mlin commented May 15, 2019

brainstorm commented May 17, 2019 •

edited

Loading

brainstorm commented Jun 26, 2019

brainstorm commented Jul 17, 2019

jmarshall commented Sep 2, 2019

brainstorm commented Sep 2, 2019 •

edited

Loading

jmarshall commented Sep 16, 2019

jmarshall commented Sep 16, 2019

Add htsget 1.2.0, OpenAPI v3.0.2 spec #385

Add htsget 1.2.0, OpenAPI v3.0.2 spec #385

Conversation

brainstorm commented Feb 21, 2019 • edited Loading

brainstorm commented Feb 21, 2019

delagoya commented Feb 21, 2019

jmarshall commented Feb 21, 2019

daviesrob commented Feb 21, 2019

brainstorm commented Feb 22, 2019 • edited Loading

jmarshall May 15, 2019

Choose a reason for hiding this comment

mlin May 15, 2019

Choose a reason for hiding this comment

jmarshall May 15, 2019

Choose a reason for hiding this comment

mlin commented May 15, 2019

brainstorm commented May 17, 2019 • edited Loading

brainstorm commented Jun 26, 2019

brainstorm commented Jul 17, 2019

jmarshall commented Sep 2, 2019

brainstorm commented Sep 2, 2019 • edited Loading

jmarshall commented Sep 16, 2019

jmarshall commented Sep 16, 2019

brainstorm commented Feb 21, 2019 •

edited

Loading

brainstorm commented Feb 22, 2019 •

edited

Loading

brainstorm commented May 17, 2019 •

edited

Loading

brainstorm commented Sep 2, 2019 •

edited

Loading