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

cardigann: check for page size #14287

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

cardigann: check for page size #14287

wants to merge 2 commits into from
+62 −0

Conversation

mynameisbogdan
Copy link
Contributor

No description provided.

@mynameisbogdan mynameisbogdan added Work in Progress Don't Merge Hold up - don't merge this labels May 1, 2023
@mynameisbogdan mynameisbogdan force-pushed the cardigann-pageable branch 6 times, most recently from b821028 to 7de7dcd Compare May 1, 2023 18:40
ilike2burnthing
ilike2burnthing approved these changes May 1, 2023
View reviewed changes
Copy link
Contributor

@ilike2burnthing ilike2burnthing left a comment

Choose a reason for hiding this comment

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

Working well!

@ilike2burnthing
Copy link
Contributor

What would we use for limitsDefault and limitsMax in YGG, given that the number of requests made will be different depending on the categories used? 50 and 900 respectively?

@mynameisbogdan
Copy link
Contributor Author

mynameisbogdan commented May 1, 2023 •
edited

What would we use for limitsDefault and limitsMax in YGG, given that the number of requests made will be different depending on the categories used? 50 and 900 respectively?

None, the default 100 is okay.

This is useful for pagination rather, but not the scope of this PR. It was on mind for some time, I'll move it to cardigann pagination in the end.

Actually, if the indexer isn't able to support pagination, the higher number will prevent an extra request from sonarr/radarr.

@Qstick
Copy link
Contributor

Qstick commented May 2, 2023

I think some thought needs to go into the overall approach for paging before anything goes in. We've got 4 different PRs now related to Cardigann paging between the projects, so it would be good to align before moving on anything to ensure we are covering all bases.

@ilike2burnthing ilike2burnthing mentioned this pull request Aug 11, 2023
Closed
@mynameisbogdan
Copy link
Contributor Author

lol

@mynameisbogdan mynameisbogdan marked this pull request as draft January 30, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilike2burnthing ilike2burnthing ilike2burnthing approved these changes

@winlover32 winlover32 winlover32 approved these changes

Assignees
No one assigned
Labels
Don't Merge Hold up - don't merge this Work in Progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mynameisbogdan @ilike2burnthing @Qstick @winlover32