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

Issue 419 - changing Identity id from URI format to plain string #420

Merged
merged 14 commits into from
Jul 13, 2018

Conversation

kstreeter
Copy link
Collaborator

This PR is to address issue #419, where the definition of @id in the Identity schema is too constrained for real usage. This PR changes the field from @id to xdm:id, and updates all of the examples.

@kstreeter
Copy link
Collaborator Author

@chrisdegroot, @cmathis, @jwen-adobe, @cluby-adobe, @saurabhere, @gabebarcelos-adobe this is a proposed breaking change to the Identity schema to address concerns about the need to support unstructured identity values. Please take a look and let me know if this change is acceptable. I know it will have impact on existing data generators, but lets have the schedule impact discussion in another forum and just review technical correctness here. Thank you!

@jwen-adobe
Copy link
Contributor

@kstreeter This looks good from tooling perspective based on my test.

@cdegroot-adobe
Copy link
Contributor

Technically this PR does remove the need for URIs, but the examples still are URIs.

What is the reason we are not using URL encoding? The cases where there are non URL safe characters is a corner case.

Copy link
Contributor

@cdegroot-adobe cdegroot-adobe 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 clear and addresses the requirements as i understand them.

@saurabhere
Copy link
Contributor

Looks good. So, _id becomes id in XED for identities. Hope this is communicated to all the consumers(UPS, siphon)

@saurabhere
Copy link
Contributor

Can this also include similar changes in the orgunit.schema and geounit.schema ?

@cdegroot-adobe
Copy link
Contributor

@saurabhere this change is limited to the Identity schema. The standard in XDM is to use @id for the globally unique identifier of all entities.

@cdegroot-adobe
Copy link
Contributor

@fmeschbe - we need to include this in the July 16th milestone.

@@ -0,0 +1,6 @@
{
"xdm:id": "ISBN13#978-0765382030",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we really use an ISBN example where ISBN is already properly covered with an official URN namespace ?

In addition, the xdm:id field is identifyig consumers as per the description, where an ISBN is not exactly a consumer ;-)

@@ -1,10 +1,16 @@
{
"xdm:identities": [
{
"@id": "https://data.adobe.io/entities/identity/id123",
"xdm:id": "https://data.adobe.io/entities/identity/id123",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This example, while not exactly wrong, does not seem to reflect the intent of this PR.

@fmeschbe fmeschbe merged commit 6e4668b into master Jul 13, 2018
@fmeschbe fmeschbe deleted the issue-419-identity-type branch July 13, 2018 15:27
@fmeschbe fmeschbe added the v0.9.3 Scheduled for v0.9.3 label Jul 13, 2018
@fmeschbe fmeschbe restored the issue-419-identity-type branch July 13, 2018 17:30
@fmeschbe fmeschbe deleted the issue-419-identity-type branch July 14, 2018 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v0.9.3 Scheduled for v0.9.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants