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

Paginated list reversed only gives the first page of results backwards #1136

Open
micahsteinberg opened this issue Jun 11, 2019 · 8 comments · May be fixed by #2929
Open

Paginated list reversed only gives the first page of results backwards #1136

micahsteinberg opened this issue Jun 11, 2019 · 8 comments · May be fixed by #2929

Comments

@micahsteinberg
Copy link

micahsteinberg commented Jun 11, 2019

When I do

g = Github("xxx")
repo = g.get_repo("xxx")

for comm in repo.get_commits(since=datetime).reversed:
    ....

it only goes through the most recent 30 results in reverse order.

I was able to fix this with

def reverse_github_results(paginated_list):
    for i in range(paginated_list.totalCount//30, -1, -1):
        page = paginated_list.get_page(i)
        page.reverse()
        for item in page:
            yield item

but this possibly runs a bit slower and I thought I should let you guys know

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 10, 2019
@stale stale bot closed this as completed Aug 17, 2019
@engn33r
Copy link

engn33r commented Jan 6, 2024

This was still an issue with v2.1.1 but the fix above worked for me 👍

It would be great if this could be fixed in the library itself, with an example showing reversed usage in the docs.

@EnricoMi
Copy link
Collaborator

EnricoMi commented Jan 6, 2024

I'd be surprised this does not work, there are some unit tests that cover this. Maybe test data have to be recorded again. Reopening, contribution welcome.

@EnricoMi EnricoMi reopened this Jan 6, 2024
@stale stale bot removed the stale label Jan 6, 2024
@etiennnr
Copy link

Can confirm this is still a problem for me, using version 1.59.1 though

@EnricoMi
Copy link
Collaborator

Can you confirm with latest release?

@etiennnr
Copy link

Yes, Just tried with v.2.2.0 and got the same result

However, my "global" per_page setting is set to 40. I tried setting it back to the default (30), and then it actually started from the last item of the PaginatedList!

So this actually seem to be somehow related to how the reversed algorithm deals with a different from the default per_page setting.

@engn33r
Copy link

engn33r commented Mar 24, 2024

For reference, I did not set per_page so I was using the default of 30 when I encountered this issue. But I was testing on only one repo with over 30 issues. It could make sense to test with repos with different numbers of issues to see if that is a factor.

@EnricoMi
Copy link
Collaborator

Confirmed, the per_page surfaced a bug: #2929

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 a pull request may close this issue.

4 participants