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

Use task queue for VM actions #13782

Merged
merged 1 commit into from
Feb 16, 2017
Merged

Conversation

sseago
Copy link
Contributor

@sseago sseago commented Feb 6, 2017

Uses task queue instead of making direct provider API calls from the UI.
The following model actions are affected:
resize
evacuate
associate_floating_ip
disassociate_floating_ip

This commit contains the necessary model changes.

Uses task queue instead of making direct provider API calls from the UI.
The following model actions are affected:
  resize
  evacuate
  associate_floating_ip
  disassociate_floating_ip

This commit contains the necessary model changes.
@miq-bot
Copy link
Member

miq-bot commented Feb 6, 2017

Checked commit sseago@e425566 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 1 offense detected

app/models/manageiq/providers/cloud_manager/vm.rb

@tzumainn
Copy link
Contributor

tzumainn commented Feb 7, 2017

Looks good! Two quick questions:

a) the other *_queue methods in vm.rb are class methods - do you know if it's appropriate for these to be the same?

b) will there be a follow up PR against the UI that updates their calls to use the _queue methods? It looks like the UI will still be functional in the meantime, even if this is merged, correct?

@sseago
Copy link
Contributor Author

sseago commented Feb 7, 2017

@tzumainn
Regarding your questions:
a) At least for the _queue methods I created, since the _queue method is a stand-in for the direct method, whether it's a class method or not depends on whether the direct method is a class method. The queuing operation works for both class and instance methods, so it works either way.

b) For all of the calls I've converted, the UI PR should be referencing only the _queue methods now. Did I miss any? If there are other UI actions that still make direct calls instead of _queue ones, we should fix those too. If they relate to these particular new _queue calls, they probably belong in the associated UI PR for this -- if they're remnant direct calls for methods previously converted to using task queues, then a separate PR is probably better.

@tzumainn
Copy link
Contributor

tzumainn commented Feb 7, 2017

@sseago Gotcha; I missed the UI PR, but all looks to be in order there. Thanks!

👍 from me - @blomquisg, any concerns?

@blomquisg blomquisg merged commit b95585e into ManageIQ:master Feb 16, 2017
@blomquisg blomquisg added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 16, 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.

5 participants