api._continue has a infinite loop bug

[aidanlister]

I added some debugging into the loop print(f"Getting {len(records)} / {total_records} records from page {page} from {url}"):

Getting 0 / None records from page 1 from https://api.knack.com/v1/objects/object_7/records/
Getting 954 / 6693 records from page 2 from https://api.knack.com/v1/objects/object_7/records/
Getting 1954 / 6693 records from page 3 from https://api.knack.com/v1/objects/object_7/records/
Getting 2954 / 6693 records from page 4 from https://api.knack.com/v1/objects/object_7/records/
Getting 3954 / 6693 records from page 5 from https://api.knack.com/v1/objects/object_7/records/
Getting 4954 / 6693 records from page 6 from https://api.knack.com/v1/objects/object_7/records/
Getting 5954 / 6693 records from page 7 from https://api.knack.com/v1/objects/object_7/records/
Getting 6647 / 6693 records from page 8 from https://api.knack.com/v1/objects/object_7/records/
Getting 6647 / 6693 records from page 9 from https://api.knack.com/v1/objects/object_7/records/
Getting 6647 / 6693 records from page 10 from https://api.knack.com/v1/objects/object_7/records/
Getting 6647 / 6693 records from page 11 from https://api.knack.com/v1/objects/object_7/records/
Getting 6647 / 6693 records from page 12 from https://api.knack.com/v1/objects/object_7/records/
Getting 6647 / 6693 records from page 13 from https://api.knack.com/v1/objects/object_7/records/
Getting 6647 / 6693 records from page 14 from https://api.knack.com/v1/objects/object_7/records/

It looks like even though we requested 1000 rows we only got 954. Should we add a failsafe like below:

while knackpy.api._continue(total_records, len(records), record_limit):
            ...
            fetched_records = res.json()["records"]
            records += fetched_records
            page += 1
            if len(fetched_records) == 0:  # If we're no longer fetching rows, break out of the loop
                break

[johnclary]

@aidanlister thanks for flagging. i remember once bumping into this and then forgetting the exact situation it happens. feel free to open a PR or i'll get around to it in the next week or so.

[johnclary]

@aidanlister if you're able to reproduce this API behavior you can test the patch by installing our dev package (v1.0.19).

[aidanlister]

Great, thanks!

[aidanlister]

Yep, that works. Thanks!

It'd be super handy if you could follow the python logging convention in that file, something like:

import logging

logger = logging.getLogger(__name__)
logger.debug('foo')

[johnclary]

Thanks for the TIL. Done. You'll need to test it by pulling this branch. The CI on the dev package can't handle another build. It's passing tests and will be included in v1.0.19 patch.

[aidanlister]

Unreal, thanks!

[johnclary]

Really appreciate you catching bugs. I'm actually going to bump this to v1.0.20 so we can build the dev package and run some tests on our own ETLs. This should definitely be backwards compatible but I don't want to break our own stuff. Will run it through the ringer.