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

Add all currencies to JSON schema #126

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Conversation

dlackty
Copy link
Contributor

@dlackty dlackty commented Feb 11, 2023

I pull the currency list from Google Ad Managers's setting UI.

@dlackty
Copy link
Contributor Author

dlackty commented Sep 26, 2023

@dshore any chance to get this merged? this should be a minor change but help lots of international developers.

@dshore
Copy link
Collaborator

dshore commented Sep 28, 2023

@dlackty See: https://docs.prebid.org/dev-docs/modules/currency.html

Prebid.org’s currency file
Prebid.org hosts a conversion file at cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json. The currencies currently supported are: AUD, BGN, BRL, CAD, CHF, CNY, CZK, DKK, EUR, GBP, HKD, HRK, HUF, IDR, ILS, INR, ISK, JPY, KRW, MXN, MYR, NOK, NZD, PHP, PLN, RON, RUB, SEK, SGD, THB, TRY, USD, ZAR.

This is the list in the schema validation file.

I am going to close this PR but I will allow CLI option to provide your own schema.

@dshore dshore closed this Sep 28, 2023
@dlackty
Copy link
Contributor Author

dlackty commented Sep 29, 2023

Hi @dshore,

I'm aware of the currency module and previously attempted to add TWD to the list of currencies in the current file a few years ago (see pull request). However, according to @bretg's comment at the time:

Unfortunately, we're limited to the currencies available from our sources. If you find a reliable source that includes TWD, we'll consider using it. Meanwhile, you'll need to generate your own file as described at http://prebid.org/dev-docs/modules/currency.html.

As a result, we started maintaining our own version of the currency file for Prebid, specifically for the Taiwan market. Many publishers and adtech vendors are using this file (link to repository).

For this case, it seems that for those unsupported currencies, we need to use customization everywhere from currency module file to line-item-manager. It actually adds so many obstacles for people adopting Prebid.js here but I understood your situation.

However, I still believe that the line-item-manager project should support all the currencies available in Google Ad Manager. Some publishers use only local bidders that place bids in TWD (e.g., clickforce/appier) and send the auction results to their GAM in TWD without requiring any currency conversions.

In such cases, they are not using currency conversions and they're still unable to use line-item-manager without a custom schema. I believe this is a common and valid use case.

I understand that this may be a minor topic, but it holds significant importance for those countries and currencies that are currently unsupported.

Thank you for taking the time to read and consider this matter.

@bretg
Copy link

bretg commented Sep 29, 2023

@dshore - FWIW, I'm in favor of allowing the line item manager to recognize currencies not on the main Prebid conversion file.

allow CLI option to provide your own schema

What does 'schema' mean here?

@bretg bretg reopened this Sep 29, 2023
@dshore
Copy link
Collaborator

dshore commented Sep 29, 2023

@bretg I was about to release the following: #138

@dshore
Copy link
Collaborator

dshore commented Sep 29, 2023

@bretg The new CLI setting easily allows for a custom validation list. But if we want to merge all these currencies into the default I am fine with that as well, just seems we are creating a Prebid tool and not an explicit GAM tool.

@bretg
Copy link

bretg commented Sep 29, 2023

Thanks @dshore .

@dlackty - will #138 meet your needs?

not an explicit GAM tool

This tool only works with GAM, right?

@dshore
Copy link
Collaborator

dshore commented Sep 29, 2023

@bretg Yes it only works with GAM, but code was structured to add other management services

@dshore
Copy link
Collaborator

dshore commented Sep 29, 2023

@dlackty You can try #138 as a dev release:

$ pip uninstall line_item_manager
$ pip install https://github.com/prebid/line-item-manager/archive/master.zip

$ line_item_manager show schema > my_schema.yml
# edit my_schema.yml; e.g. use a custom currency list
$ line_item_manager create my_config.yml --single-order --schema my_schema.yml

@dlackty
Copy link
Contributor Author

dlackty commented Oct 1, 2023

@dshore Yes, I can confirm that this works. Thank you!

I understand the motivation to make it a Prebid tool but not a GAM tool.

Prebid.js now have lots of bidders that supports various currencies, and some of the currencies are not supported by this tool.

However, publishers should be able to use these bidders with this tool, without currencies conversions, if their ad server does support bidder's currency, right?

@dshore
Copy link
Collaborator

dshore commented Oct 1, 2023

@bretg Looks like @dlackty Is successfully using the custom schema support. But to close out this issue can we agree on what the default supported list of currencies should be for this tool? The list exists in the schema to support user validation, which can be helpful in finding errors in configuration.

@bretg
Copy link

bretg commented Oct 2, 2023

I think the tool should support whatever the ad server supports. But as long as there's a way to make it work and it's reasonably documented, all good.

@dshore
Copy link
Collaborator

dshore commented Oct 2, 2023

@bretg That is the change I will make, i.e. I will move the validation to reflect it is an ad server constraint, and not a prebid constraint.

@dshore dshore merged commit 1f4051b into prebid:master Oct 2, 2023
5 checks passed
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