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

10 - add-show-all-statuses-filter-option #16

Closed
distributev opened this issue Jun 30, 2016 · 18 comments
Closed

10 - add-show-all-statuses-filter-option #16

distributev opened this issue Jun 30, 2016 · 18 comments

Comments

@distributev
Copy link
Owner

10 - add-show-all-statuses-filter-option

@nabilzhafri
Copy link
Contributor

sorry, but i'm aware of this.
unfortunately, this is not possible with the current kue api when jobtypes is provided.
I filter it out on purpose, since all states only available when jobtype is not provided.

"Show All Job Types" + "Show All States" is available.

Hope it clears.

@distributev
Copy link
Owner Author

In the GUI you call this ""Show All States" (it's up to us how we label the GUI)

On the backend you just call again again existing /jobs/email and this will return you all the email jobs no matter which status (and this is what I actually need).

@nabilzhafri
Copy link
Contributor

Update to "Show All Statuses" - I can do this.

But to "Show All Statuses" when a job type is selected - I can't since Kue API doesn't support this, like I mentioned earlier.
For the moment, "Show All Statuses" is only available when job type is not selected i.e show all types

@distributev
Copy link
Owner Author

Is there any kue API /jobs/email or /jobs/backup, etc.?

@nabilzhafri
Copy link
Contributor

nope. unfortunately. when selecting job type, must provide state i.e inactive, failed, etc. If there is, I've already enable it :) easier for me to implement also.

@distributev
Copy link
Owner Author

Please raise a query / enhancement request to kue and link to this case. The existence of such API looks very useful to me. Actually I'm surprised is not there already.

Depending on their answer I'll see how to approach this.

@distributev
Copy link
Owner Author

GET /job/search?q=email see what is returns
and then use returned IDs to call GET /jobs/:from..:to/:order?

or as an alternative

try GET /jobs/:type/ - see what is returns

@nabilzhafri
Copy link
Contributor

already tried the searching, not working.
there's no /jobs/:type/, but there's /jobs/:state/.

@distributev
Copy link
Owner Author

"already tried the searching, not working."

you mean it's a kue bug?

@nabilzhafri
Copy link
Contributor

no i meant, their searching cannot be used as a workaround. searching is for job's data

@distributev
Copy link
Owner Author

I understand.

Please tell me about following approach. There are 5 different states. On /server/ you roll your own simple API getAllJobs (type = null)

type argument is optional and if not provided (null) the meaning is 'All Types' - it can also be provided i.e. 'email'

inside this API you call sequentially /jobs/type/inactive/, /jobs/type/active/, /jobs/type/complete/, /jobs/type/failed/ and /jobs/type/delayed/ ==> you get 5 separate lists of jobs

then you merge yourself the 5 lists into a single complete list to get all states. You have lodash _ available which will make collection merging easy. At the end you return the complete list.

P.S - I understand the performance implications of calling 5 different APIs in a single call. This is to overcome the kue API limitation.

An even better approach would be to extend the current kue API and submit the enhanced code to their repository. This would probably require some knowledge of redis API to return the needed jobs.

@nabilzhafri
Copy link
Contributor

nabilzhafri commented Jul 1, 2016

Hi, understand your request.

But as this task will require outside of the current task scope i.e. extending kue APIs, can we put this task in the next project? I remember you said there'll be kue jobs related. Or if you can't wait, please give me a while before I can help you with this.

For now, I wanna focus on completing the current tasks / fixing any existing project related issues first. Note that enabling this on the frontend is an easy task.

Thanks

@distributev
Copy link
Owner Author

I understand what you are saying. I agree it might be outside of the scope of this task if we go the 'let's add this API enhancement to kue repository to make the kue api better' - this will also imply learning a bit of Redis apis so let's put this approach aside for now.

However what about the first approach suggested in which we don't enhance the kue API but we "work-around" by calling 5 different lists (inactive, active, complete, etc.) and then we merge the separate lists to get the complete one. Is this doable?

P.S - For a user this kue API limitation gives very strange / confusing results in the GUI. I just clicked the 'Email Jobs' menu entry - Naturally I expected to see my list of emails jobs however what I got instead was a single row with an inactive email. Then I saw at the bottom filtered 1 out of 36 rows - and I asked myself 'When did I filter this list? Where are my emails?' - Getting correct jobs lists in the GUI is for sure part of the current task ==> as a result I'm trying to find a good enough solution.

@nabilzhafri
Copy link
Contributor

I understand. First approach can totally be done :) but like I said, if you don't mind waiting in case if you wanna test it right away. I'll help you to enable this - but note that it could be slower in term of performance.

Biggest problem here is the ordering - since now we're fetching them separately, there's no easy way to tell how to order them once we merged them together. Unless if we don't care about this and just display it to user.

What do you think?

@nabilzhafri nabilzhafri mentioned this issue Jul 2, 2016
@distributev
Copy link
Owner Author

on /sever

Step 1 - you call individual APIs for /active, /inactive, /completed, etc ==> you get 5 separate lists of jobs
Step 2 - using lodash _ you merge the 5 separate lists into a single one
Step 3 - using lodash _ you sort the commune DESC based on job submission time (newest first)

At the end you return the sorted jobs to /client UI

Is that possible?

@nabilzhafri
Copy link
Contributor

ok let me do this. will update you in a bit

@nabilzhafri
Copy link
Contributor

Hi! I've enabled this feature for you. Please take a look :)

Note that this is more to custom work, cause the original scope is to implement directives using existing kue api, but this one requires custom api implementation.

Hope everything works well.

@distributev
Copy link
Owner Author

For more details follow - Automattic/kue#916

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