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

Allow top tags API to return all tags #55

Open
johnzhu2014 opened this issue Oct 14, 2014 · 6 comments
Open

Allow top tags API to return all tags #55

johnzhu2014 opened this issue Oct 14, 2014 · 6 comments
Assignees

Comments

@johnzhu2014
Copy link

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
Copy link
Contributor

dlecocq commented Oct 28, 2014

Looking into this one.

@dlecocq
Copy link
Contributor

dlecocq commented Oct 28, 2014

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

@dlecocq
Copy link
Contributor

dlecocq commented Oct 28, 2014

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
Copy link
Contributor

dlecocq commented Oct 28, 2014

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
Copy link
Contributor

b4hand commented Oct 28, 2014

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
Copy link
Contributor

dlecocq commented Oct 28, 2014

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

@dlecocq dlecocq changed the title tags is missing when calling client.call("tag", "top", offset, count) Allow top tags API to return all tags Oct 28, 2014
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

3 participants