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.

Sign up for GitHub

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

Jump to bottom

Add tasks and models for retire as a request #16933

Merged
merged 3 commits into from
Mar 27, 2018
Merged

Add tasks and models for retire as a request #16933

merged 3 commits into from
Mar 27, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Feb 1, 2018 •
edited
Loading

VM retirement should be a request. This is ☠️ first pass at creating task and model.

This PR includes the service and orch stack task and request models because it has to in order to pass specs in the retirement manager.

Related to:

classic-ui pr here: ManageIQ/manageiq-ui-classic#3409
automate changes: ManageIQ/manageiq-automation_engine#157
ManageIQ/manageiq-content#262

@d-m-u d-m-u changed the title [WIP] Add skeleton task and request for vm migrate [WIP] Add skeleton task and model for vm migrate request Feb 1, 2018
@miq-bot miq-bot added the wip label Feb 1, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 1, 2018

@tinaafitz

@d-m-u d-m-u force-pushed the starting_retirement_request branch from b3ad86f to 62ced64 Compare February 1, 2018 18:57
@d-m-u d-m-u changed the title [WIP] Add skeleton task and model for vm migrate request [WIP] Add skeleton task and model for vm retire request Feb 1, 2018
@d-m-u d-m-u force-pushed the starting_retirement_request branch from 62ced64 to 8bd5174 Compare February 1, 2018 19:02
@d-m-u d-m-u mentioned this pull request Feb 14, 2018
Merged
d-m-u
d-m-u commented Feb 14, 2018
View reviewed changes
app/models/vm_retire_request.rb Outdated
SOURCE_CLASS_NAME = 'Vm'.freeze
ACTIVE_STATES = %w(retired) + base_class::ACTIVE_STATES

validates :request_state, :inclusion => { :in => %w(pending finished) + ACTIVE_STATES }, :message => "should be pending, #{ACTIVE_STATES.join(", ")} or finished"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line breaks the UI change in the associated classic-ui PR

@d-m-u d-m-u force-pushed the starting_retirement_request branch 4 times, most recently from 8f8b706 to 9f123f6 Compare February 16, 2018 13:20
@miq-bot
Copy link
Member

miq-bot commented Feb 19, 2018

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

@d-m-u d-m-u force-pushed the starting_retirement_request branch from d7cbaa5 to 774c7f0 Compare February 19, 2018 21:49
@miq-bot
Copy link
Member

miq-bot commented Feb 19, 2018

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

@d-m-u d-m-u force-pushed the starting_retirement_request branch from a9a7774 to 405eb90 Compare February 20, 2018 14:14
@d-m-u d-m-u mentioned this pull request Feb 20, 2018
Merged
@d-m-u d-m-u force-pushed the starting_retirement_request branch 8 times, most recently from cb3312e to b376da7 Compare February 28, 2018 16:03
@d-m-u d-m-u force-pushed the starting_retirement_request branch 3 times, most recently from 841a36e to 0d789ad Compare March 22, 2018 19:57
mkanoor
mkanoor reviewed Mar 22, 2018
View reviewed changes
app/models/vm_retire_request.rb Outdated
@@ -0,0 +1,10 @@
class VmRetireRequest < MiqRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

@d-m-u Should this inherit from MiqRetireRequest?

mkanoor
mkanoor reviewed Mar 22, 2018
View reviewed changes
app/models/service_retire_task.rb Outdated
Service
end

def after_request_task_create
Copy link
Contributor

Choose a reason for hiding this comment

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

@d-m-u
Can't this function be used from MiqRetireTask?

@d-m-u d-m-u force-pushed the starting_retirement_request branch 2 times, most recently from 60455f4 to 4a82497 Compare March 22, 2018 20:33
@d-m-u d-m-u changed the title [WIP] Add skeleton task and model for retire as a request Add tasks and models for retire as a request Mar 22, 2018
@miq-bot miq-bot removed the wip label Mar 22, 2018
mkanoor
mkanoor approved these changes Mar 23, 2018
View reviewed changes
app/models/service_retire_task.rb Outdated
update_attribute(:description, get_description)
end

def after_ae_delivery(ae_result)
Copy link
Contributor

Choose a reason for hiding this comment

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

@d-m-u Can this also be from MiqRetireTask?

@d-m-u d-m-u force-pushed the starting_retirement_request branch 2 times, most recently from de816b2 to 98ec4bd Compare March 26, 2018 13:50
@tinaafitz
Copy link
Member

@d-m-u Looks good!
@gmcculloug Please review.

bdunne
bdunne requested changes Mar 26, 2018
View reviewed changes
app/models/miq_retire_task.rb Outdated

def self.get_description(req_obj)
name = nil
if req_obj.source.nil?
Copy link
Member

Choose a reason for hiding this comment

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

name = if req_obj.source.nil? then you can get rid of all of the other name = inside the conditional

spec/factories/orchestration_stack_retire_request.rb Outdated
@@ -0,0 +1,3 @@
FactoryGirl.define do
factory :orchestration_stack_retire_request
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the empty factories?

@d-m-u d-m-u force-pushed the starting_retirement_request branch 2 times, most recently from fc80a60 to 215f1f0 Compare March 26, 2018 21:51
@d-m-u d-m-u force-pushed the starting_retirement_request branch from 215f1f0 to c4882a5 Compare March 26, 2018 21:59
@miq-bot
Copy link
Member

miq-bot commented Mar 26, 2018

Checked commits d-m-u/manageiq@42aebe6~...c4882a5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
15 files checked, 1 offense detected

app/models/miq_retire_task.rb

bdunne
bdunne reviewed Mar 26, 2018
View reviewed changes
app/models/miq_request.rb
:VmRetireRequest => {
:vm_retire => N_("VM Retire")
},
:ServiceRetireRequest => {
Copy link
Member

Choose a reason for hiding this comment

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

Normally I would complain about this being alphabetical, but it's already messed up ¯\_(ツ)_/¯

Copy link
Member

Choose a reason for hiding this comment

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

Let's address this in a followup PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#17206

@gmcculloug gmcculloug merged commit 9d70fbc into ManageIQ:master Mar 27, 2018
@gmcculloug gmcculloug added this to the Sprint 82 Ending Mar 26, 2018 milestone Mar 27, 2018
@d-m-u d-m-u deleted the starting_retirement_request branch March 27, 2018 00:53
@lfu lfu mentioned this pull request Jul 26, 2019
Merged
@gmcculloug gmcculloug mentioned this pull request Oct 21, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmcculloug gmcculloug gmcculloug left review comments

@mkanoor mkanoor mkanoor approved these changes

@bdunne bdunne bdunne approved these changes

Assignees

@tinaafitz tinaafitz

Labels
enhancement lifecycle/retirement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@d-m-u @miq-bot @tinaafitz @bdunne @gmcculloug @mkanoor