-
Notifications
You must be signed in to change notification settings - Fork 71
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 more filters to the different job statuses #30
Comments
I'd like to tackle this one! |
hi @bruno-oliveira, are u still working on this? |
hi @rosa, is it okay if give this a try? |
@Xeitor sure thing! |
One thing that would be awesome here would be if we could filter on a job argument. The basic scenario is that we do a lot of jobs on different accounts, and often we have questions of "what is still running for Account X". Right now we use sidekiq and do something kludgy to access this in the UI, and doing it properly with Mission Control would be really awesome. |
@nhorton: sure, sounds useful, will take a look |
hi @rosa, sorry for the delay, I've been getting myself familiar with the codebase I was hoping you could help me out with a doubt, I am confused about what you said in the description about using |
I just realized there already is some logic for raising errors if a filter is set for a non supported job status example: |
I am guessing we could have one hand an error raised if a filter is used for the wrong status (like in the example above) and in the other a method like the one you mention ( |
Hey @Xeitor, sorry, my description was from over 2 months ago, and things have changed since then. mission_control-jobs/lib/active_job/queue_adapters/resque_ext.rb Lines 50 to 54 in 3a826cf
It takes into account what filters the adapter supports but also the |
hi, no problem, from what I can tell, mission_control-jobs/lib/active_job/queue_adapters/resque_ext.rb Lines 50 to 54 in a64a9d0
in the resque adapter so I suppose we need another method that tells you if the filter is supported for the status, unless I am missing something |
@Xeitor ah yes! Sorry, you're totally correct. Other filters could not be natively supported but are fine if performed in memory. I was getting confused with I'll continue the discussion in the PR, ultimately these are all implementation details that might not matter in the end. |
@rosa no problem, thanks for your help :) |
I opened a PR related to this discussion, adding date filters using enqueued_at, scheduled_at and finished_at. I didn't think it was necessary to limit any filter by Job type, considering that regardless of the status, the field is always there, and can only be null or not, so it could just be a filter that will make the query return nothing, such as trying to filter pending by a finished_at date. But this won't break your code and you won't be prevented from doing it, you just won't see any results until you change your filter to something that makes more sense. (After all, of course a pending job shouldn't have a finished_at) I left a deal to resolve Date Ranges as well and it seems to be fine. The only point is: Applying filters to queues (or statuses) with /MANY/ jobs has an impact on performance, but I think we already know that at this point, right? |
For example, filter scheduled jobs by
scheduled_at
time ranges, or finished jobs byfinished_at
time ranges, or blocked jobs by the block (Blocked by) key. The current filters by queue name and job class are generic and apply to all statuses, but we could have some filters that apply only to certain statuses. TheAdapter#supported_filters
already takes aJobsRelation
as argument, so it can easily return different filters depending onjobs_relation.status
.The text was updated successfully, but these errors were encountered: