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

DRS preference in File profile #60

Merged
merged 3 commits into from
Oct 28, 2024
Merged

DRS preference in File profile #60

merged 3 commits into from
Oct 28, 2024

Conversation

a-l-holmes
Copy link
Collaborator

Motivation

Make DRS file types the preferred attachment type for the File profile and send out a warning if the attachment URI is not DRS.

Approach

Adapt the DRSattachment profile to make DRS preferred but not strictly required.

@a-l-holmes a-l-holmes requested a review from torstees September 30, 2024 17:20
@github-actions github-actions bot temporarily deployed to pull request September 30, 2024 17:25 Inactive
Copy link

github-actions bot commented Sep 30, 2024

🚀 IG Site Preview Deployed

Latest commit: 6936d85

View deployment

Copy link
Collaborator

@torstees torstees left a comment

Choose a reason for hiding this comment

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

Can you make a separate profile, DRS File, which is a child of the NCPI File. Also, can you slice it as "open ended" so that the first content member is required to be a DRS url, but subsequent ones could be something else (i.e. require a DRS id, but optionally allow other URLs in addition)

@github-actions github-actions bot temporarily deployed to pull request October 1, 2024 18:48 Inactive
@a-l-holmes a-l-holmes requested a review from torstees October 3, 2024 13:24
Copy link
Collaborator

@torstees torstees left a comment

Choose a reason for hiding this comment

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

This looks good, but there are some minor tweaks that should make it easier to consume

* content ^slicing.discriminator.type = #pattern
* content ^slicing.discriminator.path = "code"
* content ^slicing.rules = #openAtEnd
* content ^slicing.ordered = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we need slicing at this point since we are fine with regular attachments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right now we have:

* content.attachment.url 1..1 /*Location uri*/
* content.attachment.url ^short = "The URI at which this data can be accessed"

and this made me think there was only one attachment url you could make, thus why I tried doing the slicing. But content is 1..* which inherently means one could attach more than one. Do you want to change how we write the profile or do you think it's self-explanatory?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So, the tricky part is that content is an array of 1 or more things, each of which will contain one attachment. Each of those attachments must have 1 and only 1 URL. It is a bit confusing, but it does allow multiple URLs, but only because you can have more than one thing inside the content array.

* content[DRS].attachment only DRSAttachment
//* content[DRS].profile only
* content[Other].attachment only Attachment
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe you can leave the Other slice out. I believe the open at end is sufficient for allowing other types of attachments in addition to the DRS one(s).

* content ^slicing.discriminator.path = "code"
* content ^slicing.rules = #openAtEnd
* content ^slicing.ordered = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe it would be better if we could state that the DRS Attachment URI will always be the first. So, this should be set to true.

@github-actions github-actions bot temporarily deployed to pull request October 3, 2024 20:15 Inactive
Copy link
Collaborator

@torstees torstees left a comment

Choose a reason for hiding this comment

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

This looks good. I did respond to your comment (only 2 weeks after the fact!). Happy to chat more if it still seems unclear.

* content ^slicing.discriminator.type = #pattern
* content ^slicing.discriminator.path = "code"
* content ^slicing.rules = #openAtEnd
* content ^slicing.ordered = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, the tricky part is that content is an array of 1 or more things, each of which will contain one attachment. Each of those attachments must have 1 and only 1 URL. It is a bit confusing, but it does allow multiple URLs, but only because you can have more than one thing inside the content array.

@a-l-holmes a-l-holmes merged commit 3613fe5 into main Oct 28, 2024
2 checks passed
@a-l-holmes a-l-holmes deleted the DRS-filetype branch October 28, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants