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
Fixes #20881 - Clean legacy api from discovery #374
Conversation
Issues: #20881 |
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.
Looks good let's merge then!
@@ -180,7 +180,7 @@ def test_reboot_success_legacy | |||
host = discover_host_from_facts(facts) | |||
Host::Discovered::any_instance.stubs(:proxied?).returns(false) | |||
Host::Discovered::any_instance.stubs(:proxy_url).returns("http://1.2.3.4:8443") | |||
::ForemanDiscovery::NodeAPI::PowerLegacyDirectService.any_instance.expects(:reboot).returns(true) | |||
::ForemanDiscovery::NodeAPI::PowerService.any_instance.expects(:reboot).returns(true) | |||
ActiveSupport::Deprecation.silence do | |||
post "reboot", { :id => host.id }, set_session_user | |||
end |
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.
There is also test_reboot_success_legacy test, can you remove that? It is useless I think. Other that that we can merge!
3ec3b83
to
a720a87
Compare
def legacy_direct_service(data) | ||
ForemanDiscovery::NodeAPI::PowerLegacyDirectService.new(data) | ||
end | ||
|
||
def legacy_proxied_service(data) | ||
ForemanDiscovery::NodeAPI::PowerLegacyProxiedService.new(data) | ||
end |
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.
The legacy_proxied_service can go away too, it should be unused.
::ProxyAPI::BMC.new(:url => url).power :action => "cycle" | ||
end | ||
end | ||
end |
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.
Also remove the legacy_proxied.service.rb file please.
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.
@lzap good catch, thanks :)
a720a87
to
e89c844
Compare
Ok we are good to go once it is all green. |
I retriggered failed develop: http://ci.theforeman.org/job/test_plugin_foreman_discovery_develop/324/ |
Foreman core fixed so [test] |
We can hopefully [test] now. |
e89c844
to
c5357c8
Compare
One failure in core is unrelated. Thanks. |
No description provided.