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.

Sign up for GitHub

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

Jump to bottom

Paginate backwards for stargazers #121

Closed
wants to merge 2 commits into from
Closed

Paginate backwards for stargazers #121

wants to merge 2 commits into from

Conversation

ericboucher
Copy link
Contributor

Temporary solution to avoid exploding API quotas because the stargazers stream does not provide "since" or "sort/direction" parameters

To mitigate this, we traverse backwards and try to exit early.

WARNING - this does not work for large repositories such as facebook/react, see #120

@ericboucher ericboucher requested a review from laurentS May 17, 2022 17:42
laurentS
laurentS approved these changes May 17, 2022
View reviewed changes
Copy link
Contributor

@laurentS laurentS left a comment

Choose a reason for hiding this comment

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

So this PR tries to emulate a since param on the endpoint? Are the results actually ordered by starred_at? What happens on repos with lots of stars? Does it just crash?

tap_github/repository_streams.py Outdated Show resolved Hide resolved
@ericboucher ericboucher mentioned this pull request May 18, 2022
Merged
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ericboucher
Copy link
Contributor Author

Yes for large repositories it basically gets one page of data and then exists if there is a since parameter.
If there is no since parameter, it will do the same thing as before -> get all the data it can, which is not everything.

Eg for faceook/react we would get 40k out of 180k stargazers.

Note that this PR will probably be closed / useless if we move forward with #123

@ericboucher
Copy link
Contributor Author

Less useful thanks to #120

@ericboucher ericboucher deleted the fix-stargazers-pagination branch November 22, 2022 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurentS laurentS laurentS approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ericboucher @laurentS