New Adapter: ResetDigital #3766
Conversation
no need to include `required` if its empty _Originally posted by @onkarvhanumante in prebid#3452 (comment)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| func getBidType(imp openrtb2.Imp) (openrtb_ext.BidType, error) { | ||
| if imp.Banner != nil { |
There was a problem hiding this comment.
Consider this as a suggestion. The current implementation follows an anti-pattern, assumes that if there is a multi-format request, the media type defaults to openrtb_ext.BidTypeBanner, nil. Prebid server expects the media type to be explicitly set in the adapter response. Therefore, we strongly recommend implementing a pattern where the adapter server sets the MType field in the response to accurately determine the media type for the impression.
There was a problem hiding this comment.
Implemented as suggested
could you point out or link where MType changes are implemented?
There was a problem hiding this comment.
Sorry, it was addressed on the point that we support only single format bids, so we could assume the anti pattern. Anyway, it would be more advisable to change to the normal pattern?
There was a problem hiding this comment.
Anyway, it would be more advisable to change to the normal pattern?
Prebid team recommends using MType field. But if it's not doable then current change suffices single format bid. @bruno-siira should mention in Bidder docs that adapter expects only single format bids in the incoming request
There was a problem hiding this comment.
When we're talking about the Bidder Docs what is this file exacly @onkarvhanumante
There was a problem hiding this comment.
Code coverage summaryNote:
resetdigitalRefer here for heat map coverage report |
Implementation of suggestions
Code coverage summaryNote:
resetdigitalRefer here for heat map coverage report |
Code coverage summaryNote:
resetdigitalRefer here for heat map coverage report |
…ate conditionals as requested by Brian Sardo.
Making the last version of the objects to the V3 of openRTB
|
I'm not sure what happened here but something went awry when you merged with master as nearly every file is showing as modified. Please fix and resubmit. |
|
@bsardo , I think that this PR is ready for review again. But, please let me know if you have additional requests before you can review it again. |
static/bidder-info/resetdigital.yaml
Outdated
| key: resetdigital | ||
| supportCors: false |
There was a problem hiding this comment.
You can delete key and supportCors.
static/bidder-info/resetdigital.yaml
Outdated
| key: resetdigital | ||
| supportCors: false | ||
| redirect: | ||
| url: https://sync.resetdigital.co/csync?pid=rubicon&redir={{redirect_url}} |
There was a problem hiding this comment.
Your redirect macro is incorrect. I also see pid=rubicon in your URL which I assume is a typo. Perhaps your url should look like this?
https://sync.resetdigital.co/csync?redir={{.RedirectURL}}
| @@ -0,0 +1,16 @@ | |||
| endpoint: http://b-us-east14.resetdigital.co:9001 | |||
There was a problem hiding this comment.
Endpoint is reachable.
curl -i --location --request POST http://b-us-east14.resetdigital.co:9001
HTTP/1.1 400 Bad Request
Content-Length: 0
Content-Type: text/html
Date: Wed, 13 Nov 2024 17:48:14 GMT
Server: Kestrel
Accept-Ranges: bytes
Cache-Control: no-cache, no-store, must-revalidate
| @@ -0,0 +1,16 @@ | |||
| endpoint: http://b-us-east14.resetdigital.co:9001 | |||
| maintainer: | |||
| email: [email protected] | |||
There was a problem hiding this comment.
We've sent an email to this email address. Please respond to the email with "Confirmed".
| - banner | ||
| - video | ||
| - audio | ||
| gvlVendorID: 1162 |
There was a problem hiding this comment.
Verified GVL ID:
curl https://vendor-list.consensu.org/v3/vendor-list.json | jq '.vendors."1162"'
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 640k 100 640k 0 0 1314k 0 --:--:-- --:--:-- --:--:-- 1311k
{
"id": 1162,
"name": "Reset Digital Europe Ltd",
"purposes": [
1,
3,
4,
5,
6
],
"legIntPurposes": [
2,
7,
8,
9,
10
],
"flexiblePurposes": [
2,
7,
8,
9,
10
],
"specialPurposes": [
1,
2
],
"features": [
2,
3
],
"specialFeatures": [
1,
2
],
"cookieMaxAgeSeconds": 31536000,
"usesCookies": true,
"cookieRefresh": true,
"urls": [
{
"langId": "en",
"privacy": "https://resetdigital.co/policies.html",
"legIntClaim": "https://resetdigital.co/policies.html"
}
],
"usesNonCookieAccess": false,
"dataRetention": {
"stdRetention": 365,
"purposes": {},
"specialPurposes": {}
},
"dataDeclaration": [
1,
3,
8
],
"deviceStorageDisclosureUrl": "https://resetdigital.co/GDPR-TCF.json"
}
There was a problem hiding this comment.
Please update your file name from invalid_media_def.json to invalid-media-def.json.
There was a problem hiding this comment.
Please update your file name from invalid_placement_id.json to invalid-placement-id.json.
There was a problem hiding this comment.
Please update your file name from invalid_video_dim.json to invalid-video-dim.json.
There was a problem hiding this comment.
Please update your file name from multi_format.json to multi-format.json.
There was a problem hiding this comment.
Please update your file name from unknown_media.json to unknown-media.json.
Code coverage summaryNote:
resetdigitalRefer here for heat map coverage report |
Pr review comments 11 13 24
|
|
||
| func GetMediaTypeForImp(reqImp openrtb2.Imp) (openrtb_ext.BidType, error) { | ||
|
|
||
| if reqImp.Video != nil { |
There was a problem hiding this comment.
Consider this as a suggestion. The current implementation follows an anti-pattern, assumes that if there is a multi-format request, the media type defaults to openrtb_ext.BidTypeVideo, nil. Prebid server expects the media type to be explicitly set in the adapter response. Therefore, we strongly recommend implementing a pattern where the adapter server sets the MType field in the response to accurately determine the media type for the impression.
Code coverage summaryNote:
resetdigitalRefer here for heat map coverage report |
| if reqImp.Video != nil { | ||
| return openrtb_ext.BidTypeVideo, nil | ||
| } | ||
| if reqImp.Audio != nil { |
There was a problem hiding this comment.
Consider this as a suggestion. The current implementation follows an anti-pattern, assumes that if there is a multi-format request, the media type defaults to openrtb_ext.BidTypeAudio, nil. Prebid server expects the media type to be explicitly set in the adapter response. Therefore, we strongly recommend implementing a pattern where the adapter server sets the MType field in the response to accurately determine the media type for the impression.
|
@BrunoJacinto, I think this PR is ready for another review, please. |
Ok Dirk, I'll review it from my part. |
|
Ugh, I tagged Bruno instead of Brian. I meant to let @bsardo know that the PR is ready for another review. |
This PR adds a new adapter for Reset Digital to the Prebid Server (Go version). The adapter enables Prebid Server to communicate with Reset Digital for real-time advertising auctions.
Changes:
Added the Reset Digital adapter in adapters/resetdigital/.
Implemented necessary methods for the adapter.
Added unit tests for the adapter in adapters/resetdigital/resetdigital_test.go.
Updated the documentation to include the configuration for the Reset Digital adapter.
Testing:
Unit tests have been written and verified to ensure the adapter works correctly.
Manual testing has been performed to verify the integration with Reset Digital.
Notes:
This system is based on single bid, so it's based on that premise.
Related Issues: