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

Add id to icarResource.json #264

Closed
gra-moore opened this issue Dec 2, 2021 · 10 comments
Closed

Add id to icarResource.json #264

gra-moore opened this issue Dec 2, 2021 · 10 comments

Comments

@gra-moore
Copy link
Collaborator

gra-moore commented Dec 2, 2021

For the streaming API to work all resources must have an id.

Options:

  1. add id field to icarResource. Simple string value. Aligns well with the current use of id on many sub resources. e.g. icarResource.json and many others. Means some resources without the id property will now need to be given it.

  2. add id field to meta
    Protocol States: resource instance representations must have the meta property and this must contain meta and id.
    Backwards compatible. Requires protocol to define the need for this to be present. No change to any resources as is. id would be defined as string. Maybe call this resourceId?

  3. Mandate the use of @self property in the protocol. This by definition must be a globally unique value. No changes needed for this. Slightly mixes addressing and identity.

Proposal: (1).

@gra-moore
Copy link
Collaborator Author

For now the datatype of the Id is string. But it must be a unique value across all resources coming from a given server. This can be stated in the API and does not affect the data model. So for now we are NOT requiring the use of URIs for identity of individual resources.

@gra-moore
Copy link
Collaborator Author

It appears the use of id in sub types of resource is not consistent. Some are strings and some are schema and value. e.g. icarReproEmbryoResource

@gra-moore
Copy link
Collaborator Author

Option 4. Use sourceId

@cookeac
Copy link
Collaborator

cookeac commented Dec 2, 2021

Option 4. Use sourceId

See the discussion of sourceId and source in issue #153 and #154. These are indeed very close to what we need, but as we saw from today's discussion, they might not be guaranteed to be unique across resource types.

@ahokkonen
Copy link
Contributor

ahokkonen commented Dec 7, 2021

Thank you @cookeac for putting links to our previous discussions.
There was also a discussion at pull request comments and as I remember there we'd agreed on the final solution.
And yes, uniqueness is not guaranteed - for example in our data exchange solution we can only guarantee it to be unique withing the given resource type, but not entire system. It all depends on the ability of the integration parties to produce such identifier or good will to create an additional layers for handling resource identifier mappings between different sources (internal or external).

@gra-moore
Copy link
Collaborator Author

I have reviewed this some more. From the discussion in #153 and the latest text in the resource type:

 "source": {
      "type": "string",
      "description": "Source where data is retrieved from. URI  or reverse DNS that identifies the source system."
    },
    "sourceId": {
      "type": "string",
      "description": "Unique Id (ideally UUID) for the resource in the original source system. Treat source and sourceId as an icarIdentifierType."
    },

I think it can be concluded that sourceId should be a globally or at least source unique identifier. Specifically, it is not scoped to resource type.

Proposal: Require sourceId in the meta section of resources being exchanged via the protocol. The client consuming a feed from a server will need to label the source so this is not required.

While not ideal (looks a little bit ugly, possibly confusing) it requires no data model changes and no redefinition of what should be in the property.

@gra-moore
Copy link
Collaborator Author

Agreed to use sourceId. This issue will be closed as it requires no changes to the model.

@gra-moore gra-moore reopened this Jan 14, 2022
@gra-moore
Copy link
Collaborator Author

Although we decide on approach. Decided to leave open up PR is merged.

@stale
Copy link

stale bot commented Apr 14, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-issue Identifies that an issue is stale and will be closed unless reactivated. label Apr 14, 2022
@cookeac cookeac removed the stale-issue Identifies that an issue is stale and will be closed unless reactivated. label Apr 14, 2022
@stale
Copy link

stale bot commented Jul 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-issue Identifies that an issue is stale and will be closed unless reactivated. label Jul 13, 2022
@cookeac cookeac removed the stale-issue Identifies that an issue is stale and will be closed unless reactivated. label Jul 19, 2022
@cookeac cookeac closed this as completed Sep 8, 2022
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

No branches or pull requests

3 participants