-
Notifications
You must be signed in to change notification settings - Fork 224
OpenX: Determine bid type based on mtype #3811
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
OpenX: Determine bid type based on mtype #3811
Conversation
|
@CTMBNara could you please review this PR? |
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.
Remove all code related to the old logic.
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.
@CTMBNara We would like to keep the old logic and fallback to it when there is no mtype in the bid response.
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.
@gmiedlar-ox You can revert the commit at any time later. We don't keep "dead" code in our repository.
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.
@CTMBNara I don't think we have "dead" code here. In case bid.getMtype() is null or there is no case match, the "old" getBidType logic will be executed.
case null, default -> impIdToBidType.getOrDefault(bid.getImpid(), BidType.banner);
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.
@gmiedlar-ox Yes, you're right. Sorry for that, missed this part
🔧 Type of changes
✨ What's the context?
OpenX started to return mtype field for bids in bid response, so now bid type can be determined based on this property in OpenX bidder adapter.
🧠 Rationale behind the change
We want to determine bid type based on bid mtype from bid response, to be able to process multi-format requests.
🔎 New Bid Adapter Checklist
🧪 Test plan
How do you know the changes are safe to ship to production?
🏎 Quality check