-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Conversation
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.
There was a problem hiding this 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, |
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Type of change
Description of change
Official update from Yieldlab. Many thanks to our friends from Platform Lunar for the work on the adapter.
Bugfixes:
Other:
Changed params to the actual naming used by Yieldlab to avoid confusion from customers. Corresponding Pull Request to the documentation: Update Yieldlab params with new naming prebid.github.io#651
Updated maintainer to be Yieldlab
contact email of the adapter’s maintainer: [email protected]
official adapter submission