Added new forex provider to fix "Currency Rates Source Not Ready" - #127
Added new forex provider to fix "Currency Rates Source Not Ready"#127ShanePadellaro wants to merge 2 commits into
Conversation
Sorry, something went wrong.
| return date_str | ||
|
|
||
| def _decode_rates(self, response, use_decimal=False, date_str=None): | ||
| def _decode_rates(self, response, use_decimal=False, date_str=None,base_cur=None): |
There was a problem hiding this comment.
Missing sapce before ,
Sorry, something went wrong.
There was a problem hiding this comment.
fixed
Sorry, something went wrong.
| response = requests.get(source_url, params=payload) | ||
| if response.status_code == 200: | ||
| rates = self._decode_rates(response, date_str=date_str) | ||
| rates = self._decode_rates(response,date_str=date_str,base_cur=base_cur) |
There was a problem hiding this comment.
Missing sapce before ,
Sorry, something went wrong.
There was a problem hiding this comment.
fixed
Sorry, something went wrong.
|
|
||
| def _source_url(self): | ||
| return "https://theforexapi.com/api/" | ||
| return "https://api.exchangerate.host/" |
There was a problem hiding this comment.
why not allow specying source url as parameter?
Sorry, something went wrong.
There was a problem hiding this comment.
Most apis are not going to be this similar in terms of api surface. This one is 95% there so still requires minor changes. I don't think having the provider url as a parameter makes sense unless theforexapi.com comes back online again.
If supporting multiple apis is required there needs to be some refactoring to abstract the api provider from the currency class.
Sorry, something went wrong.
There was a problem hiding this comment.
It makes sense, you can host your own api since theforexapi is open source, there should be an option to override the url...
Sorry, something went wrong.
Added forex provider (exchangerate.host) which is almost a 1to1 replacement for the old theforexapi.com.
Added a check to throw if a currency is not available since new provider always returns 200
All unit tests for converter.py are working a part from test_with_valid_currency_code which looks to be broken because the included json file doesn't contain the expected symbol.