search

seomoz / qless-core

Core Lua Scripts for qless
MIT License
85 stars 34 forks source link

Allow `top tags` API to return all tags #55

Open johnzhu2014 opened 10 years ago

johnzhu2014 commented 10 years ago

Original:

We see the tags in redis/ui but when call client.call("tag", "top", offset, count) we always get empty tag list, i.e. {}. That happens in ruby binding and java binding.

Updated:

The top tags API is currently only designed to return tags with at least 2 jobs tagged as such. This task is to make the minimum number of backing jobs an argument (with a sane default) to the top tags API.

dlecocq commented 10 years ago

Looking into this one.

dlecocq commented 10 years ago

Was able to repro on ba8e556, working on a fix.

dlecocq commented 10 years ago

Blerg. Nevermind, actually. It looks as though this is by design. The tag top API only returns tags with at least two backing jobs.

In the relevant qless-java test, you can just enqueue a second job with the same tags as the first and it will pass.

dlecocq commented 10 years ago

WRT the reasoning for setting the limit to 2, I've been trying to recall it exactly. My recollection is that we didn't necessarily want to expose every tag in use, but only those that were used by multiple jobs.

In hindsight, I don't have any particular objection to changing this functionality if others feel strongly. That said, I have a feeling this is functionality that's not used particularly much.

b4hand commented 10 years ago

I think the logical thing to do would be to simply return all tags, but you could provide a limit to the top call such that tags must have at least a given population for including them.

dlecocq commented 10 years ago

That seems reasonable to me. Not sure I'll tackle that right now, but instead I'll repurpose this ticket to fit that task.