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

Object ID Versioning #37

Closed
HadleyKing opened this issue Nov 12, 2021 · 12 comments · Fixed by #42 or #51
Closed

Object ID Versioning #37

HadleyKing opened this issue Nov 12, 2021 · 12 comments · Fixed by #42 or #51
Assignees
Labels
enhancement New feature or request Major
Milestone

Comments

@HadleyKing
Copy link
Collaborator

The BCO_API should handle versioning the way NCBI does for GenBank records. 
For Example:

should be different object in the DB but available by requesting that specific URL or object...

but https://biocomputeobject.org/BCO_28 should ONLY return https://biocomputeobject.org/BCO_28/1.1 as that is the most RECENT version. 

Also the object listing [/api/objects/token/] should only return the most RECENT version by default.

@HadleyKing HadleyKing added the enhancement New feature or request label Nov 12, 2021
@HadleyKing HadleyKing added this to the 2.0.0 milestone Nov 12, 2021
@HadleyKing HadleyKing self-assigned this Nov 12, 2021
@HadleyKing
Copy link
Collaborator Author

related to #22

@HadleyKing
Copy link
Collaborator Author

RE: @syntheticgio

that’s an actual RESTful design which isn’t currently implemented properly. I’ll take a look at how to filter down to that in the DB and I think it’s like a 10 min change.

Actually looks like this is already implemented. I’ll try to check to make sure the functionality is what you’ve described but looks like it.

It takes ‘object root/version id’ and should work in your 2.0.0/4.0.0 version

The two different URL endpoints are here:

bco_api/bco_api/api/urls.py

Lines 169 to 174 in a68a768

path('<str:object_id_root>',
ObjectIdRootObjectId.as_view()
),
path('<str:object_id_root>/<str:object_id_version>',
ObjectIdRootObjectIdVersion.as_view()
),

From what I have tested the <str:object_id_root> URL does not work, and the <str:object_id_root>/<str:object_id_version> will fail with a 500 if an exact match is not made.

My thinking was to get rid of the <str:object_id_root> and fix the logic for the <str:object_id_root>/<str:object_id_version> so that is can handle ALL the logic for what we are talking about.

Thoughts?

@syntheticgio
Copy link
Collaborator

syntheticgio commented Nov 14, 2021 via email

@syntheticgio
Copy link
Collaborator

So when I look at this, it looks like you want <PREFIX> + "_" + <ID>/<VERSION>

But I don't know what the ID part is in this case. The PREFIX is fixed in the object metadata (although it doesn't show in the object listing) as well as the VERSION (user sets it during build). For an object I created through the builder (and manually marked as PUBLISHED in the DB), I see this:

{"object_id": "http://127.0.0.1:8000/BCO_DRAFT_b6ca21b33f3f4ef686cf81f33b97aa6e", "spec_version": "IEEE", "eTag": "", "provenance_domain": {"name": "testBCO", "version": "1", "created": "Sun Nov 14 2021 17:51:48 GMT-0500 (Eastern Standard Time)", "modified": "2021-11-14T22:52:20.894Z", "contributors": [{"contribution": ["createdBy"], "name": "John", "affiliation": "GWU"}], "license": "None"}, "usability_domain": ["Anywhere"], "description_domain": {"keywords": [""], "pipeline_steps": [{"step_number": 0, "name": "", "description": "", "prerequisite": [{"name": "", "uri": {"uri": ""}}], "input_list": [{"uri": ""}], "output_list": [{"uri": ""}]}]}, "execution_domain": {"script": [{"uri": {"uri": ""}}], "script_driver": "", "software_prerequisites": [{"name": "", "version": "", "uri": {"uri": ""}}], "external_data_endpoints": [{"name": "", "url": ""}], "environment_variables": {}}, "io_domain": {"input_subdomain": [{"uri": {"uri": ""}}], "output_subdomain": [{"mediatype": "", "uri": {"uri": ""}}]}, "parametric_domain": [{"param": "", "value": "", "step": ""}]}

Is name meant to be the ID in this case? Or is it object_id (which seems like it is clunky for this). Or is there something else that is missing that either gets created during publication (I've short circuited the process since the publish via the portal doesn't appear to work atm) or otherwise is missing because of a bug?

@syntheticgio
Copy link
Collaborator

MR 42 seems to work (at least at a basic level) if the above is what you want @HadleyKing

@syntheticgio
Copy link
Collaborator

Looks like it is working to support http://localhost:8000/BCO_000010/1.0. Not sure if there was something off about this, but this matches an example object I have on my system. Originally I had split the prefix out but it looks like the regex is fixed (or started working?) and so we might not have to deal with that.

@syntheticgio
Copy link
Collaborator

NOTE: this does look like authentication w/ the bco database is required for this to work - is that intended?

@syntheticgio
Copy link
Collaborator

The first part of this ticket is finished, but working on the second part:

Also the object listing [/api/objects/token/] should only return the most RECENT version by default.

Will try to get it done tonight, its a decent bit of non difficult code so will take an hour or two.

@HadleyKing
Copy link
Collaborator Author

NOTE: this does look like authentication w/ the bco database is required for this to work - is that intended?

This needs to work WITHOUT authentication. It is a mistake as returning a versioned object was originally intended to be only for authenticated access.

But anything that is PUBLISHED should be public, IMO

@syntheticgio
Copy link
Collaborator

Ok, looks like the underlying issue is fixed with only the most recent versions. I'm going to make a different ticket for this not requiring authentication although that might be tricky (since the authentication and user component are being defined by the token). This is ready to be tested (seems to be working for me in my testing, but should be validated if possible), and merging.

@syntheticgio
Copy link
Collaborator

I've changed it for the time being to reflect:

<host/port>/<object ID> is used for DRAFTS (to unbreak what is being used under the hood currently.
<host/port>/bco/<object ID> and <host/port>/bco/<object ID>/<version>/ are used for fetching published BCOs.

This is through the BCO API. I suspect that additionally this wanted to be supported via the Portal. Will make a separate ticket for that though (in the Portal repo).

@syntheticgio
Copy link
Collaborator

Tested and merged; closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment