sentinel-hub / sentinelhub-py

Download and process satellite imagery in Python using Sentinel Hub services.
http://sentinelhub-py.readthedocs.io/en/latest/
MIT License
798 stars 243 forks source link

Add configuration to limit rate limit retries #527

Closed Regan-Koopmans closed 2 months ago

Regan-Koopmans commented 2 months ago

At the moment the default behaviour is to retry requests indefinitely when rate limits are reached on the server side. For some use-cases it would be helpful to limit this behaviour, and allow the calling code to handle this with some backoff mechanism. In particular, if multiple processes are submitting requests it does not make sense for them to compete over the finite rate limit.

To achieve this I have added a new config called max_rate_limit_retries, which is None by default. If this configuration has a numeric value we will raise an OutOfRequestsException once the number of download attempts has exceeded the configuration value. I am open to changing the naming if necessary.

zigaLuksic commented 2 months ago

I switched the base to develop I also see that there are some issues with the tests ATM, i'll investigate

zigaLuksic commented 2 months ago

@Regan-Koopmans after the switch to develop it is a bit harder to see what the actual changes are. Could you perhaps rebase your changes to develop?

zigaLuksic commented 2 months ago

The pylint error should be ignored (since it's a configuration class anyway) You can change L27 in sentinelhub/config.py to class _SHConfig: # pylint: disable=too-many-instance-attributes

Regan-Koopmans commented 2 months ago

Thanks @zigaLuksic, I have rebased and disabled that pylint check 👍

Regan-Koopmans commented 2 months ago

Do you have any preferences on when this should be released?

We have a soft launch next week. In an ideal world we could incorporate this change before then, but we would not need to block on this, as it is considered a "nice to have". We can also already develop against this change being released, since we know what exception to expect.

zigaLuksic commented 2 months ago

Do you have any preferences on when this should be released?

We have a soft launch next week. In an ideal world we could incorporate this change before then, but we would not need to block on this, as it is considered a "nice to have". We can also already develop against this change being released, since we know what exception to expect.

We'll try to find time to release this week, but no promises :)

Regan-Koopmans commented 2 months ago

Also thanks for the quick turnaround time with reviews!