VmOrTemplate supports_terminate instead of validate_terminate #11018

jameswnl · 2016-09-06T06:53:15Z

Purpose or Intent

Replace VmOrTemplate.validate_terminate with supports_terminate

jameswnl · 2016-09-06T06:55:28Z

@miq-bot add_labels wip, refactoring, "pluggable providers"

miq-bot · 2016-09-06T06:56:10Z

@jameswnl Cannot apply the following label because they are not recognized: "pluggable providers"

durandom · 2016-09-06T14:50:24Z

app/models/vm_or_template/operations.rb

-    return {:available => false, :message => 'The VM is terminated'} if self.terminated?
-    {:available => true, :message => nil}
+    supports :terminate do
+      unsupported_reason_add(:terminate, 'The VM is terminated') if terminated?


translate the string with _()

durandom · 2016-09-07T10:23:52Z

@jameswnl I'm ok with having this called terminate for now, but IMHO it should be named closer to the actual method vm_destroy, but this would involve more refactoring.

So, could you either add this as comments to the supports :terminate method or even better refactor the calls to supports :vm_destroy? There is only a couple of occurences in the code, as I can see

jameswnl · 2016-09-07T13:21:36Z

@durandom yes, I can rename terminate to vm_destroy. I thought about that too but wasn't sure - you know naming is hard 😄
Since you also support, I'll do that.

jameswnl · 2016-09-07T15:42:23Z

@durandom About renaming terminate: I saw a previous test for validate_terminate was removed by your PR. Can you advise on how to confirm the name change is not breaking UI?

Update: I've tried to rename it and it breaks a bunch of tests. I've since reverted the renaming. Let's keep this PR focusing on getting the supports_terminate done first.

durandom · 2016-09-07T19:23:39Z

app/models/vm_or_template/operations.rb

-    {:available => true, :message => nil}
+    supports :terminate do
+      msg = unsupported_reason(:control) unless supports_control?
+      msg ||= _("Provider doesn't support vm_destroy") unless ext_management_system.respond_to?(:vm_destroy)


when checking for two reason we just added all reasons

unsupported_reason_add(:vm_destroy, unsupported_reason(:control)) unless supports_control? unsupported_reason_add(:vm_destroy, _("Provider doesn't support vm_destroy") unless ext_management_system.respond_to?(:vm_destroy))

This way we could later even stack the reasons when asking for this.

durandom · 2016-09-08T15:33:39Z

ok, 🏆 from my side.

@miq-bot assign blomquisg
@blomquisg merge?

jameswnl · 2016-09-08T18:35:58Z

Thanks @durandom 😄

durandom

@blomquisg merge?

jameswnl · 2016-09-15T18:55:31Z

@blomquisg do you need a physical ping? 😛

miq-bot · 2016-09-30T21:47:24Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

durandom · 2016-10-04T13:31:02Z

@jameswnl can you rebase, please, after that I'll make sure its merged asap :)

miq-bot · 2016-10-04T19:57:02Z

Checked commits jameswnl/manageiq@0a95dcd~...9bab451 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 0 offenses detected
Everything looks good. ⭐

durandom · 2016-10-05T06:58:51Z

@miq-bot assign blomquisg
@miq-bot add_label euwe/no
@blomquisg please merge, thanks

romanblanco · 2017-01-17T18:58:56Z

@dclarizio @blomquisg This PR needs to go to Euwe to fix this BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1385587

VmOrTemplate supports_terminate instead of validate_terminate (cherry picked from commit 91d5c94) https://bugzilla.redhat.com/show_bug.cgi?id=1414550

simaishi · 2017-01-18T21:22:30Z

Euwe backport details:

$ git log -1
commit 084ecea58bfb4fa5481839c9a7196a3c018e4bdf
Author: Greg Blomquist <[email protected]>
Date:   Wed Oct 5 14:37:39 2016 -0400

    Merge pull request #11018 from jameswnl/validate_terminate
    
    VmOrTemplate supports_terminate instead of validate_terminate
    (cherry picked from commit 91d5c94c112abf7bcc3b73f743865f7ac682b3ca)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1414550

https://bugzilla.redhat.com/show_bug.cgi?id=1423470 b691300 for PR ManageIQ#11418 was backported before 084ecea for PR ManageIQ#11018, causing some git context to bring over the :terminate key twice.

miq-bot added refactoring wip labels Sep 6, 2016

jameswnl changed the title ~~[WIP] VmOrTemplate supports_terminate instead of validate_terminate~~ VmOrTemplate supports_terminate instead of validate_terminate Sep 6, 2016

chessbyte assigned durandom Sep 6, 2016

