ga4gh-beacon / specification-v2

GA4GH Beacon v2 specification.
Apache License 2.0
3 stars 6 forks source link

Beacon API improvement. #29

Closed redmitry closed 3 years ago

redmitry commented 4 years ago

In the Beacon 2.0 clients may query different information: Individuals, Biosamples, Genome Variants, etc. Nevertheless, the ‘RequestQuery’ object is the same for all queries, keeping possible queries for all types of requested information. This embraces scenarios where biosamples endpoint provided with genome variant query. It should be more transparent if each endpoint had defined its own request query object defined.

Best,

Dmitry

mbaudis commented 4 years ago

In my understanding these are 2 different issues:

We had discussed the optional scoping a la biosample.filters=NCIT:C3262. But this requires a clearer adherence to a logical schema & we still need clear use cases for that... My standard example:

... i.e. variants in a reference sample (EFO:0009654) from an individual with a cancer diagnosis (NCIT:C3262). But the cancer diagnosis itself might be encoded w/ the sibling cancer sample etc. So this requires a cookbook ...

Examples, suggestions?

redmitry commented 4 years ago

Well, as far as I understand, this is the same issue.

RequestQuery:
    individual: $ref: '#/components/schemas/IndividualFields'
    biosample: $ref: '#/components/schemas/BiosampleFields'
    g_variant: $ref: '#/components/schemas/GenomicVariantFields'
    datasets: $ref: '#/components/schemas/RequestDatasets'
    filters: $ref: '#/components/schemas/Filters'
    customFilters: $ref: '#/components/schemas/CustomFilters'

The 'RequestQuery' is inside the 'Request' object. "/individuals:" accepts "$ref: '#/components/schemas/Request'", as well as "/biosamples" or "/g_variants"

IMHO it's better to define BiosampleRequest with query: $ref: '#/components/schemas/BiosampleFields' filters: $ref: '#/components/schemas/Filters' customFilters: $ref: '#/components/schemas/CustomFilters'

This way each request will have its own query object and filters. Anything wrong with it?

D.

sdelatorrep commented 4 years ago

Hi @redmitry. Currently, all endpoints accept the same set of parameters because you may want to do a query like "give me all the biosamples which have this SNP". I'm quite sure that in the near future there will be differences in the request parameters for each endpoint, and I agree that your proposal will be the right way to go, but at this moment this is not happening yet.

redmitry commented 4 years ago

Hi @sdelatorrep. I understand that there are many moments to tackle and hope to have a decent final spec. Currently, for instance "/biosamples/{id}/individuals" endpoint has the same Request object, which may have biosamples id provided in a query and be different to the one in the path.

sdelatorrep commented 4 years ago

In this specific case, either it is an error and the request should not contain the id field (but note that it should exist in the receivedRequest present in the response) or it is intentional. Maybe @jrambla knows.

mbaudis commented 4 years ago

@sdelatorrep @redmitry In my understanding (and implementation) this is just an unmatched condition, like for any endpoint & query resulting in 0 items.

sdelatorrep commented 4 years ago

Good point, @mbaudis! And I guess that the Beacon implementers can also decide to throw an error if they want.

redmitry commented 4 years ago

Yeah, but that means that "/biosamples/{id}/individuals" may be substituted with "/individuals" and proper query.

mbaudis commented 4 years ago

@sdelatorrep I think we use 422 ... @redmitry Yes. (And I have to say - and others disagree - that I still do not see the point for discrete paths but rather would use query + handovers.)

jrambla commented 3 years ago

I'm not getting the issue sorry. I need someone to translate that for me. So far, I've been looking much more into the coherence of the responses than in the "correct"/"preferred" way to represent that in OpenApi 3.

mbaudis commented 3 years ago

Yeah, but that means that "/biosamples/{id}/individuals" may be substituted with "/individuals" and proper query.

Yes, but the point is that one has a dedicated path for a biosample (by id) and its associated data; so this seems logical.

However, at least the dataset has to be provided, too; so it should be /datasets/{ds_id}/biosamples/{bios_id}/..., IMO.

jrambla commented 3 years ago

Well, as far as I understand, this is the same issue.

RequestQuery:
    individual: $ref: '#/components/schemas/IndividualFields'
    biosample: $ref: '#/components/schemas/BiosampleFields'
    g_variant: $ref: '#/components/schemas/GenomicVariantFields'
    datasets: $ref: '#/components/schemas/RequestDatasets'
    filters: $ref: '#/components/schemas/Filters'
    customFilters: $ref: '#/components/schemas/CustomFilters'

The 'RequestQuery' is inside the 'Request' object. "/individuals:" accepts "$ref: '#/components/schemas/Request'", as well as "/biosamples" or "/g_variants"

IMHO it's better to define BiosampleRequest with query: $ref: '#/components/schemas/BiosampleFields' filters: $ref: '#/components/schemas/Filters' customFilters: $ref: '#/components/schemas/CustomFilters'

This way each request will have its own query object and filters. Anything wrong with it?

D.

@sdelatorrep there is a way to use Dmitry's suggestion while keeping the request parameters common (like the filters object)?

jrambla commented 3 years ago

In my understanding we are addressing this in the Beacon Framework & Model. Further discussions should happen there.