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

fix(iterators): prevent recursion issue #542

Merged
merged 5 commits into from
Dec 13, 2021
Merged

fix(iterators): prevent recursion issue #542

merged 5 commits into from
Dec 13, 2021

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Dec 3, 2021

Q A
Bug fix? yes
New feature? no
BC breaks? no
Related Issue Support ticket
Need Doc update yes

Describe your change

if self._raw_response["nbHits"] < self._data["hitsPerPage"]: was never validated since nbHits does not represent the number of returned hits but the total of hits.

We now compute nbHits on the len(hits).

What problem is this fixing?

Recursion was never ending with iterations > 1000 hitsPerPage.

Reproduction

I've set an index (python_repro_rules) with 3k+ rules on this app (QPBQ67WNIG)

from algoliasearch.search_client import SearchClient, SearchConfig

config = SearchConfig('QPBQ67WNIG', 'SEARCH_API_KEY')
client = SearchClient.create_with_config(config)
index = client.init_index('python_repro_rules')

i = 0
for rule in index.browse_rules():
    print('called in main', i)
    i += 1

@shortcuts shortcuts marked this pull request as ready for review December 3, 2021 10:52
@shortcuts shortcuts self-assigned this Dec 3, 2021
@shortcuts shortcuts requested a review from tkrugg December 6, 2021 12:44
import json
import requests

from requests.models import Response

from algoliasearch.exceptions import RequestException, ObjectNotFoundException
from algoliasearch.responses import MultipleResponse
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might be able to use the response object from here instead from requests.models

@shortcuts shortcuts merged commit 1aa719a into master Dec 13, 2021
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