-
Notifications
You must be signed in to change notification settings - Fork 744
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
Fix few adapter aliases bug #3055
Conversation
We no longer require to maintain aliasBidderNames in separate slice. Instead, we will be needing aliasBidderToParentBidder map to maintain so that we can use it to get parent schema in later commit.
This refactor makes it easier to test the NewBidderParamsValidator function. Also adds the test cases.
This is required to fix the issue where we read bidderInfo first, check if it is an alias and then SetAliasBidderName. So that normalizeBidderName returns the dynamically added alias names.
# Conflicts: # config/bidderinfo.go # openrtb_ext/bidders.go # openrtb_ext/bidders_test.go
config/bidderinfo.go
Outdated
//required for CoreBidderNames function to also return aliasBiddernames | ||
openrtb_ext.SetAliasBidderName(openrtb_ext.BidderName(string(bidderName))) | ||
if err := openrtb_ext.SetAliasBidderName(openrtb_ext.BidderName(string(bidderName)), openrtb_ext.BidderName(aliasBidderInfo.AliasOf)); err != nil { |
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.
bidderName
need be typecasted to string as it is already a string. We have done these changes in this commit
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 should be done in the merge master
commit.
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.
Looks good to me, added minor comment and question
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.
Lgtm!
@@ -552,11 +556,11 @@ var bidderNameLookup = func() map[string]BidderName { | |||
lookup[bidderNameLower] = name | |||
} | |||
return lookup | |||
}() | |||
} |
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.
mostly for my understanding, any specific reason to make this change?
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.
because of - #3055 (comment)
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.
some suggestions
This PR makes the following changes -
/bidders/params
endpoint to also return aliases bidder params.