Skip to content
This repository has been archived by the owner on Dec 4, 2021. It is now read-only.

Implement DRS Get Object for files (closes #86) #162

Merged
merged 3 commits into from
Mar 20, 2020
Merged

Implement DRS Get Object for files (closes #86) #162

merged 3 commits into from
Mar 20, 2020

Conversation

natanlao
Copy link
Contributor

@natanlao natanlao commented Mar 18, 2020

Implements partial support for the DRS Get Object endpoint. This
implementation does not permit using the Get Object endpoint to
retrieve information on bundles or collections, and is missing
access information due to ongoing access control work.

Closes #86

@natanlao natanlao added this to the Fir milestone Mar 18, 2020
@natanlao natanlao requested a review from amarjandu March 18, 2020 23:41
@natanlao natanlao self-assigned this Mar 18, 2020
Implements partial support for the DRS Get Object endpoint. This
implementation does not permit using the Get Object endpoint to
retrieve information on bundles or collections, and is missing
access information due to ongoing access control work.
@codecov-io
Copy link

Codecov Report

Merging #162 into master will increase coverage by 0.02%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
+ Coverage   78.15%   78.18%   +0.02%     
==========================================
  Files          93       94       +1     
  Lines        6804     6825      +21     
==========================================
+ Hits         5318     5336      +18     
- Misses       1486     1489       +3     
Impacted Files Coverage Δ
dss/api/drs.py 85.71% <85.71%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91c2730...5216550. Read the comment docs.

Copy link
Contributor

@amarjandu amarjandu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There needs to be some small changes to this. We need to sort out the access_methods field thats returned. I would look at https://github.com/DataBiosphere/azul/blob/58c152df067da3fba40a8191977be2937bfa450b/src/azul/drs.py#L117

This would also mean that we would need 1 more endpoind, the current one being for the metadata, the other one being actually access the data.

dss-api.yml Show resolved Hide resolved
dss/api/drs.py Show resolved Hide resolved
# This only implements file access for now. As such, `expand` is ignored
# `access_url` is also not implemented, pending FLAC work
version = request.args.get("version", None)
replica = request.args.get("replica", "aws")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make this a required parameter in the swagger yml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could. Would we be breaking compatibility with the DRS API if we did so?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah i see what your saying, perhaps we can return a 400 if there are missing query parameters that are required. @chmreid opinion?

dss/api/drs.py Outdated Show resolved Hide resolved
@natanlao
Copy link
Contributor Author

This would also mean that we would need 1 more endpoind, the current one being for the metadata, the other one being actually access the data.

Spoke with @amarjandu over Slack. Since we're only dealing with public files at this point, I'll update the Get Object endpoint to return presigned URLs to files but will not implement the Get Object Access endpoint since there's no point, as all file are already public anyway.

@natanlao natanlao requested a review from amarjandu March 20, 2020 20:01
Copy link
Contributor

@amarjandu amarjandu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets give this a shot.

@natanlao natanlao changed the title Implement DRS Get Object for files (rel #86) Implement DRS Get Object for files (closes #86) Mar 20, 2020
@natanlao natanlao merged commit fa96624 into master Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DSS API: DRS Support
3 participants