-
Notifications
You must be signed in to change notification settings - Fork 22
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
In endpoints.yaml/.json for cohorts and datasets the /{id}/ request should be served by a beaconCollectionsResponse
(or summary responses)
#123
Comments
I agree with the table description about how the responses are expected to be. |
I don't think the alternatives are necessary, as with the correction above, the behavior would be the expected one.
This is the history and rationale of these terms and responses. |
I always saw the point of having a distinction between "data entities" and "collections". But then,
Well, it could make sense that "collections" endpoints always return a "collectionsResponse"; since you already define the unique collection you can deliver the list of records for the selected response entity. This might be the cleanest definition:
And both have |
... but one thing I still do not see is where in the schema we define which type of |
This is in the configuration endpoint.
|
@jrambla This doesn't help. Logically a dataset isn't a collection of variants but a collection of records in the entities of the data model, with inherently consistent ids used for referencing between the records of the different entities. Remembering correctly, as a different type of collection we designed cohorts, which can (potentially) contain entries from different datasets (allowing for transversal discovery though never really war gamed). It isn't clear to me where a "default schema" would come in here:
BUT My point was different: There is no way to indicate what the requested collection type (i.e.
I made earlier statements about the necessity to have parameters for these. I remember our design discussions about having the responses separated into resultsets to allow multiple aggregations (and this is now in line with my suggestion to treat every Beacon response collected by an aggregator simply as a resultsets entry). But you cannot aggregate into resultsets if you cannot define what they are ort how to select amongst them. (We currently do this by providing a |
(I'll be telegraphic, in order to be concise) About what is a In the Similarly, In the resultSet response the A topic, that must happen in a separate discussion, is how do you select just some datasets or cohorts in your request. I agree that today we don't have a direct solution for that other than doing a query per each dataset or cohort you would select for. |
@jrambla Datasets: Well, the unitary connection in the diagram was mostly to emphasize the difference between cohorts as a transversal collection type, and datasets as the "internal organization" one, as described:
The model doesn't describe the dataset to "contain" only variants but effectively also all related entry types which can be accessed through the relationship graph. I actually only left the
Well, if you can "navigate" to individuals belonging to the variants, you have a finite set of individuals representing all that are part of the dataset. I don't really understand what you mean w/ "internal navigation"; here it is just about which entities in principle can be accessed as belonging to the dataset - the internal handling of record retrieval is of no interest. But this all isn't really part of the problem; it comes here:
... which is obvious. But:
The
Yes, exactly. We just use |
... is actually in line w/ the framework examples (which in principle don't have to be in line w/ the default model but that would be a bit confusing): beacon-v2/framework/src/requests/examples-fullDocuments/beaconRequestBody-MAX-example.yaml Line 8 in afaf76a
|
beacon-v2/models/src/beacon-v2-default-model/datasets/endpoints.yaml
Line 63 in 75850c3
This picks up where some back-and-forth discussions in #116 started and rephrases this as the minimal issue. In short, since we use a separate
beaconCollectionsResponse
response schema for record-level responses in collections (datasets
,cohorts
) the single entity id endpoints (/datasets/{id}/
and/cohorts/{id}/) should use this response schema (invoked through locally defined
CollectionsResponse) instead of a
beaconResultsetsResponse(invoked through locally defined
ResultsOKResponse`):/datasets
beaconCollectionsResponse
response.results
/datasets/{id}
beaconCollectionsResponse
response.results
/datasets/{id}/biosamples
beaconResultsetsResponse
response.resultSets
with a list of biosamples inresponse.resultSets[0].results
... etc.
Alternative 1
The special
beaconCollectionsResponse
could be discarded since in principle we could have e.g. a list of datasets or cohorts insideresponse.resultSets
without theresults
component used for record level biosamples etc.Alternative 2
Collections should always respond with a
beaconCollectionsResponse
, also for/datasets/{id}/biosamples
style endpoints since it is just one list of records associated with one collection (i.e. not potentially multiple resultsets).The text was updated successfully, but these errors were encountered: