Skip to content

Replace paginated with iterator #1845

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kunalsz
Copy link

@kunalsz kunalsz commented Apr 2, 2025

In reference to the issue #1798

  • Replaced the custom paginated by using the inbuilt .iterator() method
  • Also removed the paginated method from the BaseQuerySet

@keshav-space Looking forward to your insights

@pombredanne
Copy link
Member

@kunalsz Thanks! Did you make any quick measurement before and after to validate that this is more efficient in ram/cpu/number of queries? Would there be a small unit test that could check this?

@kunalsz
Copy link
Author

kunalsz commented Apr 3, 2025

Hi @pombredanne , I don't know how to test this out like @keshav-space did in the respective issue, if he could share the code/test file which he used to compare both the functions then I'll happily add the test results too.

@keshav-space
Copy link
Member

Hi @pombredanne , I don't know how to test this out like @keshav-space did in the respective issue, if he could share the code/test file which he used to compare both the functions then I'll happily add the test results too.

@kunalsz a very simple way to evaluate the performance would be to start VulnerableCode in Docker and run a large pipeline, like the GitHub importer pipeline, and collect the logs. Do this process twice, once with the old paginated and then with the built-in iterator and compare the logs. Make sure to reset the database between the two runs.

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.

3 participants