-
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
Payload value types #411
Payload value types #411
Conversation
make up to date with main repo
…payloadValueTypes
@@ -118,6 +118,15 @@ | |||
"title": "Advertising", | |||
"$ref": "https://ns.adobe.com/xdm/context/advertising", | |||
"description": "The information related to advertising activity related to the experience event" | |||
}, | |||
"xdm:segmentMemberships": { |
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.
Move xdm:segmentmemberships in experience event to PR request around profile stitch (profile merge)
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.
@harleensahni, What would you think about breaking this up into 2 PRs. One to just add xdm:segmentMemberships
to ExperienceEvent and a separate one for the payloads? Then we can more forward with mapping the segment IDs Analytics and others get from AAM now to XDM while the new payload functionality gets worked out.
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.
@cluby-adobe, Will do that, see #444 . Will remove segment membership from this one.
{ | ||
"type": "object", | ||
"properties": { | ||
"xdm:payloadPropensityValue": { |
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 thinking is that propensity can live outside of payload and belongs more with the core of a what segment is.
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.
We revisited this with @kstreeter and decided to bring back propensity here.
Hey @harleensahni, in general I am supportive of this. I did have a question about the "payload" concept. Is there a more semantically specific way to describe this concept? It seems like this is describing the "strength" of the membership, or some characteristic of placement into the segment. Can we use language that describes that? Regarding the "oneOf" construct, I think this is a good idea. One thing we should do is out in some unit tests that make sure that we don't allow property names to be defined differently across choices. I can see this technique being abused :) |
@kstreeter That is a good question. Propensity or strength of membership may more naturally fall as it's own property. But we have use cases for other values that should be tied to the membership and have the same ttl. Here are some examples:
With these, and other use cases, I am hoping to come to a generic solution to this problem of storing values on segments that is broader than just propensity. |
} | ||
|
||
] | ||
}, |
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 don't see any metadata related to the nature of payload itself. The metadata is related to the payload value. Just by looking at data, I won't be able to find out what it represents.
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 belongs in the segment definition (which we haven't defined an xdm about yet). I think you'll have to join between segment definition and segment membership. Does that fit with how you see it, @rkpandey ?
Propensity will live here instead: #452 this pull request (411) is concerned with everything other than propensity. |
We're moving towards storing this payload as map to allow storing multiple values that are semi structured. |
We will proceed with this PR with its simple mechanism for storing a single value for the most basic use cases, like propensity. #452 has been closed and will be added back to this PR |
This reverts commit 929eb20.
…n exiting the segment.
@fmeschbe, I think @chrisdegroot had meant for you to review #452, but it's been supplanted by #411 (which #452 itself supplanted). |
Please link to the issue #…
Add a concept of payload value types to segment membership. Need feedback on if this is an acceptable use of OneOf or if we prefer to enforce restrictions on multiple values in APIs and not the json schema itself.