Silvermob: Use mtype and add global host #3936
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| errs = append(errs, err) | ||
| } else { | ||
| bidResponse.Bids = append(bidResponse.Bids, &adapters.TypedBid{ | ||
| Bid: &bid, |
There was a problem hiding this comment.
Found incorrect assignment made to Bid. bid variable receives a new value in each iteration of range loop. Assigning the address of bid (&bid) to Bid will result in a pointer that always points to the same memory address with the value of the last iteration. This can lead to unexpected behavior or incorrect results. Refer https://go.dev/play/p/9ZS1f-5h4qS
Consider using an index variable in the seatBids.Bid loop as shown below
for _, seatBid := range response.SeatBid {
for i := range seatBids.Bid {
...
responseBid := &adapters.TypedBid{
Bid: &seatBids.Bid[i],
...
}
...
...
}
}
Code coverage summaryNote:
silvermobRefer here for heat map coverage report |
Code coverage summaryNote:
silvermobRefer here for heat map coverage report |
bsardo
changed the title
|
Hi @freemmy, 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 |
Merge upstream/master and update imports to v3
Code coverage summaryNote:
silvermobRefer here for heat map coverage report |
Done, could you please review? Thanks in advance |
Hello @bsardo, could you let us know when the review might be completed? Our pull request has been pending for 2 months now, and during this time, our adapter has an issue where bid requests containing both video and banner are always treated as video. This prevents us from serving banners in such cases. As a result, OpenWrap isn’t functioning properly with 300x250 and 320x480 ad sizes. |
|
Hi @freemmy, sorry for the delay. We've been overloaded. I'll give this a look today. |
adapters/silvermob/silvermob.go
Outdated
| case openrtb2.MarkupAudio: | ||
| return openrtb_ext.BidTypeAudio, nil | ||
| default: | ||
| return "", fmt.Errorf("Unable to fetch mediaType for imp: %s", bid.ImpID) |
There was a problem hiding this comment.
According to your YAML file you don't support audio which leads me to believe this case statement should be removed so that an audio mtype results in an error.
There was a problem hiding this comment.
Actually we do support audio, but I've removed for now this case statement. We will back it later when add tests and update yaml
Code coverage summaryNote:
silvermobRefer here for heat map coverage report |
Metdia type detection by mtype rather than by imp object
global host for geo-based routing added