-
Notifications
You must be signed in to change notification settings - Fork 9
Add loid created timestamp #64
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
Conversation
c8eed9f
to
7aea8ca
Compare
reddit_decider/__init__.py
Outdated
"id": self._user_id, | ||
"logged_in": self._logged_in, | ||
"cookie_created_timestamp": self._cookie_created_timestamp, | ||
"loid_created_timstamp": self._loid_created_timestamp, |
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.
this function is used to dump data for v2 event exposures, and the v2 event schema doesn't have this field on user so we can eliminate it here
reddit_decider/__init__.py
Outdated
"device_id": self._device_id, | ||
"origin_service": self._origin_service, | ||
"cookie_created_timestamp": self._cookie_created_timestamp, | ||
"loid_created_timestamp": self._loid_created_timestamp, |
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.
similar here, there's no top level field in v2 event schema for loid (or any field for loid in v2 events) so let's get rid of it here as well
16ab2c7
to
0c6e089
Compare
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.
lgtm!
let's tag a new release after we merge this. |
💸 TL;DR
This PR passes
loid_created_ms
from the edge context to the decider asloid_created_timestamp
This corresponding PR adds
loid_created_timestamp
into the decider context.