chessbyte added pluggability/providers and removed wip labels Sep 6, 2016

durandom reviewed Sep 6, 2016
View reviewed changes

jameswnl force-pushed the validate_terminate branch 2 times, most recently from a0f26c9 to 24f197c Compare September 7, 2016 17:50

durandom reviewed Sep 7, 2016
View reviewed changes

jameswnl force-pushed the validate_terminate branch from 24f197c to 10706ac Compare September 7, 2016 20:04

miq-bot assigned blomquisg and unassigned durandom Sep 8, 2016

jameswnl force-pushed the validate_terminate branch from 10706ac to 679cfa5 Compare September 12, 2016 21:46

blomquisg assigned durandom and unassigned blomquisg Sep 14, 2016

jameswnl force-pushed the validate_terminate branch from 679cfa5 to 9704ecb Compare September 15, 2016 03:33

durandom approved these changes Sep 15, 2016

View reviewed changes

jameswnl added 2 commits October 4, 2016 15:51

VmOrTemplate supports_terminate

0a95dcd

add :terminate to SupportFeatureMixin list

9bab451

jameswnl force-pushed the validate_terminate branch from 9704ecb to 9bab451 Compare October 4, 2016 19:53

miq-bot assigned blomquisg and unassigned durandom Oct 5, 2016

miq-bot added the euwe/no label Oct 5, 2016

blomquisg merged commit 91d5c94 into ManageIQ:master Oct 5, 2016

blomquisg added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 5, 2016

durandom mentioned this pull request Oct 14, 2016

[WIP] correct check for feature :terminate on vm_or_template #11937

Closed

jameswnl deleted the validate_terminate branch December 1, 2016 22:36

martinpovolny added euwe/yes and removed euwe/no labels Jan 18, 2017

simaishi pushed a commit that referenced this pull request Jan 18, 2017

Merge pull request #11018 from jameswnl/validate_terminate

084ecea

VmOrTemplate supports_terminate instead of validate_terminate (cherry picked from commit 91d5c94) https://bugzilla.redhat.com/show_bug.cgi?id=1414550

simaishi added euwe/backported and removed euwe/yes labels Jan 18, 2017

jrafanie mentioned this pull request Feb 20, 2017

[EUWE] Hash key 'terminate' was duplicated due to backport PR order #13987

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VmOrTemplate supports_terminate instead of validate_terminate #11018

VmOrTemplate supports_terminate instead of validate_terminate #11018

jameswnl commented Sep 6, 2016 •

edited by miq-bot

Loading

jameswnl commented Sep 6, 2016

miq-bot commented Sep 6, 2016

durandom Sep 6, 2016

durandom commented Sep 7, 2016

jameswnl commented Sep 7, 2016

jameswnl commented Sep 7, 2016 •

edited

Loading

durandom Sep 7, 2016

durandom commented Sep 8, 2016

jameswnl commented Sep 8, 2016

durandom left a comment

jameswnl commented Sep 15, 2016

miq-bot commented Sep 30, 2016

durandom commented Oct 4, 2016

miq-bot commented Oct 4, 2016

durandom commented Oct 5, 2016 •

edited

Loading

romanblanco commented Jan 17, 2017 •

edited

Loading

simaishi commented Jan 18, 2017

VmOrTemplate supports_terminate instead of validate_terminate #11018

VmOrTemplate supports_terminate instead of validate_terminate #11018

Conversation

jameswnl commented Sep 6, 2016 • edited by miq-bot Loading

Purpose or Intent

jameswnl commented Sep 6, 2016

miq-bot commented Sep 6, 2016

durandom Sep 6, 2016

Choose a reason for hiding this comment

durandom commented Sep 7, 2016

jameswnl commented Sep 7, 2016

jameswnl commented Sep 7, 2016 • edited Loading

durandom Sep 7, 2016

Choose a reason for hiding this comment

durandom commented Sep 8, 2016

jameswnl commented Sep 8, 2016

durandom left a comment

Choose a reason for hiding this comment

jameswnl commented Sep 15, 2016

miq-bot commented Sep 30, 2016

durandom commented Oct 4, 2016

miq-bot commented Oct 4, 2016

durandom commented Oct 5, 2016 • edited Loading

romanblanco commented Jan 17, 2017 • edited Loading

simaishi commented Jan 18, 2017

jameswnl commented Sep 6, 2016 •

edited by miq-bot

Loading

jameswnl commented Sep 7, 2016 •

edited

Loading

durandom commented Oct 5, 2016 •

edited

Loading

romanblanco commented Jan 17, 2017 •

edited

Loading