rozzac90 / matchbook

Python wrapper for Matchbook API
MIT License
15 stars 10 forks source link

matchbook.utils.check_call_complete does not work for large responses #12

Open pjmcdermott opened 5 years ago

pjmcdermott commented 5 years ago

I found calls to APIClient.market_data.get_events() were not working when the API was returning a large number (> 500) events. The method call appears to get stuck in an infinite loop. Debugging showed that matchbook.utils.check_call_complete was the culprit.

Yesterday (23/11/18), a GET on https://www.matchbook.com/edge/rest/events returned:

<events-response>
<offset>0</offset>
<per-page>20</per-page>
<total>544</total>
<events>
<event>
....

Looking at check_call_complete:

def check_call_complete(response):
    return response.get('total', 0) < response.get('per-page', 20)

the test does not work because 544 < 500 (the default value of per_page for get_events()) always returns False, therefore the request loop for further pages continues indefinitely.

I believe the test should actually be:

def my_check_call_complete(response):
    return response.get('total', 0) < response.get('offset', 0) + response.get('per-page', 20)

or perhaps more straight-forwardly:

def my_check_call_complete(response):
    return  response.get('offset', 0) + response.get('per-page', 20) > response.get('total', 0)

i.e when the starting offset plus the current request's item count is greater than the total we are on the final page.

This issue was not apparent because the default value for per_page is 500 for the get_events() method. The number of live events on Matchbook is often less than 500.

rozzac90 commented 5 years ago

Pushed a fix to add the offset to this check, should no longer infinite loop in version 0.0.7