Default TCF1 GVL in anticipation of IAB no longer hosting the v1 GVL #1433
Conversation
SyntaxNode
changed the title
config/config.go
Outdated
| // TCF1 defines the TCF1 specific configurations for GDPR | ||
| type TCF1 struct { | ||
| FetchGVL bool `mapstructure:"fetch_gvl"` | ||
| DefaultGVLPath string `mapstructure:"default_gvl_path"` |
There was a problem hiding this comment.
The word "Default" in a config file leads me to think this might be, well, a default value and not one necessarily provided by the host. Consider renaming to FallbackGVLPath.
gdpr/vendorlist-fetching.go
Outdated
| @@ -27,8 +27,27 @@ type saveVendors func(uint16, api.VendorList) | |||
| // Nothing in this file is exported. Public APIs can be found in gdpr.go | |||
|
|
|||
| func newVendorListFetcher(initCtx context.Context, cfg config.GDPR, client *http.Client, urlMaker func(uint16, uint8) string, TCFVer uint8) func(ctx context.Context, id uint16) (vendorlist.VendorList, error) { | |||
| var defaultVL api.VendorList = nil | |||
|
|
|||
| if TCFVer == tCF1 && len(cfg.TCF1.DefaultGVLPath) > 0 { | |||
There was a problem hiding this comment.
Consider extracting this block to it's own method.
gdpr/vendorlist-fetching.go
Outdated
| if TCFVer == tCF1 && len(cfg.TCF1.DefaultGVLPath) > 0 { | ||
| defaultVLbody, err := ioutil.ReadFile(cfg.TCF1.DefaultGVLPath) | ||
| if err != nil { | ||
| glog.Fatalf("Error reading from file %s: %v", cfg.TCF1.DefaultGVLPath, err) |
There was a problem hiding this comment.
Should this method return an error as well if we can't load the default GVL? Same with the result of the ParseEagerly call.
gdpr/vendorlist-fetching.go
Outdated
| if TCFVer == tCF1 && len(cfg.TCF1.DefaultGVLPath) > 0 { | ||
| defaultVLbody, err := ioutil.ReadFile(cfg.TCF1.DefaultGVLPath) | ||
| if err != nil { | ||
| glog.Fatalf("Error reading from file %s: %v", cfg.TCF1.DefaultGVLPath, err) |
There was a problem hiding this comment.
Can you please add a test case for a failed file read?
There was a problem hiding this comment.
I can't have a fatal error at this point if I want to test for that, as won't glog.Fatalf() mean that the test will fail at that point?
| purposes: []int{2}, | ||
| }, | ||
| }) | ||
| server := httptest.NewServer(http.HandlerFunc(mockServer(1, map[int]string{ |
There was a problem hiding this comment.
Can we please use a var to hold the list version that we use here when creating the mockServer (for this and the tests above) and then later to assert that same version? It just helps understand better what those integer values represent while reading the tests.
No description provided.