Rubicon: Initial ORTB 2.6 support #2536
Conversation
adapters/rubicon/rubicon.go
Outdated
| requestCopy, err := copystructure.Copy(request) | ||
| if err != nil { | ||
| return nil, []error{err} | ||
| } | ||
|
|
||
| requestDeepCopy := requestCopy.(*openrtb2.BidRequest) | ||
| err = openrtb_ext.ConvertUpTo26(&openrtb_ext.RequestWrapper{BidRequest: requestDeepCopy}) | ||
| if err != nil { | ||
| return nil, []error{err} | ||
| } | ||
|
|
||
| request = requestDeepCopy |
There was a problem hiding this comment.
@SyntaxNode Hi. Is there another best way to do conversion?
There was a problem hiding this comment.
@onkarvhanumante, can you answer please? ^^
|
@CTMBNara - please also downgrade user.consent to user.ext.consent. Thanks! |
SyntaxNode
assigned onkarvhanumante and Sonali-More-Xandr and unassigned VeronikaSolovei9 and guscarreon
SyntaxNode
requested review from
onkarvhanumante and
Sonali-More-Xandr
and removed request for
VeronikaSolovei9 and
guscarreon
| @@ -15,6 +15,7 @@ type ExtImpRubicon struct { | |||
| Visitor json.RawMessage `json:"visitor,omitempty"` | |||
| Video rubiconVideoParams `json:"video"` | |||
| Debug impExtRubiconDebug `json:"debug,omitempty"` | |||
| PChain string `json:"pchain,omitempty"` | |||
There was a problem hiding this comment.
should create docs PR to include description for this pchain bidder param (rubicon.md)
There was a problem hiding this comment.
I'm actually pushing to get rid of this deprecated param entirely, but went ahead and added it to the docs so this PR can be merged. We'll remove it as a separate effort if I get the ok . prebid/prebid.github.io#4397
adapters/rubicon/rubicon.go
Outdated
| requestCopy, err := copystructure.Copy(request) | ||
| if err != nil { | ||
| return nil, []error{err} | ||
| } |
There was a problem hiding this comment.
copy method used is not preferred here. How about simply assigning request to new var and using it
requestCopy := request
|
@CTMBNara any updates on this PR |
CTMBNara
requested review from
onkarvhanumante
and removed request for
Sonali-More-Xandr
@onkarvhanumante This problem is caused by To fix this, we need to go back to the previous solution or do a |
adapters/rubicon/rubicon.go
Outdated
| requestCopy := request | ||
|
|
||
| err := openrtb_ext.ConvertUpTo26(&openrtb_ext.RequestWrapper{BidRequest: requestCopy}) | ||
| if err != nil { | ||
| return nil, []error{err} | ||
| } | ||
|
|
||
| request = requestCopy | ||
|
|
There was a problem hiding this comment.
MakeRequest should not modify original request. This is a part of JSON test verification. It's easy to accidentally modify a property (or sub-object) in a shallow copy of an object such as any pointer in a bid request: Site *Site, App *App, Device *Device, User *User, etc.
This code is what is causing the race condition error.
ConvertUpTo26 sets User.Eids to original request and requestCopy because user is the same object in memory in both of them.
To fix this issue user should be copied from the original request and set back to requestCopy so requestCopy has its own user.
Insert this code after line 236:
if request.User != nil {
userCopy := *request.User
requestCopy.User = &userCopy
}This fixes race condition error
|
Thank you for the changes. Please keep in mind you will need to make a copy of other sub-objects if you will make changes in them. |
This was done a while ago in #2536
No description provided.