kipcole9/money

Exchange rates service is now required to use Money.sum in 5.16.0

Closed this issue · 5 comments

haste commented

This is due to the change of default rates made in b49103f. After this commit we have to stub rates when calling Money.sum when the exchange rate service isn't used:

iex(1)> Money.sum([Money.new(:NOK, 100)], %{})
{:ok, Money.new(:NOK, "100")}

In version 5.15.4:

iex(1)> Money.sum([Money.new(:NOK, 100)])
{:ok, Money.new(:NOK, "100")}

In version 5.16.0:

iex(1)> Money.sum([Money.new(:NOK, 100)])
{:error,
 {Money.ExchangeRateError, "Exchange Rates retrieval process is not running"}}
kipcole9 commented

Sorry for the inconvenience. I've now published ex_money version 5.16.1 with the following changelog entry:

Bug Fixes

  • Money.sum/2 default exchange rates is always %{} even if the exchange rate server is not running. Thanks to @haste for the report. Closes #168.

Thanks for the report and your patience.

oliver-kriska commented

Hi, I think there is another smaller issue after these fixes:

04:53:15.781 [warning] Could not retrieve latest exchange rates: "Open Exchange Rates app_id is not configured.  Rates are not retrieved."
warning: the log level :warn is deprecated, use :warning instead
  (logger 1.16.2) lib/logger.ex:1165: Logger.elixir_level_to_erlang_level/1
  (logger 1.16.2) lib/logger.ex:898: Logger.__should_log__/2
  (ex_money 5.16.1) lib/money/exchange_rates/exchange_rates_retriever.ex:464: Money.ExchangeRates.Retriever.log/3
  (ex_money 5.16.1) lib/money/exchange_rates/exchange_rates_retriever.ex:370: Money.ExchangeRates.Retriever.retrieve_latest_rates/1
  (ex_money 5.16.1) lib/money/exchange_rates/exchange_rates_retriever.ex:297: Money.ExchangeRates.Retriever.handle_call/3
  (stdlib 5.2.2) gen_server.erl:1131: :gen_server.try_handle_call/4
kipcole9 commented

Would you let me know how you're getting the logger warning? This comes from the config/dev.exs configuration on the library - which is never used when ex_money is a dependency.

I'm surprised you're seeing it, unless of course you are configuring ex_money with log_failure: :warn instead of log_failure: :warning. (I've fixed the readme documentation about the setting).

Or are you saying you're seeing this issue with Money.sum/2? Can you show some steps, and your ex_money configuration, to reproduce?

kipcole9 commented

Or are you saying the logger message shouldn't be issued at all when Money.sum/2 is called?

oliver-kriska commented

Sorry for delayed answer. I think there are 2 things:

  1. warning about app_id
  2. :warn is deprecated

I'm not aware of anything in our configuration connected with these two things. You can see also in documentation that
https://hexdocs.pm/ex_money/readme.html

log_failure defines the log level at which api retrieval errors are logged. The default is :warn.

So it's legit warning which is not connected with this main Money.sum issue. But it's strange that we started to seeing it after recent upgrade.