Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed offset adjustment from limit calculation #23

Conversation

alextarrell
Copy link
Contributor

@alextarrell alextarrell commented Sep 6, 2022

The number of result items should be capped to the item limit (either
the total number of items reported by the json or to some arbitrary max
value). Subtracting the offset value from that limit would effectively
cap the results early once the offset exceeded the number of results
retrieved.

Ex. Retrieving 60 total items with a 50 item limit and no max cap:

  1. item_limit is 60, offset is 0, there are 50 items in the response.
    Old effective cap is (60-0) 50, new effective cap is 50. We retrieve
    and add all 50 items to the results. Let's increase the offset and
    retrieve the next batch.
  2. item_limit is 60, offset is 50, there are 10 items in the response.
    Old effective cap is (60-50) 10, new effective cap is still 50. With
    the old effective cap we simply stop and return the 50 items we had
    already retrieved. With the new effective cap we retrieve and add all
    10 items to the results, then return with the full 60 results.

The number of result items should be capped to the item limit (either
the total number of items reported by the json or to some arbitrary max
value). Subtracting the offset value from that limit would effectively
cap the results early once the offset exceeded the number of results
retrieved.

Ex. Retrieving 60 total items with a 50 item limit and no max cap:
1. item_limit is 60, offset is 0, there are 50 items in the response.
Old effective cap is (60-0) 50, new effective cap is 50. We retrieve
and add all 50 items to the results. Let's increase the offset and
retrieve the next batch.
2. item_limit is 60, offset if 50, there are 10 items in the response.
Old effective cap is (60-50) 10, new effective cap is still 50. With
the old effective cap we simply stop and return the 50 items we had
already retrieved. With the new effective cap we retrieve and add all
10 items to the results, then return with the full 60 results.
@alextarrell
Copy link
Contributor Author

This fixes a regression in #17 caused by changes in this commit.

@MinisculeGirraffe MinisculeGirraffe merged commit 92f0230 into MinisculeGirraffe:main Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants