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

[e621] implement manual pagination #3413

Merged
merged 3 commits into from
Dec 17, 2022

Conversation

ClosedPort22
Copy link
Contributor

Sometimes the number of returned post objects is smaller than the per-page limit even though there are more posts. This causes gallery-dl to stop the pagination prematurely.

Example JSON
The API returns 318 posts.

This only seems to apply to deleted posts. Not sure if other Danbooru instances are affected as well.

@@ -126,7 +127,7 @@ def _pagination(self, endpoint, params, pagenum=False):
posts = posts["posts"]
yield from posts

if len(posts) < self.per_page:
if self.strategy == "length" and len(posts) < self.per_page:
Copy link
Owner

Choose a reason for hiding this comment

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

I'd suggest introducing a new limit variable at the start of _pagination() and setting it to self.per_page or 1 depending on self.strategy and using it like if len(posts) < limit: return. Seems cleaner to me.

Maybe this option should better be called something like limit with "auto" (self.per_page or self.per_page // 2?) being the default and otherwise the number of returned posts when to stop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this option should better be called something like limit with "auto" (self.per_page or self.per_page // 2?) being the default and otherwise the number of returned posts when to stop?

Currently gallery-dl does not actually check if the value is "auto". Should I only allow int and "auto" and raise an error if an invalid type is specified?

Copy link
Owner

Choose a reason for hiding this comment

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

I'd check if the value is "auto" or None and use self.per_page in this case, and assume it is int otherwise and apply text.parse_int to it just to make sure. At least that is what I've usually done in such cases.

Your implementation from dd4a4a3 is fine as well.

@mikf mikf merged commit f36cbb3 into mikf:master Dec 17, 2022
@ClosedPort22 ClosedPort22 deleted the e621-manual-pagination branch December 17, 2022 15:17
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