-
Notifications
You must be signed in to change notification settings - Fork 750
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
New Adapter: TradPlus #3987
base: master
Are you sure you want to change the base?
New Adapter: TradPlus #3987
Conversation
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
adapters/tradplus/tradplustest/exemplary/simple-native-1.1.json
Outdated
Show resolved
Hide resolved
|
||
func (a *adapter) makeRequest(request *openrtb2.BidRequest) (*adapters.RequestData, []error) { | ||
|
||
tradplusExt, err := getImpressionExt(&request.Imp[0]) |
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.
Please link to your prebid.github.io PR and make it clear in that PR that the TradPlus only looks at the ext for the first impression and ignored the rest. Some other adapters do it, but its not a publisher expected behavior.
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.
ok,I've added to the first comment if anything else is needed ?
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.
Yes. New adapters must also open a PR against the Prebid Docs repository here: https://github.com/prebid/prebid.github.io/
Please see the User Documentation section of our new adapter developer guide.
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.
Follow up question: adapter code takes imp[0].ext
and uses it to build endpoint url. Then sets imp[0].ext
to nil, L49: request.Imp[0].Ext = nil
Other impressions are passed as is, with imp[n].ext.
Is this expected? Do you want to set every imp[n].ext to nil?
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
@SyntaxNode , hi, I have changed these codes,please review these codes. |
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
@SyntaxNode I re-merged from master. |
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
adapters/tradplus/tradplus.go
Outdated
if tradplusExt.AccountID == "" { | ||
return nil, &errortypes.BadInput{ | ||
Message: "imp.ext.accountId required", | ||
} | ||
} |
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.
This check is not needed here. It's handled by PBS core using properties set up:
"accountId": {
"type": "string",
"description": "Account ID",
"minLength": 1
},
and
"required": [
"accountId"
]
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.
done.
Code coverage summaryNote:
tradplusRefer here for heat map coverage report
|
TradPlus is a leading, stable, and reliable ad monetization platform that is committed to providing global developers with fair, transparent and efficient monetization solutions. We also provide ADX and Saas ADX services. We serve 2,000+ global developers. Moreover, Our SDK has been certified by IAB Tech Lab and also joins Google Play SDK Index. And we have Information Security Management Certification.
Our advertising business spans the world. This is our official website: https://tradplusad.com/en. You can get more details about TradPlus.
this is TradPlus adapter version 1.0.
Thank!
Note: TradPlus only looks at the ext for the first impression and ignored the rest.