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

Port currency fix from PBS-Java #1067

Closed
bretg opened this issue Oct 10, 2019 · 6 comments · Fixed by #1097
Closed

Port currency fix from PBS-Java #1067

bretg opened this issue Oct 10, 2019 · 6 comments · Fixed by #1097
Assignees
Labels

Comments

@bretg
Copy link
Contributor

bretg commented Oct 10, 2019

There's a currency conversion issue in PBS. We've fixed this in PBS-Java with prebid/prebid-server-java#457

Here are the requirements and some test cases:

  1. the client only ever specifies one entry in the cur array: the one expected by their adServer
    If there are more than one cur specified, use the first and warn.

  2. This cur value must be sent to each adapter

  3. PBS must try to convert bid responses to the first currency in the array

  4. if PBS can't convert a bid, then it should error with "unable to covert bid currency XYZ to desired ad server currency ABC."

Test cases:

  1. ad-server-currency is USD. (whether defined in the cur field or in the config is the same behavior)
  2. bidderA bids are in EUR. they get converted to USD
  3. bidderB bids are in GBP. they get converted to USD
  4. bidderC bids are in USD. no conversion needed
@benjaminch
Copy link
Contributor

Hey @bretg !

Sorry about that but would it be possible to explain what are the client and the core server here?

Cheers !

@bretg
Copy link
Contributor Author

bretg commented Oct 16, 2019

  • client: browser or app
  • core server: the part of PBS that receives the request, does the DB lookup, and prepares the openrtb for the bidder modules. i.e. everything that's not a bidder module

@stale
Copy link

stale bot commented Oct 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 23, 2019
@hhhjort hhhjort removed the stale label Oct 23, 2019
@hhhjort hhhjort self-assigned this Oct 28, 2019
@hhhjort
Copy link
Collaborator

hhhjort commented Oct 28, 2019

@bretg : Do we see a real need to provide the option to be a string rather than an array? In the Go version, it would take significant number of cycles to check for the possible existence of singe string. It also breaks the openrtb 2.5 spec to provide a single string.

@bretg
Copy link
Contributor Author

bretg commented Oct 30, 2019

@hhhjort objects to flexibility in accepting a string value for cur outside of the OpenRTB standard.

The core server should allow the cur value to be either a single value or an array. It should convert a string into an array of one string.

I don't know the history for this other than I have a few test files incorrectly formatted. Don't see evidence that the Prebid Server Bid adapter does the wrong thing, so I'm ok with PBS-Go being firm here.

hhhjort added a commit that referenced this issue Nov 1, 2019
@stale
Copy link

stale bot commented Nov 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 6, 2019
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 a pull request may close this issue.

3 participants