Add handover mechanism (issue #114). #230

sdelatorrep · 2018-11-06T16:58:15Z

Proposal for implementing the handover (#114). It follows the structure presented here.

mbaudis · 2018-11-06T17:56:42Z

@sdelatorrep Great - looks good at a first pass. Two points, related:

example(s)
description of suitable ontology -> example ...

Is "note" a term we (will) use throughout, for free-text comments?

sdelatorrep · 2018-11-07T08:32:29Z

@mbaudis Good points! I'll update this PR with your suggestions.
About note, we already have it in other parts of the spec, e.g. BeaconDatasetAlleleResponse and it's expected to contain free-text comments.

mbaudis · 2018-11-07T08:37:39Z

@sdelatorrep Thanks for the note about note - I didn't take the time myself to comb through the specs :-)

jrambla · 2018-11-19T14:50:11Z

@mbaudis could you review that PR and approved it if fine?. Thx

mbaudis · 2018-11-19T15:30:07Z

+1
...for the structure & general implementation. Better examples & documentation expected to follow in the refinement process.

mbaudis

See note in the discussion. The ontology examples aren't fitting very well; but I support getting the structure in & update the documentation later.

mcupak

A few questions/comments:

url should be a required field.
It seems there's some redundancy with id, label, and note fields. Why not have just one of them as a general description?
I'm not familiar with the ontology, but 3 use cases I have for handover are handover to a different beacon (e.g. controlled access), to a DOS/DRS service, and to a 3rd party portal (not a standard API). How would these be captured with the given ontology/handover object?
There's probably a better name for this field than datasetHandover, maybe something like externalLink.

sdelatorrep · 2018-12-10T14:07:26Z

Hi @mcupak. I'll try to answer your questions:

url is already mandatory (and id too).
About id, label and note, I'm not sure I understand your concern. They are meant for different purposes as you can see in the examples.
I guess it could be something like:

{
      "id": "CUSTOM",
      "url": "http://somebeacon/query?assemblyId=grch37&blabla",
      "label": "Beacon with controlled access",
      "note": "This Beacon requires you have an account in this institution blabla"
}

(maybe in this specific context label and note seem redundant but if you consider the examples where an ontology is used, then they differ).

We proposed different names, datasetHandover and beaconHandover, so they can be easily retrieved by name instead of having to navigate through the structure (which would be needed if we use the same name, externalLink, everywhere).

mbaudis · 2018-12-10T14:48:24Z

Mostly concur w/ @sdelatorrep . However, still not sure about the id => ontology term option, and how this would be handled:

An ontology term would point to a definition of a data structure in an external reference:

{
  "id" : "http://www.ebi.ac.uk/efo/EFO_0004157",
  "label" : "BAM format",
  "url" : "https://api.mygenomeservice.org/handover/9dcc48d7-fc88-11e8-9110-b0c592dbf8c0/",
  "label" : "Sequencing data of the callsets matched by the Beacon query"
}

If we go w/ the Ontology_class prototype provided in SchemaBlocks (and e.g. used in Phenopackets ...), we would have an attribute using this prototype.
While note seems o.k. to me for a free-text attribute, we usually use description (which looks a bit more formal).
I would prefer to have a list of handover objects, especially since they provide their types. If one does a "one type only" representation, this is easily being addressed w/o type.id query through the index...

So I would go with:

{
  "datasetHandover" : [
    {
      "type" : {
        "id" : "http://www.ebi.ac.uk/efo/EFO_0004157",
        "label" : "BAM format"
      },
      "description" : "Sequencing data of the callsets matched by the Beacon query",
      "url" : "https://api.mygenomeservice.org/handover/9dcc48d7-fc88-11e8-9110-b0c592dbf8c0/"
    }
  ]
}

mbaudis · 2018-12-10T15:04:17Z

    Handover:
      type: object
      required:
        - type
        - url
      properties:
        type:
          ref: "https://ga4gh-metadata.github.io/schemas/main/yaml/common.yaml#definitions.Ontology_class
          description: |
            Handover type, as an Ontology_term object with CURIE syntax for the "id" value. Use “CUSTOM” for the "id" when no ontology is available. The "label" attribute would be the "preferred Label"  in the case of an ontology term.
          default:
            id: CUSTOM
            type : null
          example: 
            id: "http://www.ebi.ac.uk/efo/EFO_0004157",
            label: "BAM format"
        description:
          type: string
          description: An optional text describing the handover action.
          example: "VCF file containing structural variants of the samples matched by the Beacon query."
        url:
          type: string
          description: URL endpoint to where the handover process could progress (in RFC 3986 format).
          example: "https://api.mygenomeservice.org/handover/9dcc48d7-fc88-11e8-9110-b0c592dbf8c0/"

sdelatorrep · 2018-12-17T11:44:01Z

Hi! I've applied the proposal by @mbaudis (I renamed type with handoverType so there is no confusion with the OpenAPI keyword type):

    Handover:
      type: object
      required:
        - handoverType
        - url
      properties:
        handoverType:
          $ref: '#/components/schemas/HandoverType'
        description:
          type: string
          description: An optional text describing the handover action.
          example: "VCF file containing structural variants of the samples matched by the Beacon query."
        url:
          type: string
          description: URL endpoint to where the handover process could progress (in RFC 3986 format).
          example: "https://api.mygenomeservice.org/handover/9dcc48d7-fc88-11e8-9110-b0c592dbf8c0/"
    HandoverType:
      type: object
      required:
        - id
      description: |
            Handover type, as an Ontology_term object with CURIE syntax for the "id" value. 
      properties:
        id:
          type: string
          description: >-
            Use “CUSTOM” for the "id" when no ontology is available.
          default: CUSTOM
          example: "http://www.ebi.ac.uk/efo/EFO_0004157"
        label:
          type: string
          description: >-
            This would be the "preferred Label" in the case of an ontology term.
          default: null
          example: "BAM format"

Please, review this and request changes if necessary. Thanks!

mbaudis

Re: Handover: Looks good to me now! I'm not sure that naming the handover "type" => "handoverType" is strictly necessary - the confusion of parameters and parameter attributes is anyway there (description: description: ...); but sure; taken by itself it makes the code here more readable.

mbaudis · 2018-12-19T10:15:16Z

@sdelatorrep I've implemented this now in Beacon+:

"datasetAlleleResponses": [
  {
    "datasetHandover": [
      {
        "handoverType": {
          "id": "pgx:handover:biosamplesdata",
          "label": "Biosamples"
        },
        "description": "retrieve data of the biosamples matched by the query",
        "url": "http://beacon.progenetix.org/beaconplus-server/beacondeliver.cgi?do=biosamplesdata&accessid=5327fe14-0375-11e9-87ab-f392226922dd"
      },
      {
        "url": "http://beacon.progenetix.test/beaconplus-server/beacondeliver.cgi?do=individualsdata&accessid=53283c62-0375-11e9-87ab-f0c3824efc45",
        "description": "retrieve data of the individuals matched by the query",
        "handoverType": {
          "label": "Individuals",
          "id": "pgx:handover:individualsdata"
        }
      },
      {
        "description": "retrieve data of the variants matched by the query",
        "url": "http://beacon.progenetix.test/beaconplus-server/beacondeliver.cgi?do=variantsdata&accessid=53273f2d-0375-11e9-87ab-958a9c7ac1d4",
        "handoverType": {
          "id": "pgx:handover:variantsdata",
          "label": "Variants"
        }
      }
    ],
 ...

jrambla · 2019-01-08T10:49:49Z

@juhtornr could you review, and hopefully approve, this PR, please?

jrambla · 2019-02-19T08:32:25Z

beacon.yaml

+      properties:
+        handoverType:
+          $ref: '#/components/schemas/HandoverType'
+        description:


Let's go back to "notes" instead of "description". The aim of this property is to provide insights or considerations on the handover URL returned, not to describe it. Additionally, using "notes" avoids the naming collision with OpenAPI.

@jrambla Fine with that if that's better fitting to the OpenAPI style.

jrambla · 2019-02-19T08:33:08Z

beacon.yaml

+          $ref: '#/components/schemas/HandoverType'
+        description:
+          type: string
+          description: An optional text describing the handover action.


"An optional text including considerations on the handover link provided"

jrambla · 2019-02-19T08:38:38Z

beacon.yaml

+        description:
+          type: string
+          description: An optional text describing the handover action.
+          example: "VCF file containing structural variants of the samples matched by the Beacon query."


The text in example isn't specific enough. Replace by "This handover link provides access to a summarized VCF. To access the VCF containing the details for each sample filling an application is required. See Beacon contact information details"

jrambla

See comments about "description" and its example

sdelatorrep · 2019-02-20T10:45:46Z

I did the changes requested. Please, review it again and approve the PR. Thanks!

sdelatorrep · 2019-02-26T10:01:36Z

Hi @garysaunders ! This PR is stalled waiting for Michael and others to review it. Please, could you help to chase them? Thanks!

blankdots · 2019-03-25T07:20:16Z

Not sure if worth opening an issue because it is just a question, It is just me but there seems to be a relationship between handover and HATEOAS and HAL ?

sdelatorrep requested review from juhtornr and mbaudis November 6, 2018 16:58

sdelatorrep added this to the 1.1.1 milestone Nov 6, 2018

sdelatorrep mentioned this pull request Nov 13, 2018

Handover method for data access #114

Closed

mbaudis previously approved these changes Nov 19, 2018

View reviewed changes

mcupak previously requested changes Dec 10, 2018

View reviewed changes

mbaudis mentioned this pull request Dec 11, 2018

Extend the ELIXIR Beacon Network UI to support multiple Beacon #239

Open

sdelatorrep dismissed mbaudis’s stale review via 972297e December 17, 2018 11:38

sdelatorrep requested a review from mbaudis December 17, 2018 11:44

mbaudis approved these changes Dec 17, 2018

View reviewed changes

sdelatorrep requested review from teemukataja, mbaudis and mcupak January 17, 2019 11:30

mbaudis previously approved these changes Jan 17, 2019

View reviewed changes

sdelatorrep requested a review from jrambla January 18, 2019 10:16

jrambla previously approved these changes Jan 28, 2019

View reviewed changes

sdelatorrep dismissed stale reviews from jrambla and mbaudis via ce64c20 January 29, 2019 09:40

sdelatorrep requested review from teemukataja, garysaunders, jrambla, juhtornr, mbaudis and mcupak and removed request for jrambla and mcupak February 11, 2019 11:01

jrambla reviewed Feb 19, 2019

View reviewed changes

jrambla suggested changes Feb 19, 2019

View reviewed changes

jrambla previously approved these changes Feb 19, 2019

View reviewed changes

mbaudis previously approved these changes Feb 19, 2019

View reviewed changes

Apply changes requested

3a7eabb

sdelatorrep dismissed stale reviews from mbaudis and jrambla via 3a7eabb February 20, 2019 10:43

sdelatorrep requested review from jrambla and mbaudis February 20, 2019 10:46

jrambla approved these changes Mar 6, 2019

View reviewed changes

sdelatorrep requested review from mbaudis and removed request for mbaudis March 6, 2019 11:13

mbaudis approved these changes Mar 6, 2019

View reviewed changes

sdelatorrep merged commit 504b6e3 into develop Mar 7, 2019

sdelatorrep deleted the sabela-issue-114 branch March 7, 2019 08:45

blankdots mentioned this pull request Mar 25, 2019

(Investigate) Support for handover method for data access CSCfi/beacon-python#38

Closed

blankdots mentioned this pull request Apr 1, 2019

Relationship between HATEOAS and handover ? #276

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add handover mechanism (issue #114). #230

Add handover mechanism (issue #114). #230

sdelatorrep commented Nov 6, 2018 •

edited

Loading

mbaudis commented Nov 6, 2018

sdelatorrep commented Nov 7, 2018

mbaudis commented Nov 7, 2018

jrambla commented Nov 19, 2018

mbaudis commented Nov 19, 2018 •

edited

Loading

mbaudis left a comment

mcupak left a comment

sdelatorrep commented Dec 10, 2018

mbaudis commented Dec 10, 2018

mbaudis commented Dec 10, 2018 •

edited

Loading

sdelatorrep commented Dec 17, 2018

mbaudis left a comment •

edited

Loading

mbaudis commented Dec 19, 2018 •

edited

Loading

jrambla commented Jan 8, 2019

jrambla Feb 19, 2019

mbaudis Feb 19, 2019

jrambla Feb 19, 2019

mbaudis Feb 19, 2019

jrambla Feb 19, 2019

jrambla left a comment

sdelatorrep commented Feb 20, 2019

sdelatorrep commented Feb 26, 2019

blankdots commented Mar 25, 2019

Add handover mechanism (issue #114). #230

Add handover mechanism (issue #114). #230

Conversation

sdelatorrep commented Nov 6, 2018 • edited Loading

mbaudis commented Nov 6, 2018

sdelatorrep commented Nov 7, 2018

mbaudis commented Nov 7, 2018

jrambla commented Nov 19, 2018

mbaudis commented Nov 19, 2018 • edited Loading

mbaudis left a comment

Choose a reason for hiding this comment

mcupak left a comment

Choose a reason for hiding this comment

sdelatorrep commented Dec 10, 2018

mbaudis commented Dec 10, 2018

mbaudis commented Dec 10, 2018 • edited Loading

sdelatorrep commented Dec 17, 2018

mbaudis left a comment • edited Loading

Choose a reason for hiding this comment

mbaudis commented Dec 19, 2018 • edited Loading

jrambla commented Jan 8, 2019

jrambla Feb 19, 2019

Choose a reason for hiding this comment

mbaudis Feb 19, 2019

Choose a reason for hiding this comment

jrambla Feb 19, 2019

Choose a reason for hiding this comment

mbaudis Feb 19, 2019

Choose a reason for hiding this comment

jrambla Feb 19, 2019

Choose a reason for hiding this comment

jrambla left a comment

Choose a reason for hiding this comment

sdelatorrep commented Feb 20, 2019

sdelatorrep commented Feb 26, 2019

blankdots commented Mar 25, 2019

sdelatorrep commented Nov 6, 2018 •

edited

Loading

mbaudis commented Nov 19, 2018 •

edited

Loading

mbaudis commented Dec 10, 2018 •

edited

Loading

mbaudis left a comment •

edited

Loading

mbaudis commented Dec 19, 2018 •

edited

Loading