ga4gh-beacon / beacon-v2-Models

Models that leverage the Beacon Framework v2
Apache License 2.0
4 stars 7 forks source link

Question: `defaultSchema.json`, aliases, local $refs ... #32

Closed mbaudis closed 2 years ago

mbaudis commented 3 years ago

In the current models one provides a default schema (e.g. for biosample). I struggle a bit with the implementation here:

Apologies if answers seem obvious; not to me right now ...

redmitry commented 3 years ago

Correct me if I am wrong. These schemas are for the results found in beacon-framework-v2/responses/sections/beaconResultsets.json :

"definitions": {
  "ResultsetInstance": {
    "properties": {
      "results": {
        "type": "array",
        "items": {
          "type": "object"
        },

Right?

D.

mbaudis commented 3 years ago

@redmitry Yes. Shorter version of my question: Is there any need to stick then in models to the currently demonstrated structure (i.e. biosamples/defaultSchema.json or is it fine to do whatever, as long as the configuration points to the right schema?

    "biosample": {
      "id": "biosample",
      "name": "Biological Sample",
      "ontologyTermForThisType": {
        "id": "NCIT:C70699",
        "label": "Biospecimen"
      },
      "partOfSpecification": "Beacon v2.0.0-draft.4",
      "description":...",
      "defaultSchema": {
        "id": "Biosample",
        "name": "Progenetix schema for a biological sample",
        "referenceToSchemaDefinition": "https://progenetix.org/services/schemas/Biosample",
        "schemaVersion": "v2021-03-05"
      },
      "additionallySupportedSchemas": []
    },
mbaudis commented 3 years ago

... but also: should then be organized better in model definitions.

Tom-Shorter commented 3 years ago

I'm not sure why beacon points to the default schemas, other API's expect that you have read the docs before using it and the beacon docs can contain all the info that's needed about the default schemas in a much more readable format. If using beacon through a front end then the front end should make clear how it can be used, e.g. descriptions, examples and value types, then displaying this info nicely when requested.

The only use case I can see for declaring your schemas is for alternate schemas where the user might not be familiar with the schemas a beacon uses and would therefore want to look at them. The user wouldn't be likely to query a beacon directly, instead users will use front end interfaces. If we assume that the front end was developed in isolation from a beacon implementation (and not built around the available schemas within a beacon) then the front end would need to know what schemas can be used in a beacon, the default schemas should be available in the front end systems as these are publicly available so only the custom schemas need to be defined by the beacon.

If we made the change to only showing the alternate schemas then beacons which use nothing but the default schemas don't need to make available any schemas, which is much easier for them to do and makes the implementation less frustrating.

redmitry commented 3 years ago

Somehow draft-4 avoids using concrete response objects reusing generic BeaconRequestBody and BeaconResultsetsResponse. Note that we still need to define specific RequestParameters (i.e. requestParameters.json). ... by the way the file looks erroneous.

I have a feeling that Beacon implementers expected to provide strict OpenAPI schemas specific for each endpoint defined in each endpoints.json ...

jrambla commented 2 years ago

My comments on the above questions:

  1. The Beacon Framework knows nothing about the models and if they are using any external schema as the default schema. Hence a given model could simply reference to different schemas (SchemaBlocks, Phenopackets, FHIR...) as the defaul schema for a given entry type. As the default schema is the one by default, it must be declared somewhere. Using the documentation would not be enough if we expect to automate some operations in the context of networks and so on.
    1. The default schemas must be findable somewhere, indeed just for testing an implementation/document against it. As these are the default schemas for Model v2, we named it like that. It seemed to me a natural (non-irritating) way of naming them. We could have used another organization and another naming, but this was the one chosen at that time. If we agree that all of them should be in a common/central repo and the Model referencing them, that would be also OK. For me that step should happen / had to happen when the schemas are stable enough,
  2. As we were expecting that different Beacon instances implement only some entry types (not all), we organized the folders for simple deleting one folder would get rid of most of the stuff for that entry type. Probably, using a common/central schema repo would be equally easy for that purpose (I need to review the details).