Skip to content
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

Michao Bid Adapter: Initial release #12507

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Conversation

hogekai
Copy link

@hogekai hogekai commented Nov 26, 2024

Type of change

  • Bugfix

  • Feature

  • New bidder adapter

  • Updated bidder adapter

  • Code style update (formatting, local variables)

  • Refactoring (no functional changes, no api changes)

  • Build related changes

  • CI related changes

  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

  • Other

Description of change

  • contact email of the adapter’s maintainer: [email protected]
  • test parameters for validating bids:
{
  bidder: 'michao',
  params: {
    placement: 1,
    site: 1
  }
}

(Turn on debug mode)

Other information

Nice to meet you!
Thanks for the great solution.
We have developed a new adapter and noticed that the outstream video ad (in renderer) is not well maintained around it.
Prebid-outstream is not working in its current version and the examples on Prebid.org are not well maintained.

So we thought it would be useful to have a library dedicated to in-renderer integration without the need for hosting, so we developed one. (https://github.com/hogekai/in-renderer-js)

The UI has been adjusted for outstream video ads, and we plan to develop the README and other related documentation in the future.
(If you want to check how it works, you can check the test ads by using the debug mode and test parameters, as they are included in this adapter.)

If you would like, please let us know what you think and what steps you would like to take to get the in-renderer integration area in place.

(Perhaps this is not the right place to ask. If I'm wrong, please let me know where I should ask. I would appreciate it if you would consider that I am not as familiar with Prebid.js as the maintainer).

@hogekai
Copy link
Author

hogekai commented Dec 13, 2024

Hi, regarding the proposed library, Muuki has provided support.
Thanks!

@hogekai hogekai requested a review from patmmccann January 1, 2025 19:30
@patmmccann
Copy link
Collaborator

This appears to add a lot of unhandled rejection messages to the circleci log

Is it possible to make it less noisy? And if not, could it be more explicit in where the logs messages are coming from?

Compare to master at:
https://app.circleci.com/pipelines/github/prebid/Prebid.js/24014/workflows/5d8fa6eb-8201-4d95-b324-2089793d5229/jobs/42427

}
}

const video = bid.mediaTypes?.video;
Copy link
Collaborator

@patmmccann patmmccann Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these type checks are not appropriate in your adapter, the type check should have already happened somewhere else. If not, let's add it in that place so we don't have every adapter validating its input duplicatively

}

if (params?.badv) {
if (!isArray(params?.badv)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the following two params you must also support from ortb.

return [];
},

onBidBillable: function (bid) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting to see this getting defined

const bidRequest = context.bidRequests[0];
const openRTBBidRequest = buildRequest(imps, bidderRequest, context);
openRTBBidRequest.cur = [ENV.DEFAULT_CURRENCY];
openRTBBidRequest.test = config.getConfig('debug') ? 1 : 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a dangerous pattern, not all debug requests are test requests

const openRTBBidRequest = buildRequest(imps, bidderRequest, context);
openRTBBidRequest.cur = [ENV.DEFAULT_CURRENCY];
openRTBBidRequest.test = config.getConfig('debug') ? 1 : 0;
openRTBBidRequest.bcat = bidRequest.params?.bcat || [];
Copy link
Collaborator

@patmmccann patmmccann Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be defined on the ortb request object as well, am I reading correctly you only support it as a param?

);
}

return openRTBBidRequest;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your doc says you only support site and user, but you seem to support device and other parts of ortb as well?

@hogekai
Copy link
Author

hogekai commented Jan 7, 2025

It is unclear why this build fails. What should I do?

@hogekai hogekai requested a review from patmmccann January 7, 2025 05:03
@ChrisHuie
Copy link
Collaborator

It is unclear why this build fails. What should I do?

Just needed to be reran because it disconnected.

@hogekai
Copy link
Author

hogekai commented Jan 8, 2025

@ChrisHuie
okay, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants