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

Set the initiator from the workflow/request #14070

Merged
merged 7 commits into from
Mar 6, 2017

Conversation

mkanoor
Copy link
Contributor

@mkanoor mkanoor commented Feb 24, 2017

Set the initiator in the workflow and request

Depends on #14035

@mkanoor
Copy link
Contributor Author

mkanoor commented Feb 24, 2017

@tinaafitz @gmcculloug
Please review

@gmcculloug gmcculloug self-assigned this Feb 24, 2017
@miq-bot miq-bot added the wip label Feb 24, 2017
Copy link
Member

@gmcculloug gmcculloug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR contains lines from #14035 and #13874 which should be mentioned as dependents.

@@ -799,7 +806,7 @@ def action_cancel_task(action, rec, inputs)
end

MiqPolicy.logger.info("MIQ(action_cancel_task): Now executing Cancel of task [#{source_event.event_type}] on VM [#{source_event.vm_name}]")
ems = ExtManagementSystem.find_by(:id => source_event.ems_id)
ems = ExtManagementSystem.find_by_id(source_event.ems_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you changing this? There have been a number of PRs merged to use the other form: https://github.com/ManageIQ/manageiq/pulls?utf8=%E2%9C%93&q=is%3Apr%20author%3Achessbyte%20is%3Aclosed%20find_by

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmcculloug
since i need the miq_action changes i might have picked up extra changes from master. in only need the line 228 update

@tinaafitz
Copy link
Member

@mkanoor Looks good.

@miq-bot
Copy link
Member

miq-bot commented Feb 28, 2017

This pull request is not mergeable. Please rebase and repush.

@gmcculloug
Copy link
Member

@mkanoor PR ##14035 is merged. Please rebase.

@mkanoor mkanoor changed the title [WIP] Set the initiator from the workflow/request Set the initiator from the workflow/request Mar 2, 2017
@miq-bot miq-bot removed the wip label Mar 2, 2017
@mkanoor
Copy link
Contributor Author

mkanoor commented Mar 3, 2017

@gmcculloug
Please review

@@ -113,6 +113,16 @@
end
end

context "initiator" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkanoor Do we have any tests for when :initiator is not passed or when it is passed as nil? If the key is passed with a nil value I think it will save nil in the record instead of using the default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmcculloug Yes that was an issue, I fixed it

@miq-bot
Copy link
Member

miq-bot commented Mar 3, 2017

Checked commits mkanoor/manageiq@9efba3a~...1c171b8 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks good. 👍

@mkanoor
Copy link
Contributor Author

mkanoor commented Mar 3, 2017

@gmcculloug Please review

@@ -187,6 +187,8 @@ def create_service(service_task, parent_svc = nil)
# convert template class name to service class name by naming convention
nh[:type] = self.class.name.sub('Template', '')

nh[:initiator] = service_task.options[:initiator] if service_task.options[:initiator]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmcculloug
If the value is nil we don't set the initiator causing the default value of user to be used.

@gmcculloug gmcculloug merged commit ba06004 into ManageIQ:master Mar 6, 2017
@gmcculloug gmcculloug added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants