package: Add support for different query job types:#452
Conversation
| SELECT {QUERY_JOBS_TABLE_NAME}.id as job_id | ||
| FROM {QUERY_JOBS_TABLE_NAME} | ||
| WHERE {QUERY_JOBS_TABLE_NAME}.status={QueryJobStatus.CANCELLING} | ||
| AND {QUERY_JOBS_TABLE_NAME}.type={QueryJobType.SEARCH} |
There was a problem hiding this comment.
Assume we only cancel search job to simplify the implementation
junhaoliao
left a comment
There was a problem hiding this comment.
The WebUI changes look good to me.
|
|
||
|
|
||
| class SearchJob(QueryJob): | ||
| search_config: SearchConfig |
There was a problem hiding this comment.
Should we rename this to just "job_config"
There was a problem hiding this comment.
I think search_config is fine.
| SELECT {QUERY_JOBS_TABLE_NAME}.id as job_id | ||
| FROM {QUERY_JOBS_TABLE_NAME} | ||
| WHERE {QUERY_JOBS_TABLE_NAME}.status={QueryJobStatus.CANCELLING} | ||
| AND {QUERY_JOBS_TABLE_NAME}.type={QueryJobType.SEARCH} |
There was a problem hiding this comment.
I think the naming of QueryJobType.SEARCH gets confusing for me here. When I see this my first thought is wondering about what happens to aggregation jobs, but actually QueryJobType.SEARCH represents search AND aggregation jobs.
I think we might want to eventually split these out into different job types, but to avoid holding you up here how about we just rename this to QueryJobType.SEARCH_OR_AGGREGATION which is ugly but more clear at least.
There was a problem hiding this comment.
would it also require renaming other places in the query_scheduler？
for example, "pending_query_job" will become "pending_query_or_aggregation_job", and "search_config" becomes "search_or_aggregation_config"?
I personally prefer to not rename those, but only change QueryJobType.SEARCH to QueryJobType.SEARCH_OR_AGGREGATION.
There was a problem hiding this comment.
Yeah, let's just rename the SEARCH_OR_AGGREGATION and leave everything else. The aggregations are a query of sorts, so I think its fine.
|
|
||
|
|
||
| class SearchJob(QueryJob): | ||
| search_config: SearchConfig |
There was a problem hiding this comment.
I think search_config is fine.
kirkrodrigues
left a comment
There was a problem hiding this comment.
A few small questions / suggestions.
…/query_scheduler.py Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
kirkrodrigues
left a comment
There was a problem hiding this comment.
For the PR title & body, how about:
package: Add support for different query job types:
- Add abstract classes for QueryJobType and QueryJobConfig.
- Submit and track job type in the search jobs table.
- Guard search-job specific handling in the scheduler.
- Add abstract classes for QueryJobType and QueryJobConfig. - Submit and track job type in the search jobs table. - Guard search-job specific handling in the scheduler.
- Add abstract classes for QueryJobType and QueryJobConfig. - Submit and track job type in the search jobs table. - Guard search-job specific handling in the scheduler.
Description
This PR is a preparation to let query-scheduler supports more than one type of jobs.
The following changes are made:
Validation performed
Built the package
Verified that search and reducer works properly from both commandline and webui