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

add pagination to worker counts, preserves backwards compatibility #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wr0ngway
Copy link

With any significant number of workers (we have 1300, soon to be 8K+), workers.counts becomes unwieldy, especially in the qless ruby client's webui.

I tried to preserve backwards compatibility, so workers.counts without any args returns the entire set like it did before.

I also tried to match existing signatures that have pagination (e.g. offset, count as arguments).

However, as a result of the above two requirements (?) the QlessWorker.counts argument processing is a little hokey - basically I check wether the first arg is a number to determine wether the user intended to paginate all or get a single worker.

if worker then
return {
jobs = redis.call('zrevrangebyscore', 'ql:w:' .. worker .. ':jobs', now + 8640000, now),
stalled = redis.call('zrevrangebyscore', 'ql:w:' .. worker .. ':jobs', now, 0)
}
else
local offset = assert(tonumber(arg[1] or 0),
'Failed(): Arg "offset" is not a number: ' .. tostring(arg[1]))
local count = assert(tonumber(arg[2] or 0),
Copy link
Author

Choose a reason for hiding this comment

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

If backwards compatibility is not an issue, the count should default to 25 like the rest of the paginated methods. When both offset and count are 0, it gets the entire data set

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.

1 participant