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

Refactoring and fixing cloud/vm/provisioning/placement/best_fit_amazon method. #63

Merged
merged 3 commits into from
Mar 7, 2017
Merged

Refactoring and fixing cloud/vm/provisioning/placement/best_fit_amazon method. #63

merged 3 commits into from
Mar 7, 2017

Conversation

pkomanek
Copy link
Contributor

@pkomanek pkomanek commented Mar 1, 2017

Purpose or Intent

Refactoring ManageIQ/Cloud/VM/Provisioning/Placement.class/methods/best_fit_amazon.rb method, adding new spec and fix bug with getting error. This PR is based on the issue bellow.

Links

#8
https://bugzilla.redhat.com/show_bug.cgi?id=1425068

find_by method returns nil instead of raising error.
@miq-bot
Copy link
Member

miq-bot commented Mar 1, 2017

Checked commits pkomanek/manageiq-content@9005a5e~...a522fc9 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. ⭐

@pkomanek pkomanek changed the title [WIP]Refactoring and fixing cloud/vm/provisioning/placement/best_fit_amazon method. Refactoring and fixing cloud/vm/provisioning/placement/best_fit_amazon method. Mar 1, 2017
@miq-bot miq-bot removed the wip label Mar 1, 2017
instance_id = prov.get_option(:instance_type)
raise "Instance Type not specified" if instance_id.nil?

flavor = @handle.vmdb('flavor').find_by(:id => instance_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

@pkomanek
should this be a find_by!

Copy link
Contributor

Choose a reason for hiding this comment

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

@pkomanek I think flavor is optional

Copy link
Contributor

Choose a reason for hiding this comment

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

so we can't raise an exception

@mkanoor mkanoor merged commit 018e6b5 into ManageIQ:master Mar 7, 2017
@mkanoor mkanoor added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 7, 2017
@pkomanek pkomanek deleted the refactoring_cloud_vm_provisioning_placement/best_fit_amazon_method branch March 8, 2017 13:41
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.

4 participants