Add pagination to "Busy" page #5556
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fatkodima
force-pushed
the
busy-pagination
branch
from
744470d
to
0eb1859
Compare
mperham
reviewed
Oct 3, 2022
lib/sidekiq/paginator.rb
Outdated
| @@ -2,14 +2,21 @@ | |||
|
|
|||
| module Sidekiq | |||
| module Paginator | |||
| def page(key, pageidx = 1, page_size = 25, opts = nil) | |||
| def page(key_or_items, pageidx = 1, page_size = 25, opts = nil) | |||
There was a problem hiding this comment.
Break this codepath out into a separate method, page_items or page_enum.
lib/sidekiq/web/application.rb
Outdated
| @@ -74,6 +74,9 @@ def self.set(key, val) | |||
| end | |||
|
|
|||
| get "/busy" do | |||
| @count = (params["count"] || 25).to_i | |||
There was a problem hiding this comment.
WDYT about increasing the default page size to 100 so the user's not paginating as much, given that it's a bit slow?
fatkodima
force-pushed
the
busy-pagination
branch
from
0eb1859
to
97f3ccb
Compare
|
Thanks, @mperham. Agreed with the suggestions. |
|
We're seeing an error when visiting the Busy page after this change: Maybe |
|
@ngan Please provide the full backtrace. |
|
Apologies, this might be something on our end. We're investigating further. If it's a bug on Sidekiq's side, I'll report back with lots more information. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #5555.
The pagination is slower (compared to
/retriesetc pages), because we need to get all of the entries to sort them (byrun_at) before returning. But now this loads with no problems.Seems like it is also problematic to add mass actions like "Kill"/"Delete", because we will need to inefficiently iterate over the large list
list(compared to/retriespage where we can efficiently remove from thezset).You mentioned filtering in the issue. This is Pro/Ent feature, right? So I have not access to it. Better to be implemented by you or someone else who has access.