[Feature] The code should retry fetching the data if the connection fails

[DragonWarrior15]

Currently the code does not support any kind of retry in order to call the api again to fetch the results. The following piece of code might work for this (this will be wrapped around the base api call so that modifications are needed at only one place)

success = False
for i in range(Constants.max_retries):
    if(success):
        break
    try:
        some_result = api_call()
        success = True
    except Exception as e:
        # wait few sec and continue
        time.sleep(Constants.sleep_time)
        continue

We could keep Constants.max_retries as 3 and Constants.sleep_time as 2.
I am not sure if this is the best way to do this, but if this is something useful, I can add it to the code with a PR. This is useful in cases where the API may not be reachable, or when any part of the package, or an external user is running a loop which calls the api multiple times.

[backtrackbaba]

There are actually two lines of thought on this feature: retry, rate limit, and other such things should be a part of the library or should be taken care of by the consumer.

Instead of using a for loop, an alternate approach could be by using some internal configurations in the comment above.

We are using requests library for our API calls and it internally uses urllib3.

In urllib3/requests, you can configure the number of retries you want, the response codes on which you want to retry, an exponential backoff factor, and so on. You can read more about the same here

In this case, I feel handling the retry could be a part of the library itself. Let me know you views

[DragonWarrior15]

This will take a while for me to get through. We also have to consider the changes to the api now, mentioned in the news here. Per IP there is a limit of 100 request per 5 minutes.

[DragonWarrior15]

All right, we should at least implement the retry based on the web post you have linked above. The default settings seem to make sense for our purpose.

Another thing linked with all of this is the following: the code in the base api can return an error as well. How should I handle it in the code here, essentially, what would be the right syntax to check that a dict (even empty) has been returned ? Will an isinstance check suffice ?

[backtrackbaba]

There are two changes linked to this feature: 1) Rate Limiting & 2) Raising Custom Exception

Rate Limiting is something that can be implemented right away as it's not a breaking change. On the other hand, Raising Custom Exceptions is something that would be a breaking change as the consumers of the current implementation would have to do some additional changes to take care of it.

Regarding changes to the code to check the right values would be a part of the changes involving custom exceptions. Over there, you could have some simple checks like a) Applying filters only if the list of centers is more than 1, b) Raising a custom exception wherever needed, and so on.

You can go ahead with the implementation for retry logic and we can push it to Pypi after the dependant changes are complete.

[DragonWarrior15]

Should 403 also be a part of the list where retry is called ? I guess a lot of people are facing 403 due to so many requests to the API.

[DragonWarrior15]

Bad idea to do this, ignore based on real world experience.