-
Notifications
You must be signed in to change notification settings - Fork 898
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
Change the role for Service provisioning create_request_tasks MiqQueue.put. #17297
Change the role for Service provisioning create_request_tasks MiqQueue.put. #17297
Conversation
of Service Template Provision Request create_request_tasks. https://bugzilla.redhat.com/show_bug.cgi?id=1534068
@miq-bot add_label provisioning, services, gaprindashvili/yes |
Checked commit tinaafitz@905d525 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@@ -429,7 +429,7 @@ def execute | |||
:instance_id => id, | |||
:method_name => "create_request_tasks", | |||
:zone => options.fetch(:miq_zone, my_zone), | |||
:role => my_role, | |||
:role => kind_of?(ServiceTemplateProvisionRequest) ? 'automate' : my_role, |
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.
The my_role
method is used so sub-classes can override it, like you point out in the description for AutomationRequest
. This value should be changed in the my_role
method in the ServiceTemplateProvisionRequest
model here app/models/service_template_provision_request.rb#L36-L38
Then you do not need this change to the base miq_request
model.
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.
@gmcculloug I specifically changed it that way because I want to limit the change to only the create_request_tasks call.
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.
It would be clearer if the my_role
method took a parameter and the model could then determine the proper response. The refactoring would effect several files so I am ok with doing that as a follow to this PR to this.
@tinaafitz @mkanoor Any thoughts on this approach?
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.
@gmcculloug I'll look into it and we could discuss. I would prefer we do work as a follow up.
@mkanoor Please review. |
Some actions my require different roles during the lifecycle of a request. Based on work from PR ManageIQ#17297
Some actions my require different roles during the lifecycle of a request. Based on work from PR ManageIQ#17297
Most provision request types have a
my_role
value ofems_operations
which makes sense because the provisions have to interact with a provider. Note - the automation request value formy_role
isautomate
.The creation of the request tasks for Service provisioning shouldn't be tied to the
ems_operations
role.Changed the MiqQueue.put role for the Service Template Provision Request
create_request_tasks
fromems_operations
toautomate
. I changed the MiqQueue.put call instead of changing the ServiceTemplateProvisionRequestmy_role
method because I want to limit the role change to just thecreate_request_tasks
method.fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534068