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

33Across: Added note for Prebid Server Adapter #2484

Merged
merged 32 commits into from
Dec 4, 2020
Merged

Conversation

curlyblueeagle
Copy link
Contributor

Based on feedback from CR in prebid/prebid-server#1557 (comment) added note to clarify how the Prebid Server Adapter will behave when multiple impressions are sent.

Added note to clarify Prebid Server Adapter
Copy link
Contributor

@bretg bretg left a comment

Choose a reason for hiding this comment

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

This isn't really an issue with your doc, but more like a topic of conversation for improving the 33across sever-side adapter

---

### Prebid Server Note:
The 33Across Adapter in Prebid Server does not support multi-impression requests, and will respond with a 400 level response if more than one impression is present in `$.imp[]` of the request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you saying that PBS fails the whole request if 33across gets an imp with both banner and video? That seems extreme. Shouldn't the 33across adapter just choose one of them and run with that? FWIW, that's what the Rubicon adapter does -- if it sees both banner and video, it chooses video.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not quite, what the note is saying is that our exchange server does not support SRA and will respond with a 400. We do support multi-format within the same imp however.

@bretg
Copy link
Contributor

bretg commented Nov 19, 2020

Please fix conflicts and I'll merge.

@curlyblueeagle
Copy link
Contributor Author

Please fix conflicts and I'll merge.

Done.

@curlyblueeagle
Copy link
Contributor Author

This isn't really an issue with your doc, but more like a topic of conversation for improving the 33across sever-side adapter

Not an issue with the doc true, we were asked to add this note as a clarification to Publishers while we are working to improve the server-side adapter.

@bretg
Copy link
Contributor

bretg commented Dec 4, 2020

Sorry for the delay. I set up a test request to confirm what happens here.

I have to say this is a major problem in your PBS adapter... it's job is to prepare data for your endpoint. If your endpoint can't handle SRA, then your PBS adapter isn't doing its job. It should be either:

  1. picking one of the several requests (e.g. the first) and at least trying to monetize that one
  2. split the imps into separate endpoint requests

@bretg bretg merged commit c95ce19 into prebid:master Dec 4, 2020
@curlyblueeagle
Copy link
Contributor Author

curlyblueeagle commented Dec 4, 2020

@bretg yes we have decided to go with approach No.2 and split imps. I have filed a PR prebid/prebid-server#1609 and am awaiting CR. I know this PR just got merged but I will be filing a follow up PR to remove the NOTE added in anticipation of our adapter's support for multi-imp requests.

osazos pushed a commit to onfocusio/prebid.github.io that referenced this pull request Jan 21, 2021
* Add publisher usage docs for 33Across

* Added 33Across to partners list

* Clarified example pubID

* Reinstated changes that got lost during rebase

* 33across adapter is GDPR compliant

* update docs

* added usp support

* We support Schain!

* Update for video support

* fixes to video params

* fixed context

* fixed instream params

* fix for multi-format

* Fixed Ad Unit titles

* protocols is required

* typo fix

* Added required bidder param

* removed duplicate video params

* formatting edits

* prebid-server adapter will support only single imp reqs

* Updated based in CR

* minor fix

* resolved merge conflicts

Co-authored-by: Gleb Glushtsov <[email protected]>
Co-authored-by: Gleb Glushtsov <[email protected]>
Co-authored-by: Aparna Hegde <[email protected]>
Co-authored-by: Aparna Hegde <[email protected]>
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