-
Notifications
You must be signed in to change notification settings - Fork 321
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
Schema Descriptors Framework #288
Conversation
@lrosenthol - last chance to object… |
@treiloff if you are just asking if I am OK with splitting it up and we will review each of the smaller ones individually - that’s fine. Anything else, no comment |
@lrosenthol then please review this PR. @kstreeter is fine with it (he wrote it) and I'm fine with it, too (I do have problems with some of the descriptors that will come in later PRs) |
docs/descriptors.md
Outdated
|
||
Data described by an XDM schema may change over time, and as such a data object may reflect an update of a previous instance of that object. There are different ways that an update may be handled, and this way depends both on the nature of the data and the specific application it is being used for. | ||
|
||
XDM defines a schema descriptor of type `xdm:descriptorUpdatePolicy`, which describes several common methods of handling an update: |
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.
Is this really about schema updating? seems more about the data described in a 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.
Yes, it is about how the data described by the schema is updated. How the update occurs can be described on a property-by-property basis. The update policies might be different for different datasets that share a schema, which is why we need to put this in a descriptor versus just annotating the schema in the repo.
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.
but isn't this outside the scope of XDM and the schemas? This seems like a business thing, perhaps even specific to XC?
docs/descriptors.md
Outdated
|
||
A number of additional schema descriptors are defined by XDM: | ||
|
||
* `xdm:identityContext`: allows a property in a schema to be used as an [Identity](https://github.com/adobe/xdm/blob/master/docs/reference/context/identity.schema.md), even if it does not conform to the Identity 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.
these don't seem like good ideas to me! I would remove this whole section.
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.
I think @trieloff intends on having a review for each of these. Probably easier to leave the doc text in for now and updating it if/when those PRs are merged.
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.
@lrosenthol, @kstreeter – yes, that was my intention. I can remove the entire contentious section from the doc, we just have to remember putting them back in in the individual PRs.
docs/descriptors.md
Outdated
} | ||
``` | ||
|
||
The customer ID is present, but does not contain other information needed to ensure the identity is fully described, such as the ID namespace, or whether this value represents the application's native ID for this customer or if this is an ID given my some external system. |
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.
why is this specific to identify and IDs? Wouldn't this concept apply to other things?
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.
It could: I am sure we could define descriptors to transform or augment any data to match an XDM definition. This descriptor isn't meant to be that general though. This is used to solve a specific (and common) problem we have when ingesting customer data: pulling out the identifiers so that we can link or join the customer-provided data to data that we generate that contains identifiers.
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.
so just as with the other case, this seems very specific to your business and not to XDM in general
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.
The concept of end user identity is a key aspect of both how XDM models data about end-user interactions (ExperienceEvents
) and how that is translated to an actionable view of the end user (Profile
). The Identity
and Namespace
schemas describe the framework used to manage that end user identity. All of the schemas mentioned are part of XDM.
This descriptor solves a part of that overall problem: sometimes we need to connect data modeled as XDM (like EE and Profile) back to data that isn't modeled in XDM (for example, a legacy customer management system). It is likely that anyone using EE or Profile would have this problem, and need this descriptor.
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.
Yes, and EEs are very tied to your business - and that's fine. But they have nothing to do with mine or that of CC. As such, identity is not a core piece of our XDM strategy.
I am perfectly fine with this being an XC-specific extension to XDM - but as is, it is not something that belongs in the "core" (IMO).
|
||
Next, they define an extension to `SchemaDescriptor` containing the in-use flag: | ||
|
||
```json |
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.
I think this example is wrong, esp in terms of namespacing
Removing examples and schema descriptor descriptions that are not yet part of the framework and will be submitted in separate PRs. @kstreeter – let's bookmark this edit, so we don't forget about them.
@lrosenthol Are we good to merge this so far? We can discuss the details of the various descriptors in separate PRs. |
Yes, go ahead and merge and we'll work out the details in separate PRs |
The actual schemas will come in separate PRs, imported from #154
I'm splitting the big PR into multiple smaller PRs, so that we can discuss every schema descriptor on its own merits.