-
Notifications
You must be signed in to change notification settings - Fork 32
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
Allow inclusion of an external DRS URI in file descriptors #1437
Conversation
For a mutually exclusive condition, the "oneOf" keyword should be used. With that said, I am not sure these field lists should be mutually exclusive. I can imagine implementation layers that would want the content type, size, and hashes of a file while also utilizing a drs_uri to serve the file. For instance, a data shopping cart where you can check boxes to add files to your shopping cart, and you are summing the file size to inform the user of their total end payload. So, I think the "anyOf" should be used to allow both field sets to be supplied, while requiring at least one of them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tag
@JoshuaFortriede does your comment still hold? The current rev of this PR is not using an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the description of the field, we are only expecting a single value at a time (or the absence of the field) and therefore I don't think we need the "oneOf" keyword:
https://json-schema.org/understanding-json-schema/reference/type.html
The schema change itself looks good to me, but all the PRs must go through staging first as stated in the commiters document
@ESapenaVentura, we wanted to make sure that people don't accidentally create phantom files by omitting the property. That's why the PR distinguishes between absence and |
Co-authored-by: Hannes Schmidt <[email protected]>
d20a5a3
to
f86b255
Compare
@ESapenaVentura apologies for not following the SOP, I've rebased against the staging branch according to the committers doc. |
}, | ||
"drs_uri": { | ||
"description": "Data Repository Service URI pointing at this file. If this property is absent, a data file exists in the repository containing this descriptor. If this property is present and not `null`, a data file exists in another repository at the specified URI. If this property is present but `null`, the data file is not available, but will likely become available in the future, along with an updated descriptor referencing it.", | ||
"user_friendly": "DRS URI", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be better to use "Data Repository Service URI" as user-friendly name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conflicts.
## [Released](https://github.com/HumanCellAtlas/metadata-schema/) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know why this change is here. It seems unrelated but I don't have sufficient insight into the release process use in this repository.
Continued in #1575 |
Context
The Lungmap project has a requirement to source files from an external system that will provide DRS URIs. These files will not be importable to TDR but the DRS URIs will need to be wired through into the file descriptor so they can be surfaced downstream in the browser. This is a draft PR capturing a proposal to allow the inclusion of these external DRS URIs.
Related DCP system spec PR