New Adapter: Rise #2815
New Adapter: Rise #2815
Conversation
|
@misha93m PR checks are failing. Requesting you to please check. |
|
@misha93m PR is created as Draft. Let us know when PR is ready for review |
misha93m
force-pushed
the
typea-adapter
branch
2 times, most recently
from
927a112 to
ebcc451
Compare
|
@misha93m Please let us know if this PR is ready to be reviewed. |
|
Hi @gargcreation1992 @onkarvhanumante |
adapters/rise/rise.go
Outdated
| _ *adapters.ExtraRequestInfo) ( | ||
| requestsToBidder []*adapters.RequestData, | ||
| errs []error, | ||
| ) { |
There was a problem hiding this comment.
Nitpick - for consistency in the codebase please use single line for MakeRequests function declaration.
func (a *adapter) MakeRequests(openRTBRequest *openrtb2.BidRequest, _ *adapters.ExtraRequestInfo) (requestsToBidder []*adapters.RequestData, errs []error) {
//your code
}
adapters/rise/rise.go
Outdated
| return "", errors.New("no publisherID supplied") | ||
| } | ||
|
|
||
| return publisherID, nil |
There was a problem hiding this comment.
nitpick - you could combine the above if statement and simply return publisherID, errors.New("no publisherID supplied")
There was a problem hiding this comment.
Not sure this solution will work. Due to your code, err is initialised, and the caller will handle err properly:
publisherID, err := a.extractPublisherID(openRTBRequest)
if err != nil {
errs = append(errs, fmt.Errorf("extractPublisherID: %w", err))
return nil, errs
}
There was a problem hiding this comment.
I updated code a bit. Please check.
adapters/rise/rise.go
Outdated
| ) | ||
|
|
||
| for _, imp := range openRTBRequest.Imp { | ||
| if err := json.Unmarshal(imp.Ext, &bidderExt); err == nil { |
There was a problem hiding this comment.
you should not ignore Unmarshal error here. Please do the error handling properly.
if err := json.Unmarshal(imp.Ext, &bidderExt); err != nil {
return "", err
}
adapters/rise/rise.go
Outdated
|
|
||
| for _, imp := range openRTBRequest.Imp { | ||
| if err := json.Unmarshal(imp.Ext, &bidderExt); err == nil { | ||
| if err = json.Unmarshal(bidderExt.Bidder, &impExt); err == nil && impExt.PublisherID != "" { |
There was a problem hiding this comment.
same comment as above. Please do not ignore Unmarshaling error here.
adapters/rise/rise.go
Outdated
| func (a *adapter) MakeBids( | ||
| request *openrtb2.BidRequest, | ||
| _ *adapters.RequestData, | ||
| responseData *adapters.ResponseData) (*adapters.BidderResponse, []error) { |
There was a problem hiding this comment.
Requesting you to please move this to single line for consistency across the codebase.
| Message: fmt.Sprintf("Unexpected status code: %d. Run with request.debug = 1 for more info.", responseData.StatusCode), | ||
| } | ||
| return nil, []error{err} | ||
| } |
There was a problem hiding this comment.
Checking response status code this way is correct but you could also make use of the following code -
if httputil.IsResponseStatusCodeNoContent(response) {
return nil, nil
}
if err := httputil.CheckResponseStatusCodeForErrors(response); err != nil {
return nil, []error{err}
}
adapters/rise/rise.go
Outdated
| return openrtb_ext.BidTypeBanner | ||
| } else if imp.Video != nil { | ||
| return openrtb_ext.BidTypeVideo | ||
| } |
There was a problem hiding this comment.
Warning - This is an anti-pattern. This code assumes that for multi-format bid request, mediatype will be set to type banner by default. I would recommend to set the mediaType based on bidder response. Your bidder should be able to set openrtb2.Bid.Mtype field and use that field here to get mediaTypeForImp. Even if your bidder does not support multi-format request, I would highly encourage you to make this change. Here is an example.
| @@ -0,0 +1,18 @@ | |||
| endpoint: "https://pbs.yellowblue.io/pbs" | |||
| maintainer: | |||
| email: rise-prog-dev@risecodes.com | |||
There was a problem hiding this comment.
Sent an email confirmation to verify email addresss. Please reply received.
There was a problem hiding this comment.
email verified. thanks!
| - video | ||
| userSync: | ||
| iframe: | ||
| url: https://pbs-cs.yellowblue.io/pbs-iframe?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redirect={{.RedirectURL}} |
There was a problem hiding this comment.
User sync URL works. 👍
|
Hi @gargcreation1992 @onkarvhanumante |
|
@misha93m tests are failing with following error:
|
| return openrtb_ext.BidTypeAudio, nil | ||
| case openrtb2.MarkupNative: | ||
| return openrtb_ext.BidTypeNative, nil | ||
| default: |
There was a problem hiding this comment.
In rise.yaml, you have defined that your bidder will be supporting just Banner and Video. I believe you might wanna error out if bidder response returns Audio or Native as bid type. So here is the code -
switch bid.MType {
case openrtb2.MarkupBanner:
return openrtb_ext.BidTypeBanner, nil
case openrtb2.MarkupVideo:
return openrtb_ext.BidTypeVideo, nil
default:
return "", fmt.Errorf("unsupported MType %d", bid.MType)
No description provided.