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

Sidekiq Web Pagination Broken #309

Closed
chadrschroeder opened this issue Jul 31, 2018 · 6 comments
Closed

Sidekiq Web Pagination Broken #309

chadrschroeder opened this issue Jul 31, 2018 · 6 comments

Comments

@chadrschroeder
Copy link

When there are more than 100 digests to display on the Sidekiq Web Unique Digests page, the following error occurs:

NoMethodError
undefined method `>' for nil:NilClass

I think this is because the @current_page value is nil. Sidekiq Web expects this variable in its paging partial:

<%# sidekiq/web/views/_paging.erb %>
<% if @current_page > 1 %>
  ...

Also, when are these unique digests purged? They remain in Redis after my jobs have completed successfully and the unique:keys set continues to increase in size.

@mhenrixon
Copy link
Owner

Regarding the first question, should not be a problem with sidekiq/web as I am not paging it yet. Would be cool to know the exact line number it goes wrong.

Regarding the second question. They should be removed upon completion or when deleted from the sidekiq web extension. BUT... I found a bug, I try to delete the wrong ones. I'll have a fix for that shortly.

@mhenrixon
Copy link
Owner

This line https://github.com/mhenrixon/sidekiq-unique-jobs/blob/master/redis/signal.lua#L12 should say: redis.call('SREM', unique_keys, unique_digest)

@mhenrixon mhenrixon mentioned this issue Jul 31, 2018
@chadrschroeder
Copy link
Author

For the first question, the issue is because @count defaults to 100 here and that causes us to render the paging partial here.

@mhenrixon
Copy link
Owner

Cool, I'll add support for pagination as well while I'm at it then. Thanks for reporting

@chadrschroeder
Copy link
Author

Thanks for the quick response.

@mhenrixon
Copy link
Owner

Should be fixed in v6.0.2 @chadrschroeder! Thanks again for taking the time to report the issue.

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

No branches or pull requests

2 participants