Kraken API requests frequency exceeds standard rate limit

[wearymanateevedaknotstabooresisting]

The current request frequency has a too small period, resulting in the excess of the Kraken specified rate limit.

This leads to non-requested _get_price_kraken calls, which return 0 and seem to not be re-requested, thus leading to false calculations afterwards (an may be won't be re-queried in another run, since 0 is saved into the DB? Need confirmation, here).

2021-03-06 13:38:05,744 price_data   WARNING  Querying trades for #PAIR# at YYYY-mm-dd HH-MM-SS+T (offset=20m): Could not retrieve trades: ['EAPI:Rate limit exceeded']

[provinzio]

Oh, I thought raise_for_status (here) would catch the teapot/rate limit.

It looks like you are right.

#16 could fix this, if it's implemented for different exchanges (which would include Kraken)

[wearymanateevedaknotstabooresisting]

Calling the public endpoints at a frequency of 1 per second (or less) would remain within the rate limits, but exceeding this frequency could cause the calls to be rate limited. If the rate limits are reached, additional calls would be restricted for a few seconds (or possibly longer if calls continue to be made while the rate limits are active).

-Kraken, https://support.kraken.com/hc/en-us/articles/206548367-What-are-the-API-rate-limits-#1

A hot fix would be to increase the random uniform delay to a range between [1, 2] seconds (lower limit is currently 0.2 s).

[wearymanateevedaknotstabooresisting]

Oh, I thought raise_for_status (here) would catch the teapot/rate limit.
-@provinzio

Actually, Kraken does still send a status code in the 800s if the rate is limited. raise_for_status is only triggered with 400/500s codes for server/client errors (https://requests.readthedocs.io/en/master/user/quickstart/). Therefore, no exception will be raised.

Kraken also provides an error attribute, which would be filled with ['EAPI:Rate limit exceeded']. So we can explicitly check for the 803 status code or parse the error attribute.

[provinzio]

Kraken also provides an error attribute, which would be filled with ['EAPI:Rate limit exceeded']. So we can explicitly check for the 803 status code or parse the error attribute.

The code checks for the error attribute. It raises a warning and returns 0 as price, which is resolved unfortunately in this case.
We might want to adjust the error handling there.

A hot fix would be to increase the random uniform delay to a range between [1, 2] seconds (lower limit is currently 0.2 s).

Hotfixing with an increased delay sounds reasonable for the moment.

[wearymanateevedaknotstabooresisting]

I re-run with 1 second as the lower duration value and still got some rate limiting. I re-run with 3 seconds which turned out fine, just took a while ... However, in consequence the tax burden significantly changed from big positive to big negative for an example year.

Therefore, I will test-run again and implement an actual raise of a runtime exception, since the results are heavily affected.

[provinzio]

Yeah, missing out prices is never good. We should either recheck 0 prices on the next run or raise an exception if no price could be fetched and abort the evaluation altogether. What's your opinion on this?

I think its possible to get coins which not exist in the market right now. For example from stacking.

So the best we can do is, raise a (detailed) warning, notifying the user, about this kind of problem.

[wearymanateevedaknotstabooresisting]

Normally, I do not like interrupting a program in any way but instead offering a detailed explanation as an error log message and hinting towards criticality with action needed. But a clear interruption is needed if it is a valid pairs with otherwise valid prices. So I could imagine a dialog about exit, retrying, manual price input or ignoring - ignoring is wrong in any case, imo, so this option is only for manual correction after the run (we should not corrupt the DB for later runs, though).

Regarding the non-existing coins, I assumed in my previous program the following: I traded/did some action with a certain coin, so the coin was indeed valid of the exchange and should at least offer information about it on that exchange. For Kraken, all (even very new coins), are directly listed through the Assets and Pairs endpoints. Staked coins are noted with suffix .S so the prices are always retrievable through its asset, e.g. prefix XTZ for XTZ.S tezos staking, trading pairs, e.g. XTZEUR.

So the question emerges if a user should process any coin which does not exist in the market / supported exchanges through CoinTaxman at all? Like you already specified in the README, CoinTaxman does not support any coin on any exchange but the use-cases and exchanges which are of user's (and developer's) interest. There are just too many edge cases if there is the aspiration to support everything.

[wearymanateevedaknotstabooresisting]

I tried several time intervals, at some point I got the rate limiting, even when the interval was big (> 5 s per request) - this size of time interval is non-feasible, imo). Interestingly, the next request goes just fine. Therefore, I think that simple retries are so much better than bigly increasing the delay time.

[wearymanateevedaknotstabooresisting]

Retrying works wonders, at least for Kraken API - added with 862ddd8