New Adapter: iqiyi #4605
Open
New Adapter: iqiyi #4605
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
remove go.mod and go.sum files add go.mod go.sum format json file
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
Collaborator
|
@Heihuang your tests are failing due to formatting issues. Please run |
Code coverage summaryNote:
iqiyiRefer here for heat map coverage report |
Contributor
|
I send a request via postman: And I got the response with this error: |
| endpoint *template.Template | ||
| } | ||
|
|
||
| func pickCurrency(req *openrtb2.BidRequest, resp *openrtb2.BidResponse) string { |
Contributor
There was a problem hiding this comment.
I can't say that this would be a critic comment, perhaps a better name for the function would be selectCurrency or something like that
| return nil, errs | ||
| } | ||
|
|
||
| reqJson, err := json.Marshal(request) |
Contributor
There was a problem hiding this comment.
The same, it is not critical issue, but the better name would be reqJSON
|
|
||
| var iqiyiExt openrtb_ext.ExtImpIqiyi | ||
| if err := jsonutil.Unmarshal(bidderExt.Bidder, &iqiyiExt); err != nil { | ||
| errs = append(errs, &errortypes.BadInput{Message: "bad Iqiyi bidder ext"}) |
Contributor
There was a problem hiding this comment.
Both unmarshal operations on lines 63 and 69 use the same error message. Differentiate them
| imp := &request.Imp[i] | ||
| if imp.Banner != nil { | ||
| b := *imp.Banner | ||
| if (b.W == nil || b.H == nil || *b.W == 0 || *b.H == 0) && len(b.Format) > 0 { |
Contributor
There was a problem hiding this comment.
*b.W == 0 and *b.H == 0 are hardcoded values. Extract to named constants:
const (
defaultBannerFormatIndex = 0
)
| } | ||
| } | ||
| if imp.BidFloorCur == "" && imp.BidFloor > 0 { | ||
| imp.BidFloorCur = "USD" |
Contributor
There was a problem hiding this comment.
- Comment not only for this line.
For blockforas a whole:
At this point you are modifying the original Imp, I think it would be better to change the code to the following:
for i := range request.Imp {
// Copy the entire Imp object
impCopy := request.Imp[i]
if impCopy.Banner != nil {
bannerCopy := *impCopy.Banner
if (bannerCopy.W == nil || bannerCopy.H == nil || *bannerCopy.W == 0 || *bannerCopy.H == 0) && len(bannerCopy.Format) > 0 {
first := bannerCopy.Format[0]
bannerCopy.W = &first.W
bannerCopy.H = &first.H
}
impCopy.Banner = &bannerCopy
}
if impCopy.BidFloorCur == "" && impCopy.BidFloor > 0 {
impCopy.BidFloorCur = "USD"
}
requestCopy.Imp[i] = impCopy
}
"USD"is hardcoded value. Extract to named constants:
const (
defaultBidFloorCurrency = "USD"
)
|
|
||
| func (a *adapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *adapters.ExtraRequestInfo) ([]*adapters.RequestData, []error) { | ||
| var errs []error | ||
| if len(request.Imp) == 0 { |
Contributor
There was a problem hiding this comment.
Prebid Server's core exchange logic already validates that requests have at least one valid impression before calling adapter's MakeRequests.
The exchange will never pass a request with zero impressions to an adapter
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding iqiyi prebid server adapter