-
Notifications
You must be signed in to change notification settings - Fork 79
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
Remove oVirt provider code #118
Remove oVirt provider code #118
Conversation
Currently the part of the oVirt provider that performs event monitoring and inventory collection (using version 3 of the oVirt API and the 'ovirt' gem) lives in this repository, in the 'lib/gems/pending/ovirt_provider' directory. As the oVirt provider has been extracted to a separate repository, it is convenient to move that code there. The relevant copy has been added to that repository in this pull request: Move code from manageiq-gems-pending ManageIQ/manageiq-providers-ovirt#9 This patch removes the code from this repository. Note that part of the code can't be moved, or rather, isn't convinient to move it now. The reason is that code, the 'rhevmTest.rb' file, for example, doesn't conceptually belong into the oVirt provider. To keep this code in this repository it is necessary to add a dependency, so that 'manageiq-gems-pending' will now depend on 'manageiq-providers-ovirt'. ManageIQ/manageiq-providers-ovirt#8 Signed-off-by: Juan Hernandez <[email protected]>
@durandom @Fryguy please review. Note that I am not certain about what to do with the Adding the oVirt provider as a dependency of |
Note that this shouldn't be merged before ManageIQ/manageiq-providers-ovirt#9. |
@bdunne Are those tests that are actually run? I'm also not sure what to do with them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM. Marked as WIP to prevent merge before ManageIQ/manageiq-providers-ovirt#9
@durandom I wouldn't move that. It's related to fleecing code and needs to adapt to pluggable providers. Although, updating the |
@@ -7,6 +7,10 @@ gemspec | |||
gem "handsoap", "~>0.2.5", :require => false, :git => "https://github.com/ManageIQ/handsoap.git", :tag => "v0.2.5-5" | |||
gem "rubywbem", :require => false, :git => "https://github.com/ManageIQ/rubywbem.git", :branch => "rubywbem_0_1_0" | |||
|
|||
# This is needed because some of the smart analysis tests scripts, insde the 'lib/gems/pending/MiqVm/test' directory, | |||
# use the oVirt provider inventory classes. | |||
gem "manageiq-providers-ovirt", :require => false, :git => "https://github.com/ManageIQ/manageiq-providers-ovirt.git", :branch => "master" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this is merged, we'll need to open GH issue in manageiq-releasen repo to update the branch name when we cut gaprindashvili.
@@ -51,7 +51,6 @@ Gem::Specification.new do |s| | |||
s.add_runtime_dependency "nokogiri", "~>1.6.8" | |||
s.add_runtime_dependency "openscap", "~>0.4.3" | |||
s.add_runtime_dependency "ovirt", "~>0.15.1" | |||
s.add_runtime_dependency "parallel", "~>1.9" # For OvirtInventory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bdunne Even though this is being removed and the comment says "For OvirtInventory", can you please verify this is needed by anyone else that was getting it automatically? azure comes to mind, and also the core repo, but I'm not sure what else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 @bdunne Please coordinatethe merges.
@jhernand With ManageIQ/manageiq-providers-ovirt#9 merged, are you ready to remove the From what I see, the only other references to |
:) I marked as WIP, unmarking and merging. |
Currently the part of the oVirt provider that performs event monitoring
and inventory collection (using version 3 of the oVirt API and the
'ovirt' gem) lives in this repository, in the
'lib/gems/pending/ovirt_provider' directory. As the oVirt provider has
been extracted to a separate repository, it is convenient to move that
code there. The relevant copy has been added to that repository in this
pull request:
Move code from manageiq-gems-pending
ManageIQ/manageiq-providers-ovirt#9
This patch removes the code from this repository.
Note that part of the code can't be moved, or rather, isn't convinient
to move it now. The reason is that code, the 'rhevmTest.rb' file, for
example, doesn't conceptually belong into the oVirt provider. To keep
this code in this repository it is necessary to add a dependency, so
that 'manageiq-gems-pending' will now depend on
'manageiq-providers-ovirt'.
ManageIQ/manageiq-providers-ovirt#8