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

Expose attach/detach method for volume #14289

Merged

Conversation

andyvesel
Copy link
Contributor

Expose attach and detach methods for Cloud Volume

@miq-bot miq-bot added the wip label Mar 13, 2017
@andyvesel andyvesel closed this Mar 14, 2017
@andyvesel andyvesel reopened this Mar 14, 2017
@andyvesel andyvesel force-pushed the expose_attach_detach_for_cloud_volume branch from c808552 to 5802c2a Compare March 15, 2017 10:13
@andyvesel andyvesel force-pushed the expose_attach_detach_for_cloud_volume branch from 5802c2a to 0f11a5b Compare March 15, 2017 10:20
@miq-bot
Copy link
Member

miq-bot commented Mar 15, 2017

Checked commit andyvesel@0f11a5b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. ⭐

@aufi
Copy link
Member

aufi commented Mar 15, 2017

Looks good to me 👍

@andyvesel andyvesel changed the title [WIP] Expose attach/detach method for volume Expose attach/detach method for volume Mar 16, 2017
@miq-bot miq-bot removed the wip label Mar 16, 2017
@chessbyte chessbyte assigned mkanoor and gmcculloug and unassigned mkanoor Mar 20, 2017
@chessbyte chessbyte requested a review from mkanoor March 20, 2017 13:18
@@ -10,5 +10,7 @@ class MiqAeServiceCloudVolume < MiqAeServiceModelBase
expose :cloud_volume_backups, :association => true
expose :cloud_volume_snapshots, :association => true
expose :attachments, :association => true
expose :attach_volume, :override_return => nil
expose :detach_volume, :override_return => nil
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to return a success or failure status of some kind? The user will have no indication if the method was successful if we always return nil.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for comment. These model methods raise an error on failure or nothing meaningful on success. Would :override_return => true work or should we update methods body to return true/false (or something better)?

Copy link
Member

Choose a reason for hiding this comment

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

I think we are good it the backing method raises an error. Thanks.

@gmcculloug gmcculloug merged commit 5893b80 into ManageIQ:master Mar 20, 2017
@gmcculloug gmcculloug added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 20, 2017
@andyvesel andyvesel deleted the expose_attach_detach_for_cloud_volume branch March 21, 2017 09:05
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