Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
WIP API #1
WIP API #1
Changes from all commits
57e1818
891ab30
641ac8f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 write-only piece is negotiable. I can see why a site couldn't modify its store of impressions, but it's also not really necessary if the reading by other sites is controlled properly.
The whole business with "what does the e-Privacy Directive say about storing information" is silly. It matters, of course, but not every regulatory system will grow a PECR-like piece of baggage.
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
write-only
term was actually in the original and I also had some doubts about it. I think it's an oversimplification. We should discuss the characteristics of the store somewhere (I think I added a section for that), but it takes more than just saying "write only" to capture the important characteristics.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.
TODO: Work out whether a PrivateAttributionAggregationSystem (wow, right name, hard to type) needs any additional attributes.
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 it possible that we can omit the
PrivateAttribution
prefix from these definitions? I'm not sure where all they might flow to and whether there is namespacing other than from the prefix.I ask because I think
PrivateAttributionImpressionOptions
needs to be further refined as something likePrivateAttributionImpressionOptionsLevel1
(to specify the bag of attributes for the level 1 API), which is really quite a mouthful.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.
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.
Two things on this that are somewhat in tension:
However, I think that the resolution of that tension is in the MPC (DAP specifically). We might want to ask aggregation service operators to limit histogram size as some function of the number of reports they aggregate. (I still somewhat like the idea of refusing to return a value if an aggregate is smaller than some threshold, even though it is perverse from a strictly DP interpretation perspective.) If the aggregator supports more, then the API should not be where limits are applied. Also, different browsers will have different views on this.
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.
If we're using DAP, then doesn't the browser need to know the histogram size to construct the report?
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, we'll need to know what size to make. We'll ask the site to give us the size they want.
But we don't need to validate or limit that. If it is the wrong size, that's their problem.