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

Add cancelation_status column in MiqRequest and MiqRequestTask #254

Merged
merged 1 commit into from
Sep 7, 2018

Conversation

hsong-rh
Copy link
Contributor

@hsong-rh hsong-rh commented Aug 14, 2018

Add cancelation_status to enable cancel operation for V2V migration.

This PR is related with:
ManageIQ/manageiq#17825
ManageIQ/manageiq-automation_engine#212

@agrare
Copy link
Member

agrare commented Aug 14, 2018

@hsong-rh couldn't the cancel states just be additional types of request_state ?

@hsong-rh
Copy link
Contributor Author

@agrare We follow the same logic as approval_state and feel that cancel state is different from request states. It may occur at any request_state and may take some time to finish the cancellation.

@agrare
Copy link
Member

agrare commented Aug 14, 2018

I'll defer to @gmcculloug on this one, but what happens when we add "pause", are we going to add a "pause_state" column? How about "resume", ..."abort"? These all seem like general "state" and do not require dedicated columns IMO.

@gmcculloug
Copy link
Member

I am setting up a meeting for the end of today to get the relevant people together to work through the details of this feature. Stay tuned.

@bdunne
Copy link
Member

bdunne commented Aug 20, 2018

@gmcculloug Any update? Should this be marked as WIP in the mean time?

@gmcculloug
Copy link
Member

Yes, we are working on changes that do not require schema updates. We should mark this as WIP for now and hopefully close once we feel the approach will work.

@miq-bot add_label wip

@miq-bot miq-bot changed the title Add cancel_state column in MiqRequest and MiqRequestTask [WIP] Add cancel_state column in MiqRequest and MiqRequestTask Aug 20, 2018
@miq-bot miq-bot added the wip label Aug 20, 2018
@hsong-rh hsong-rh force-pushed the add_cancel_state_in_miq_request branch from 655c11e to 42234e1 Compare September 4, 2018 21:04
@hsong-rh hsong-rh changed the title [WIP] Add cancel_state column in MiqRequest and MiqRequestTask [WIP] Add cancel_request column in MiqRequest and MiqRequestTask Sep 4, 2018
@hsong-rh
Copy link
Contributor Author

hsong-rh commented Sep 4, 2018

After discussion, we still want to introduce new column for cancel request. No lock and backport are needed so far.

@agrare @Fryguy @gmcculloug please review and merge.

@hsong-rh hsong-rh changed the title [WIP] Add cancel_request column in MiqRequest and MiqRequestTask Add cancel_request column in MiqRequest and MiqRequestTask Sep 4, 2018
@miq-bot miq-bot removed the wip label Sep 4, 2018
@bdunne
Copy link
Member

bdunne commented Sep 6, 2018

Can anyone elaborate on this and how it will be used? Also, why would cancel_request be a string? It sounds like a boolean to me, but I don't have anything to base this off of...

@hsong-rh
Copy link
Contributor Author

hsong-rh commented Sep 6, 2018

@bdunne cancel_request will have 3 different values: cancel_requested, canceling and canceled, means cancellation is pending, processing and done. This is the reason we didn't choose to use boolean.

@bdunne
Copy link
Member

bdunne commented Sep 6, 2018

Maybe cancelation_status would be a better name? What do you think @gmcculloug ?

@carbonin
Copy link
Member

carbonin commented Sep 6, 2018

👍 On cancelation_status

cancel_request sounds like it should be used like "Should this request be canceled" rather than anything having to do with status. Or if you're basing the logic on approval_state maybe this should be cancelation_state?

@hsong-rh hsong-rh force-pushed the add_cancel_state_in_miq_request branch from 42234e1 to 61c7c87 Compare September 6, 2018 18:35
@miq-bot
Copy link
Member

miq-bot commented Sep 6, 2018

Checked commit hsong-rh@61c7c87 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@hsong-rh hsong-rh changed the title Add cancel_request column in MiqRequest and MiqRequestTask Add cancelation_status column in MiqRequest and MiqRequestTask Sep 6, 2018
@hsong-rh
Copy link
Contributor Author

hsong-rh commented Sep 6, 2018

@bdunne @carbonin Changed name to cancelation_status, please review and merge it.

@carbonin carbonin self-assigned this Sep 7, 2018
@carbonin carbonin merged commit b45e742 into ManageIQ:master Sep 7, 2018
@carbonin carbonin added this to the Sprint 94 Ending Sept 10, 2018 milestone Sep 7, 2018
@d-m-u d-m-u mentioned this pull request Sep 18, 2018
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.

6 participants