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

floorProvider can come from data obj now #5559

Merged
merged 1 commit into from
Aug 4, 2020
Merged

Conversation

robertrmartinez
Copy link
Collaborator

@robertrmartinez robertrmartinez commented Jul 30, 2020

Adding to #5538
Type of change

  • Feature

Description of change

About 1 hour after merging #5538 @bszekely1 realized we also want the floorProvider to be able to be set at the data level of floor configs.

This way, a fetch can indicate who the floorProvider is as well as just the top level.

It works similarly as skipRate, being the data level field takes precedence over the top level setConfig field.

@robertrmartinez
Copy link
Collaborator Author

@diDNA-matt Small addition to your code update. Take a look if you can please!

@diDNA-matt
Copy link
Contributor

@diDNA-matt Small addition to your code update. Take a look if you can please!

That makes a lot of sense, and looks good. Are there any issues you can think of where an analytics adapter may be expecting one vs the other? I guess that would be on the analytics adapter to handle.

@robertrmartinez
Copy link
Collaborator Author

@diDNA-matt

So the main reason we wanted to have the floorProvider is so Analytics adapters can make a decision on what they want to do when they do not know who is making the floors.

for instance, in the Rubicon analytics adapter, our floor provider team only wants to learn off of their own floor data. So we will be filtering out any floor data that comes in and is not from rubicon.

Other analytics adapters may find it beneficial to just pass in floor data regardless, and just pass through the provider name to their data processing.

So you are correct, it is dependent on what the analytics adapter's use cases are for reading floor information.

@bszekely1
Copy link

Adding PR for docs: prebid/prebid.github.io#2169

@diDNA-matt
Copy link
Contributor

@robertrmartinez

Thanks for the example and explanation 👍

@msm0504 msm0504 added LGTM needs 2nd review Core module updates require two approvals from the core team and removed needs review LGTM labels Aug 3, 2020
@robertrmartinez
Copy link
Collaborator Author

Hi @Fawke need a second review for a small addition to the Price Floors Module if you have a quick minute!

Thanks!

@msm0504 msm0504 merged commit 609da19 into master Aug 4, 2020
@robertrmartinez robertrmartinez deleted the floor_provider_data_level branch September 21, 2020 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants