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

Inconsistencies in beaconMap between entry types and paths (run, analysis) #125

Open
mbaudis opened this issue Apr 15, 2024 · 7 comments
Open
Assignees

Comments

@mbaudis
Copy link
Member

mbaudis commented Apr 15, 2024

The beaconMap schema has a number of inconsistencies in the endpoints definitions which preclude its use for determining path <-> entry type relations. Starting here:

      runs:
        returnedEntryType: run
        url: https://exampleBeacons.org/datasets/{id}/runs
      analyses:
        returnedEntryType: analysis
        url: https://exampleBeacons.org/datasets/{id}/analyses

... use the wrong entry type key; should be:

      run:
        returnedEntryType: run
        url: https://exampleBeacons.org/datasets/{id}/runs
      analysis:
        returnedEntryType: analysis
        url: https://exampleBeacons.org/datasets/{id}/analyses

This is repeated throughout the document, but not everywhere; e.g. while it is runs in individual, it is run in biosample.

@mbaudis
Copy link
Member Author

mbaudis commented Apr 15, 2024

And a wish from my side: I'd rather love to have an explicit statement about entry type <-> path element somewhere, e.g. in the form of

  genomicVariant:
    entryType: genomicVariant
    pathElement: g_variants
...
  biosample:
    entryType: biosample
    pathElement: biosamples
...

etc. which would make it much more explicit for programmatic retrieval than chomping off the last element of the rootUrl.

@redmitry
Copy link
Collaborator

I see no inconsistency here.

The endpoint name shouldn't reflect neither entry type name, nor the path.
In the analogue to the OpenAPI we have "operationId" which is the semantic name for the operation:
https://spec.openapis.org/oas/v3.1.0#operation-object
It usually reflects the operation purpose like "getBiosampleById".

Cheers,

D.

@mbaudis
Copy link
Member Author

mbaudis commented Apr 15, 2024

@redmitry Well, I wouldn't mind how you name your endpoint as long as path & entry type are provided. But it is inconsistent:

  individual:
...
    endpoints:
...
      runs:
        returnedEntryType: run
        url: https://exampleBeacons.org/individuals/{id}/runs
      analyses:
        returnedEntryType: analysis
        url: https://exampleBeacons.org/individuals/{id}/analyses

compared to:

  biosample:
...
    endpoints:
      run:
        returnedEntryType: run
        url: https://exampleBeacons.org/biosamples/{id}/runs
      analysis:
        returnedEntryType: analysis
        url: https://exampleBeacons.org/biosamples/{id}/analyses

Therefore fix needed. Also: Such inconsistencies can lead to a break of implementations, e.g. if you collect all possible endpoints etc.

@redmitry
Copy link
Collaborator

Well, this shouldn't affect implementations at all, but

 "endpointSets": {
      "Variants": {
        "entryType": "genomicVariant",
        "rootUrl": "https://beacons.bsc.es/beacon/v2.0.0/g_variants",
      },

in this logic

 "Individuals": {
        "endpoints": {
          "genomicVariant": {
            "returnedEntryType": "genomicVariant",
            "url": "https://beacons.bsc.es/beacon/v2.0.0/individuals/{id}/g_variants"
          }

"genomicVariant" should be "Variants"

¯_(ツ)_/¯

@mbaudis
Copy link
Member Author

mbaudis commented Apr 15, 2024

@redmitry Where is the

 "endpointSets": {
      "Variants": {

... from? Shouldn't be anywhere (unless it ended somewhere in a branch?).

Back to case: IMO fix is sound.

@redmitry
Copy link
Collaborator

Hm... u r right.

That's my beacon that still uses "old" names... CRG has been updated:
https://beacons.bsc.es/beacon/v2.0.0/map
https://beacon-apis-demo.ega-archive.org/api/map

@jrambla
Copy link
Contributor

jrambla commented May 4, 2024

As Dmitry was pointing, the "inconsistent" names are just names like the operationId in OpenAPI.
Probably it would be less confusing if the name was "runsEndpoints" or "endpoints4runs" than so similar to the entryTypes' names. It has no other effect in the spec and model than grouping brother endpoints together, which is mandatory if it is an object and not an array.

So I'll be more in favour of that instead of making all the names more similar to entryTypes make them clearly dissimilar.

@mbaudis, I would need more details on your needs for a path, as I don't fully understand the root of the problem.

BTW, I've spotted that in the current master the description for the "EntryType" attribute in "definitions" is emptuy. We must correct that, if not done in any of the branches already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants