-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 for #4047 #4063
Fix for #4047 #4063
Conversation
…xcept instream video and banner.
Build failure appears to be due to an error when running RealVu Analytics Adapter tests. |
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.
@dpapworth-qc
I took a look through the changes, and they seem to be good.
There was one thing I noted as a potential thought, but not strictly needed. Let me know what you think about it.
Thanks!
} else if (bid.mediaTypes.banner) { | ||
imp = makeBannerImp(bid); | ||
} else { | ||
// Unsupported mediaType |
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.
Do you want to throw a warning/info message message here to note that an unsupported bid type was ignored by the adapter (if only for future diagnostic information)?
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.
👍 done
@dpapworth-qc As an update, I reran the circleci tests. They should be good. The current failure (where the job seemed to stall) was fixed in a recent update to master. If you resync your branch and get the update; it should resolve the issue. |
@jsnellbaker i've added a log message for unsupported media types, and i've merged the current master branch. |
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.
Thanks for making the updates.
* Changed isBidRequestValid to reject bid requests missing required parameters. * Filter outstream video bid requests. * Improved detection of different media types. Ignore all media types except instream video and banner. * Added test to confirm behaviour with multi-format request. * Removed unnecessary change to bid sizes. * Removed unused imports and tests. * Added log message for unsupported media types.
Type of change
Description of change
Resolves #4047.
Change isBidRequestValid to reject bid requests missing required parameters, rather than unsupported media types.
Filter unsupported media types when building requests.
Add test for multi-format ad units to verify expected behaviour.
Other information