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

Update Yieldlab adapter and add official maintainer #2231

Merged
merged 9 commits into from
Mar 19, 2018
Merged

Update Yieldlab adapter and add official maintainer #2231

merged 9 commits into from
Mar 19, 2018

Conversation

mirkorean
Copy link
Contributor

@mirkorean mirkorean commented Mar 7, 2018

Type of change

  • Bugfix
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

Description of change

Official update from Yieldlab. Many thanks to our friends from Platform Lunar for the work on the adapter.

Bugfixes:

  • TTL was wrong. Should be 5 minutes
  • Identifier used for dealId was wrong
  • netRevenue should be "false"
  • Video adsize should be set dynamically

Other:

Yieldlab auctions are valid for 5 minutes and not 10. A delivery of an adtag would lead to a new auction after the 5 minute expiration time.
Did stands for demandId which is not a default and not a good identifer for deals. We should use pid (partnershipId) here as a unique identifer for a connection between supply and demand (including deals).
The adsize of a video placement can have a lot of different adsizes and is never "1x1".
Platform Lunar and Yieldlab agree that the Adapter should be offically maintained by Yieldlab.
Campaigns set up in Yieldlab's test network to deliver all the time.
To make it less confusing for customers, we should use the actual naming used in our systems for the params used.
Copy link
Collaborator

@harpere harpere left a comment

Choose a reason for hiding this comment

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

One minor comment.

@@ -67,16 +67,16 @@ export const spec = {
width: sizes[0],
height: sizes[1],
creativeId: '' + matchedBid.id,
dealId: matchedBid.did,
dealId: matchedBid.pid,
Copy link
Collaborator

Choose a reason for hiding this comment

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

May want to add a test that verifies pid is being mapped to dealId as you expect. Would also help verify that this was an intended change!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comment. I agree and added a commit: 4010d70

Copy link
Collaborator

Choose a reason for hiding this comment

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

looks good, thx. will get this merged.

@harpere harpere added LGTM and removed needs review labels Mar 19, 2018
@harpere harpere merged commit 6382fe6 into prebid:master Mar 19, 2018
mifefr added a commit to mifefr/Prebid.js that referenced this pull request Mar 29, 2018
* 'master' of https://github.com/prebid/Prebid.js:
  EngageBDR New Bid Adapter (prebid#2309)
  [FEAT] adunit sizes support (prebid#2320)
  Support aliases in prebidServer (prebid#2257)
  Changing default currency file to https (prebid#2306)
  Update stalebot labels (prebid#2319)
  Enhance location detection within utils (prebid#2167)
  if cache markup is not enabled, set it to the default value 0 (prebid#2302)
  Serverbid Bid Adapter: Added archon alias (prebid#2293)
  Smart Ad Server: Fix bug when multi bids (prebid#2170)
  NEW adapter AdtelligentBidAdapter (prebid#2137)
  add optional param to bridgewellBidAdapter (prebid#2289)
  Increment Pre Version
  Prebid 1.6.0 Release
  Unit test fixes (prebid#2301)
  PBS videoCacheKey and vastUrl (prebid#2101)
  Add Oneplanetonly Bid Adapter (prebid#2269)
  firing new adRenderFailed event when renderAd() fails (prebid#2210)
  Add Content Ignite adapter (prebid#2268)
  add hb_cache_id, hb_uuid should be deprecated and replaced by hb_cache_id (prebid#2273)
  Update Yieldlab adapter and add official maintainer (prebid#2231)
  Update for Media.net adapter (prebid#2232)
  Update to Rubicon Adapter for mediaTypes support (prebid#2272)
  message formatting (prebid#2285)
  Yieldbot impression image creation fix (prebid#2277)
  Updated Bid params (prebid#2275)
  Audience Network: Add 'pbv' and 'cb' query params (prebid#2252)
  Add e-planning analytics adapter (prebid#2211)
  Add vastUrl for Gamma Adapter Video (prebid#2261)
  update params for test bid (prebid#2267)
  Updated adUnitCode (prebid#2262)
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* Set time to live to 5 minutes

Yieldlab auctions are valid for 5 minutes and not 10. A delivery of an adtag would lead to a new auction after the 5 minute expiration time.

* Use correct identifer for deals

Did stands for demandId which is not a default and not a good identifer for deals. We should use pid (partnershipId) here as a unique identifer for a connection between supply and demand (including deals).

* Set revenue to gross-price

* Add dynamic adsize for vast url

The adsize of a video placement can have a lot of different adsizes and is never "1x1".

* Change maintainer to Yieldlab

Platform Lunar and Yieldlab agree that the Adapter should be offically maintained by Yieldlab.

* Use yieldlab test campaigns

Campaigns set up in Yieldlab's test network to deliver all the time.

* Use yieldlab naming for params

To make it less confusing for customers, we should use the actual naming used in our systems for the params used.

* Update test spec for yieldlab adapter

* Add test for pid to dealId mapping
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