-
Notifications
You must be signed in to change notification settings - Fork 195
Replace aggregatable_expiry with event_attribution_window #577
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
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.
Please also update the header validator to remove the aggregatable_expiry
field and add the new one.
Two points:
|
Co-authored-by: Andrew Paseltiner <apaseltiner@google.com>
I think that is reasonable, but I don't think using a single value will make swapping to a tuple or similar too difficult, so I would prefer to err on the side of YAGNI.
|
Done, PT another look. |
That's fine with me.
I was thinking of use-cases that would use this capability and also want consistency with aggregatable reports. For instance, the use-case in #561 might take advantage of this for both reports, where we have privacy restrictions about small |
If they want consistency, they would be able to set expiry == event_report_window. Do we think a reporter would ever want to:
|
(2) is required to (partially) address #561 where we'd want consistency and the window < expiry. |
Co-authored-by: Andrew Paseltiner <apaseltiner@google.com>
Preview | Diff