-
Notifications
You must be signed in to change notification settings - Fork 897
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
Activate miq_task when deliver from miq_queue #17015
Activate miq_task when deliver from miq_queue #17015
Conversation
11f10b2
to
b2a1df7
Compare
\cc @mkanoor |
\cc @Fryguy |
app/models/miq_queue.rb
Outdated
@@ -541,6 +543,13 @@ def self.get_worker(task_id) | |||
|
|||
private | |||
|
|||
def activate_miq_task(args) | |||
MiqTask.update_status(miq_task_id, MiqTask::STATE_ACTIVE, MiqTask::STATUS_OK, "Task starting") if miq_task |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer if miq_task_id
, so it doesn't need to go out and fetch the task on every dispatch. That being said, if the task is deleted out from underneath the queue item, the MiqTask.update_status method should probably handle that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 using miq_task_id
is better
When task does not exist MiqTask.update_status
does handle it it:
manageiq/app/models/miq_task.rb
Line 91 in c76b358
task.update_status(state, status, message) unless task.nil? |
app/models/miq_queue.rb
Outdated
@@ -541,6 +543,13 @@ def self.get_worker(task_id) | |||
|
|||
private | |||
|
|||
def activate_miq_task(args) | |||
MiqTask.update_status(miq_task_id, MiqTask::STATE_ACTIVE, MiqTask::STATUS_OK, "Task starting") if miq_task | |||
pars = args.first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer params
or parameters
...pars
is not a common abbreviation, so it reads funny.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@yrudman What happens to
|
app/models/miq_queue.rb
Outdated
def activate_miq_task(args) | ||
MiqTask.update_status(miq_task_id, MiqTask::STATE_ACTIVE, MiqTask::STATUS_OK, "Task starting") if miq_task_id | ||
params = args.first | ||
params[:miq_task_id] = miq_task_id if pars.kind_of?(Hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woops (if pars
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:(
72a1b2d
to
31b3663
Compare
|
also, if there is no willingness to use |
…o be saved as attribute of queued item
…bute of queued item
…linked to queue item
…here is extra parameter added to the hash when message delivered - miq_task_id
dad3655
to
f7653b0
Compare
Checked commits yrudman/manageiq@5c32ee4~...f7653b0 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 app/models/miq_task.rb
|
@miq-bot add-label gaprindashvili/no |
This PR:
started
dateHash
than:miq_task_id => <task id>
will be added to parameters when deliveringblocked by:
miq_task_id
column tomiq_queue
tableBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1543289