-
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
Add apstream adapter #5508
Add apstream adapter #5508
Conversation
your adapter submissions shouldnt be adding 100k lines to prebid.js or changing index.html |
Wrong files got in commit. Fix already. But now tests failed because another adapter. Can I somehow rerun tests? |
Tests failed by reason not related to my adapter. Do I have possibility to rerun tests manually? |
hey @frstua , I don't see the link to the docs PR: https://github.com/prebid/prebid.github.io/ |
Here is the link to the docs: prebid/prebid.github.io#2141 |
Sorry, can't find where us_privacy was mentioned. |
Sorry, my bad, I must have seen |
Hi @aleksatr. All corrections have been made. |
modules/apstreamBidAdapter.js
Outdated
}; | ||
|
||
const isConsent = getIabConsentString(bidderRequest); | ||
if (isConsent) { |
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.
can you add a bidder config option here and document it so that pubs who aren't comfortable with this functionality can disable the dsu part of your adapter? I think it is okay to default to on but should be on the bidder params section of the documentation at http://prebid.org/dev-docs/bidders.html
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.
noting from the module rules:
"Bidder modules must not(*) make requests to endpoints for functionality other than auctions without:
Disclosure (See the disclosure section below.)
Ability for the publisher to control the additional functionality."
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.
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.
Hi @patmmccann. We are not making any requests to endpoints related to DSU. As I can see in the docs, this rule is applied only for requests to endpoints for functionality other than auctions. In our case DSU just identify user and is generated on client side, this is a part of our workflow. Also it uses local storage only if consent is given.
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.
I think publishers being able to control this additional functionality (generate and pass dsu) aligns quite well with module rules. The idea is some publishers are subject to regulatory scrutiny or have stronger internal standards and may wish to control this functionality at a more detailed level than (isconsent) and may wish to check for consent for various tcf2 purposes and different publishers may want to handle purpoe 4 consent in quite different ways.
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.
Hi @patmmccann. Where is better place to set this parameter? I can add this option to params section to every bid, but does it makes sense as far this DSU flag is browser specific but not bid specific? Does it means what I should take into account parameter only from first bit or what do you could suggest?
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.
I imagine a publisher would do something like: var adUnits = [{ code: 'test-div', sizes: [[300, 600]], bids: [{ bidder: 'apstream', params: { dsu: 'off' //defaults to on } }] }];
when they want to override the dsu behavior in some context
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.
There's precedence for bidder-specific global params. e.g.
pbjs.setConfig({
apstream: {dsu: false}
});
Your code can do a getConfig
to look for this optional param.
Hi @patmmccann @aleksatr. I added possibility to disable DSU via config section. Also docs have been updated. Is it possible to merge it today as we have publisher and he is waiting for adapter. |
LGTM |
Hi @aleksatr. I see that 4.2.0 is released and the adapter in question is merged. Dev-docs PR prebid/prebid.github.io#2141 was merged as well. However, I can not find it on the Download page. I was unable to find the information regarding how this list is updated |
hey @romanych , your docs PR is not properly formatted, that's why your adapter was not picked up by the renderer (jerkyll). You're missing "front matter" - https://jekyllrb.com/docs/front-matter/. Check how You can open a discussion on the original docs PR as well, to confirm this. |
Type of change
Description of change
Inherit from prebid.js
Explicit ad-unit code
Explicit ad-unit ID
Be sure to test the integration with your adserver using the Hello World sample page.
tech@audienceproject.com
For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:
Other information