search

leoschweizer / moneymoney-coinbase-pro

Inofficial MoneyMoney extension to list Coinbase Pro balances as securities
MIT License
6 stars 4 forks source link

workaround: hardcode removed XRP support. #13

Closed Duffycola closed 2 years ago

Duffycola commented 2 years ago

Trading with XRP is paused, but it is still listed on coinbase. Hence the coin is available but the subsequent request queryExchangeRate products/"XRP"/ticker fails. This is a lazy workaround, one could investigate this further. Also it would be nice if the requests could be run in parallel and if there were options to disable coins, because the fetch is really slow with a lot of coins.

leoschweizer commented 2 years ago

@Duffycola hi and thanks for the pull request. Regarding your suggestion of having a configurable set of currencies, I don't think it is practically possible. Moneymoney does not expose an API to have something like that, and having the configuration in code doesn't work for 99% of users (e.g. because it would also break the signature).

Regarding the fix for XRP: I'm not sure if it makes sense to single XRP out like this. There are a few cases when this value-in-fiat-currency lookup fails, e.g. also often when new currencies are added. I guess the proper way to do it would be to try/catch the exchange rate lookup and just don't return anything if it fails.

If you can provide a fix like that I would happily accept and get it signed again!

TeeJayR commented 2 years ago

@Duffycola, @leoschweizer: How about this approach to fix the errors with delisted currencies like XPR instead: https://github.com/leoschweizer/moneymoney-coinbase-pro/pull/14

Duffycola commented 2 years ago

if it works yeah cool. (i still do the hardcoded version locally because it also speeds up the refresh ignoring all coins i dont trade)

leoschweizer commented 2 years ago

closing because #14 should fix this issue in a more generic way