Skip to content
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

JWPlayer Targeting Module #5493

Closed
wants to merge 32 commits into from
Closed

JWPlayer Targeting Module #5493

wants to merge 32 commits into from

Conversation

karimMourra
Copy link
Collaborator

Type of change

  • Feature

Description of change

Adds the jwplayerTargeting module which allows Ad Bidders to access JW Player's Video Ad Targeting information.
The module can be used as a submodule for prebid adapters, allowing them to use our getTargetingForBid() function.
the module will fetch segments for the media ids present in the prebid config when the module loads. If any bid requests are made while the segments are being fetched, they will be blocked until all requests complete, or the 150ms timeout expires.

@patmmccann
Copy link
Collaborator

Why build a new module type instead of utilizing an rtd submodule?

@karimMourra
Copy link
Collaborator Author

@patmmccann do you recommend making this an rtd submodule ? Why so ? The targeting info is in part historical, not always rtd

@patmmccann
Copy link
Collaborator

delaying the auction for data seems like it should use the rtd module #4610 and your functionality feels like a submodule. A bunch of unicorn modules that don't fall into module types with rules described on #5239 feels like potentially an end run around the rules at worst, duplicate functionality at best.

@patmmccann
Copy link
Collaborator

@omerBrowsi what are your thoughts on this

@karimMourra
Copy link
Collaborator Author

@patmmccann thank you for your feedback. Upon review of the rtdModule I think it does make sense for the JW Player module to be a submodule. I will close this PR for now. @omerBrowsi still looking forward to any feedback you may have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants