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

Adjust ConversionHost support check so that it uses resource instead of singleton #600

Merged
merged 4 commits into from May 29, 2019
Merged

Adjust ConversionHost support check so that it uses resource instead of singleton #600

merged 4 commits into from May 29, 2019

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented May 28, 2019

This PR updates the ConversionHostController#create method so that the supports check happens at the instance level instead of the singleton level. I also updated the comments slightly.

Effectively replaces ManageIQ/manageiq#18799, since fiddling with the resource_type won't matter any more.

https://bugzilla.redhat.com/show_bug.cgi?id=1713394

@miq-bot
Copy link
Member

miq-bot commented May 28, 2019

Checked commits https://github.com/djberg96/manageiq-api/compare/47de45359b50f9588e0b750ad501c60e90e116cc~...d9f94537c0e356c05a8c1fed263f22a46f9b5d1f with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

@djberg96
Copy link
Contributor Author

@miq-bot add_labels transformation, hammer/yes

@@ -53,9 +53,11 @@
context "create" do
let(:zone) { FactoryBot.create(:zone) }
let(:ems_openstack) { FactoryBot.create(:ems_openstack, :zone => zone) }
let(:ems_redhat) { FactoryBot.create(:ems_redhat, :zone => zone) }
let(:ems_redhat) { FactoryBot.create(:ems_redhat, :zone => zone, :api_version => '4.2.4') }
Copy link
Member

Choose a reason for hiding this comment

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

There's an :ems_redhat_v4 factory, is 4.2.4 important?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bdunne We have a minimum api version for redhat conversion hosts of 4.2.4, and the factory is 4.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I imagine the factory could probably be updated, but didn't want to touch it for now since it could affect tests outside of v2v.

Copy link
Member

Choose a reason for hiding this comment

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

@bdunne We have a minimum api version for redhat conversion hosts of 4.2.4, and the factory is 4.0.

Okay.

I imagine the factory could probably be updated, but didn't want to touch it for now since it could affect tests outside of v2v.

No, I think that makes it even less obvious that this api version is important here.

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

1 minor comment, otherwise 👍 LGTM

@bdunne bdunne merged commit a8dc191 into ManageIQ:master May 29, 2019
@bdunne bdunne added this to the Sprint 113 Ending Jun 10, 2019 milestone May 29, 2019
@bdunne bdunne self-assigned this May 29, 2019
simaishi pushed a commit that referenced this pull request Jun 10, 2019
Adjust ConversionHost support check so that it uses resource instead of singleton

(cherry picked from commit a8dc191)

https://bugzilla.redhat.com/show_bug.cgi?id=1717023
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit dc1165c75763e22e508877062c29166c1bc3b6d5
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Wed May 29 11:53:35 2019 -0400

    Merge pull request #600 from djberg96/conversion_host_supports_feature
    
    Adjust ConversionHost support check so that it uses resource instead of singleton
    
    (cherry picked from commit a8dc1913f64950fb99b2994ceb21459bc149c566)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1717023

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.

None yet

5 participants