-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New adapter: Adpixis #5965
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
base: master
Are you sure you want to change the base?
New adapter: Adpixis #5965
Conversation
✅ Deploy Preview for prebid-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
dev-docs/bidders/adpixis.md
Outdated
|
||
| Name | Scope | Description | Example | Type | | ||
|:--------------|:---------|:--------------------------------------------------------------|:-----------------------|:----------| | ||
| `host` | required | Ad network's RTB host | `'<adserving domain>'` | `string` | |
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.
Sorry, host cannot be a required parameter. See https://docs.prebid.org/prebid-server/developers/add-new-bidder-go.html#consider-your-geography
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.
The comment below suggests that 'host' is only needed for Prebid.js, but the metadata field says pbjs: false
. Something's off.
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.
@bretg thanks for your comments, we will fix everything, about the host
- we use it only as query parameter for PBS to identify the publisher on our side
https://github.com/prebid/prebid-server/blob/master/static/bidder-info/limelightDigital.yaml#L1
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.
@bretg @apykhteyev Fixed
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.
Sorry, 'host' isn't fixed. PBS does not allow any kind of domain (e.g. adpixis-rtb.com) as a required parameter. It's ok to be an optional parameter, so the adapter has to have a reasonable default.
The way to think about this is from the publisher's perspective:
- It's lazy to assume that your publisher customers can geo-balance your endpoints
- It's unreasonably duplicative to make them pass something like this to the limelight endpoint just to identify the alias.
The way you need to do that is to just hardcode the alias domain in your YAML file. e.g.
"http://ads-pbs.ortb.net/openrtb/{{.PublisherID}}?host=adpixis-rtb.com"
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.
@apykhteyev - the doc here shows the "host" parameter as "adpixis-rtb.com". That doesn't look like a publisher ID, and it appears there's already a "publisherId" parameter that does this job. Please downgrade host
to optional or even better, remove it entirely?
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.
Fixed. I've set Host to optional
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.
Is this "host" parameter different for each customer? Different for each request?
I do like the idea of renaming this because "host" has a meaning that may not be relevant here. However, you already have a "PublisherId" parameter. How would "publisher" differ from "publisherId"?
@apykhteyev can you please address these questions?
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.
The limelight digital adapter, which adpixis is an alias of, still has host
as required despite the documentation change here to optional.
Host does not correspond to the bidder domain, though I imagine it is being used to route requests internally somehow. Is that the case @apykhteyev? If that is true, @bretg does this really have to be optional?
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.
The important thing is that we do not force publishers decide how to route each request. I agree the 'host' parameter is misnamed if it's just something like an account ID.
remove required fields
Set Host to Optional
🏷 Type of documentation
📋 Checklist