New Adapter: Kobler #3904
New Adapter: Kobler #3904
Conversation
* New Adapter: Kobler
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
|
Hello @bsardo @przemkaczmarek, what can I expect as an estimate for merging this, and is there anything I can contribute with to move this forward? |
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
|
@przemkaczmarek Hello, is there anything blocking this for a second reviewer and is there anything I can do to help speed this process up? |
|
Thank you for the review so far. @bsardo When do you think you have time to review this? |
|
Hi @TommyHPettersen, I'll give this a look as soon as the following is addressed. We recently released PBS 3.0, more specifically v3.1.0, which updates Prebid Server package import references throughout the project from As a result, please merge with master (no rebase) and then ensure all Prebid Server package import references in the files you’ve changed are |
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
|
Thank you for the response @bsardo, i have now updated all the package imports to Hope this helps! |
adapters/kobler/kobler.go
Outdated
| func (a adapter) getEndpoint(request *openrtb2.BidRequest) string { | ||
| if request.Test == 1 { | ||
| return a.devEndpoint | ||
| } |
There was a problem hiding this comment.
I haven't seen this behavior before. Why do you require a separate endpoint for test requests?
There was a problem hiding this comment.
We have a test campaign in our DEV environment that's set to bid on a specific article. For this to work, we have to do some manual manipulation of the article in our system and this is easier to do in the DEV environment.
This is the same as for our Prebid.js adapter.
There was a problem hiding this comment.
IMO it's good to add some comments:
// Use a separate endpoint for testing purposes in the DEV environment.
// Required due to Kobler's internal test campaign setup.
because: Introducing a separate endpoint (devEndpoint) for test requests is unusual in Prebid Server, since standard tests use the same endpoint as the production environment.
There was a problem hiding this comment.
@przemkaczmarek I have added this now, thank you!
static/bidder-params/kobler.json
Outdated
| "description": "A schema which validates params accepted by the Kobler adapter", | ||
| "type": "object", | ||
|
|
||
| "properties": {} |
There was a problem hiding this comment.
Curious, how do map impressions to your system without any kind of identifier?
There was a problem hiding this comment.
Since we're a contextual platform, we identify placements from the combination of the article URL and the allowed sizes. Both of these are always present without having to add any other identifier.
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
|
Thank you @SyntaxNode and @przemkaczmarek for the reviews, hoping we can get this merged soon as this PR has been open for a while! |
|
Hello @VeronikaSolovei9, we have been waiting for this PR to be merged for 3 months and as far as I understand you guys have been updating to v3 in that time which might have blocked a lot of PRs, is there anything else I can do here to help get this PR merged? Thank you! |
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
adapters/kobler/kobler.go
Outdated
| // Use a separate endpoint for testing purposes in the DEV environment. | ||
| // Required due to Kobler's internal test campaign setup. | ||
| func (a adapter) getEndpoint(request *openrtb2.BidRequest) string { | ||
| if request.Test == 1 { |
There was a problem hiding this comment.
We discussed this with the team and decided request.test is not the best place to control the URL.
Instead we propose to add a test flag to imp[].ext.prebid.bidder.kobler or request.imp[].ext.kobler locations (they are equivalent).
With this approach this flag will be encapsulated to Kobler adapter only.
To make it work you will need to add an optional property to kobler.json, define imp_kobler.go and handle it in adapter code accordingly.
For the reference, please see missena.json, ExtImpMissena and usages of TestMode flag.
There was a problem hiding this comment.
Okay thank you, I have now added a similar solution to our server adapter here
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
openrtb_ext/imp_kobler.go
Outdated
| package openrtb_ext | ||
|
|
||
| type ExtImpKobler struct { | ||
| Test *bool `json:"test"` |
There was a problem hiding this comment.
No need to declare it as a pointer. Dafault value for bool is false.
Modify it to non-boolean and then just assign it without nil check: testMode = impExt.Test
Please keep in mind that if a request has more than one imp then testMode will have a value from the last impression.
There was a problem hiding this comment.
I also check i == 0, so it should only be for the first imp we do this parsing.
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
Code coverage summaryNote:
koblerRefer here for heat map coverage report |
docs - prebid/prebid.github.io#5587