Skip to content

Comments

Bidder capabilities#651

Merged
dbemiller merged 6 commits intomasterfrom
bidder-capabilities
Aug 13, 2018
Merged

Bidder capabilities#651
dbemiller merged 6 commits intomasterfrom
bidder-capabilities

Conversation

@hhhjort
Copy link
Collaborator

@hhhjort hhhjort commented Aug 9, 2018

This enforces the bidder capabilities listed in bidder-info.yaml. Server logging has been added to the redundant checks within the adapter code so we can verify they are not needed and remove them with a later PR. Note that this does not cover legacy adapters that have been wrapped with the legacy->openRTB adapter.

cirla
cirla previously approved these changes Aug 9, 2018
return false
}

// Structs to handle parsed bidder info, so we aren't reparsing every request
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

dbemiller
dbemiller previously approved these changes Aug 10, 2018
Copy link
Contributor

@dbemiller dbemiller left a comment

Choose a reason for hiding this comment

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

looks good here too.

adapters/info.go Outdated
var allowedMediaTypes parsedSupports
if request.Site != nil {
if i.info.Capabilities.Site == nil {
if i.info.site.enabled == false {
Copy link
Contributor

Choose a reason for hiding this comment

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

fairly silly nitpick, but... if !i.info.site.enabled

v.AddConfigPath(".")
v.AddConfigPath("/etc/config")

func SetupViper(v *viper.Viper, filename string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Little puzzled... what happened here?

Does this go away if you merge master into the branch? I'd've sworn we already merged these changes elsewhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I am really puzzled as config/config.go was not part of the commit when I created it.

glog.Fatalf("Failed to create the bidder params validator. %v", err)
}

bidderInfos := adapters.ParseBidderInfos("./static/bidder-info", openrtb_ext.BidderList())
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be safe, but... please do build & run this once in a docker container locally, just to make sure the files & paths still work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the exact same line as from below, just moved up so exchange.NewExchange has access to it. Is there something about moving it that might break something?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, of course... no, you're fine then. Ignore me

@hhhjort hhhjort dismissed stale reviews from dbemiller and cirla via aa46775 August 10, 2018 13:52
@dbemiller dbemiller merged commit b4f8035 into master Aug 13, 2018
@dbemiller dbemiller deleted the bidder-capabilities branch August 13, 2018 14:20
@hhhjort hhhjort mentioned this pull request Aug 17, 2018
@dbemiller
Copy link
Contributor

Contributes to #555

katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this pull request Dec 1, 2020
* Sets a config file name so we can redirect to non-existent file for tests

* Better handling of tests

* fixes mismatched name

* Enables the bidder decorator to enforce the bidder-info yamls

* improves slightly a couple of if clauses
katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this pull request Dec 2, 2020
* Sets a config file name so we can redirect to non-existent file for tests

* Better handling of tests

* fixes mismatched name

* Enables the bidder decorator to enforce the bidder-info yamls

* improves slightly a couple of if clauses
katsuo5 pushed a commit to flux-dev-team/prebid-server-1 that referenced this pull request Dec 4, 2020
* Sets a config file name so we can redirect to non-existent file for tests

* Better handling of tests

* fixes mismatched name

* Enables the bidder decorator to enforce the bidder-info yamls

* improves slightly a couple of if clauses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants