Closed btorresgil closed 6 years ago
Suggestion 1 Update:
Decided to retire poll_all
method due to the following weaknesses::
polls
poll
results in a suboptimal manner which could lead to memory exhaustion if the query is large enough.timeout
might address part of the first bullet it's not enough to justify its existence.iter_poll
and xpoll
offer a much more optimal solutions that should satisfy the vast majority of use-cases. Said another way, poll_all
doesn't address a specific use-case that couldn't be handled using iter_poll
or xpoll
.Expect poll_all
to be removed in the next minor release.
Description
Currently the
poll_all()
anditer_poll()
methods loop through all the pages of results until the last page is found. There are two suggestions for improvements to allow better control of this loop:Suggestion 1:
Add a
timeout=
keyword argument with a sensible default. This would act as a timeout for the whole operation which allows a developer to restrict how long a poll job can run. This can be done currently withiter_poll()
where the developer manages their own timeout, but we can make it easier by adding the argument.Suggestion 2:
Currently, the loop uses
else
to assume the query status isRUNNING
. This could result in an infinite loop if the API changes or fails. To better future-proof the library, check forRUNNING
explicitly and useelse
to raise an exception such asUnexpected query status: {}
.