GAM Video Module: make hook to category translation dependent on config

[patmmccann]

This would add a requirement that getConfig('brandCategoryTranslation.translationFile') exists for the hook to the translation module to occur, solving #4048

This is technically a breaking change, as publishers that want this hook to occur from dfp video module would have to define this config, however I think there are zero affected pubs, because the default translation file is for freewheel, so it wouldn't make any sense not to have the config if you are using dfp video module. As such, I don't think it actually requires a doc change on prebid.org, but I added one just in case.

[patmmccann]

@jsnellbaker any updates on this?

[jsnellbaker]

So in looking through this change and the code within the modules/categoryTranslation.js file, it should be technically fine for now, but it may cause some headache later.

At the moment, the categoryTranslation module only has 'default' functionality for the freeWheel adserver. If we ever wanted to setup a default translation file for dfp, then this change would require the publisher to specifically include the setConfig setup and list the 'default' file - or else this hook would never execute. But specifying the setConfig, it would trigger the call for the file anyway (which is the other way to use the categoryTranslation module).

While I'm not sure how I feel about that point, I'm not sure of another alternative to suppress the warning message.

@snapwich Do you have any thoughts about the above or any other alternatives that may be open?

[patmmccann]

It seems unlikely there would ever be a default for gam #3379 (comment) Since GAM doesn't have fixed categories, I imagine the configuration would always need to occur.

[gglas]

@snapwich Hey rich - have you had a chance to look at Jason's question?

[snapwich]

personally i'd just accept the warning being present and have people ignore it. that's why i created it as a warning and not an error because it's only there to signal a possible misconfiguration. but if we really want to put the hook behind a config flag i'm not opposed to that idea.

i'm not familiar enough with the dfpAdServerVideo module to say if that's a good or bad idea so i'll have to defer to @jsnellbaker's expertise.

[patmmccann]

At least half a dozen publishers have complained about this warning over the past two years or so

…

On Mon, May 10, 2021, 2:35 PM Rich Snapp ***@***.***> wrote: personally i'd just accept the warning being present and have people ignore it. that's why i created it as a warning and not an error because it's only there to signal a *possible* misconfiguration. but if we really want to put the hook behind a config flag i'm not opposed to that idea. i'm not familiar enough with the dfpAdServerVideo module to say if that's a good or bad idea so i'll have to defer to @jsnellbaker <https://github.com/jsnellbaker>'s expertise. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6464 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAM25Z27357O35OM73YM2ETTNARPDANCNFSM4ZWBVRIA> .