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

pubGENIUS bid adapter: support video #6040

Merged
merged 2 commits into from
Dec 3, 2020

Conversation

edmonl
Copy link
Contributor

@edmonl edmonl commented Nov 26, 2020

Type of change

  • Feature

Description of change

pubGENIUS bid adapter supports video mediaType with this PR.

  • test parameters for validating bids
{
  bidder: 'pubgenius',
  params: {
    adUnitId: '1001',
    bidFloor: 1,
    test: true,

    // other video parameters as in OpenRTB v2.5 spec
    video: {
      placement: 1,
      w: 640,
      h: 360,
      mimes: ['video/mp4'],
      protocols: [3],
      skip: 1
    }
  }
}

Other information

@kevinstubbs for team

@edmonl
Copy link
Contributor Author

edmonl commented Nov 26, 2020

Just a note that all tests passed locally.

@osazos
Copy link
Collaborator

osazos commented Dec 2, 2020

Hi,
Could you use the same params object you put in the PR description in your pubgeniusBidAdapter.md file, It matches the expected params regarding your isValidVideo() method.

All the rest of the code seems ok.

@edmonl
Copy link
Contributor Author

edmonl commented Dec 2, 2020

Hi,
Could you use the same params object you put in the PR description in your pubgeniusBidAdapter.md file, It matches the expected params regarding your isValidVideo() method.

All the rest of the code seems ok.

Done @osazos

@osazos osazos added LGTM and removed needs review labels Dec 3, 2020
Copy link
Collaborator

@osazos osazos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@osazos osazos merged commit 19da021 into prebid:master Dec 3, 2020
@bretg
Copy link
Collaborator

bretg commented Dec 3, 2020

@edmonl - are you sure you want to specify videoCacheKey? Please see #6078 -- better to never start using it than try to get rid of it later.

@edmonl
Copy link
Contributor Author

edmonl commented Dec 3, 2020

@edmonl - are you sure you want to specify videoCacheKey? Please see #6078 -- better to never start using it than try to get rid of it later.

@bretg Good point, thank you! You're right. While this PR has merged, we'll keep this in mind and maybe remove videoCacheKey later.

@edmonl edmonl deleted the meng/pubgeniussupportvideo branch December 3, 2020 20:17
@bretg
Copy link
Collaborator

bretg commented Dec 3, 2020

Here's the problem -- if you leave videoCacheKey in, then publishers have to set up a separate creative in the ad server for you.

Once the first one does that, you're screwed -- you can't remove videoCacheKey until you check with every pub that they're ready for it.

I highly recommend you remove videoCacheKey right now before you're live. But if you insist, I'll add you to the list of bidders in #6078 .

@edmonl
Copy link
Contributor Author

edmonl commented Dec 4, 2020

I'll remove it in another PR. #6081 @bretg

@edmonl edmonl mentioned this pull request Dec 4, 2020
2 tasks
@bretg
Copy link
Collaborator

bretg commented Dec 4, 2020

Yup, that would be fine. This one's already been merged.

stsepelin pushed a commit to cointraffic/Prebid.js that referenced this pull request May 28, 2021
* pubGENIUS bid adapter: support video

* update md doc to show more video params
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants