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

Fixes #26969 - Allow using remote core even if core gem is available #63

Merged
merged 4 commits into from
Jun 11, 2019

Conversation

adamruzicka
Copy link
Contributor

The behavior can be configured using the external_core key in the settings.

If unset, the behavior will remain the same, eg. use internal core if available,
fallback to external core otherwise. Setting it to true will force the proxy to
always use an external core. Setting it to false will make the proxy try to
always use the internal core and disable the feature if it is not available.

# by standalone Dynflow core
end
# Ensure the core gem is loaded, if configure NOT to use the external core
require 'smart_proxy_dynflow_core' if Proxy::Dynflow::Plugin.settings.external_core == false
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
require 'smart_proxy_dynflow_core' if Proxy::Dynflow::Plugin.settings.external_core == false
require 'smart_proxy_dynflow_core' unless Proxy::Dynflow::Plugin.settings.external_core

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing is I need to distinguish between false and nil. But maybe I'm just overthinking things and this whole three-valued logic is not needed at all

Copy link
Member

Choose a reason for hiding this comment

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

Since there's duplication in the http_config, should we have a helper? Can we reuse the output of this loading somehow?

Copy link
Member

Choose a reason for hiding this comment

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

Btw. would this work without Debian packaging/installer changes to explicitly disable the loading?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, by default the value will be unset. This will make the proxy try loading the core, use it if available and fallback to external core otherwise.

lib/smart_proxy_dynflow/plugin.rb Outdated Show resolved Hide resolved
The behavior can be configured using the external_core key in the
settings.

If unset, the behavior will remain the same, eg. use internal
core if available, fallback to external core otherwise. Setting
it to true will force the proxy to always use an external core.
Setting it to false will make the proxy try to always use the
internal core and disable the feature if it is not available.
Copy link
Member

@iNecas iNecas left a comment

Choose a reason for hiding this comment

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

Tested and works well. Thanks @adamruzicka

@iNecas iNecas merged commit 83413dc into theforeman:master Jun 11, 2019
@iNecas
Copy link
Member

iNecas commented Jun 11, 2019

smart_proxy_dynflow 0.2.3 released to rubygems and packaging PRs are theforeman/foreman-packaging#3849 and theforeman/foreman-packaging#3850

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