Privacy Activity Config #2740
Privacy Activity Config #2740
Conversation
There was a problem hiding this comment.
Should this be in the config or privacy package? I think config is the best choice. Though this is the same data model for the rules engine and will likely be used verbatim. Thoughts?
There was a problem hiding this comment.
I agree. The actual configuration belongs here. Data structures and methods to actually carry out the frame work can live in privacy.
| ActivityTransmitUniqueRequestIds | ||
| ) | ||
|
|
||
| func (a Activity) String() string { |
There was a problem hiding this comment.
String form is needed for metrics, logs, debugging, etc.. Defining as an integer for smaller internal representation.
There was a problem hiding this comment.
Would it be more efficient to define the strings in an array rather than case structure? You don't anticipate unused numbers, do you?
config/account.go
Outdated
| @@ -39,6 +39,7 @@ type Account struct { | |||
| PriceFloors AccountPriceFloors `mapstructure:"price_floors" json:"price_floors"` | |||
| Validations Validations `mapstructure:"validations" json:"validations"` | |||
| DefaultBidLimit int `mapstructure:"default_bid_limit" json:"default_bid_limit"` | |||
| Privacy AccountPrivacy `mapstructure:"privacy" json:"privacy"` | |||
There was a problem hiding this comment.
In GPP Phase 4 allowactivities seems to be inside a consent field. Should we rename to consent instead?
40 Validations Validations `mapstructure:"validations" json:"validations"`
41 DefaultBidLimit int `mapstructure:"default_bid_limit" json:"default_bid_limit"`
42 - Privacy AccountPrivacy `mapstructure:"privacy" json:"privacy"`
+ Consent AccountPrivacyConsent `mapstructure:"consent" json:"consent"`
43 }
config/account.go
There was a problem hiding this comment.
The Phase 4 docs are outdated. We changed it to "privacy" in the Phase 3 doc. I'm sure Phase 4 will be updated soon.
There was a problem hiding this comment.
Fixed in the referenced issue.
| Condition ActivityCondition `mapstructure:"condition" json:"condition"` | ||
| } | ||
|
|
||
| type ActivityCondition struct { |
There was a problem hiding this comment.
I believe these will need to be fleshed out a little further when we actually implement the activity infrastructure.
There was a problem hiding this comment.
This is everything needed for Phase 3, right? In Phase 4 there is also planned for privacyreg, but let's finish out Phase 3 first. The implementation for Phase 3 isn't started yet. To keep PRs small, this is just the config.
There was a problem hiding this comment.
Phase 3 defines namespace and wildcards, but those are details of the string value parsing and are not represented at this level. Perhaps they could be though? Did you intend to suggest adding a "name scoped value" of some sort with custom unmarshal/marshal methods to handle the parsing?
There was a problem hiding this comment.
Actually, this is Viper and not standard json. Probably best to leave the "named scope" parsing for other parts of code so we don't have too much Viper specialized functionality.
Implements configuration for GPP Phase 3 privacy activities.