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

Add registered_provider_plugins to Vmdb::Plugins #13983

Merged

Conversation

durandom
Copy link
Member

this allows for querying which providers are registered via plugins.
it also provides a helper method to query for provider names of all registered providers

extracted from #13942

@miq-bot add_labels pluggable providers, enhancement
@miq-bot assign @bdunne

this allows for querying which providers are registered
via plugins

it also provides a helper method to query for provider names
of all registered providers
def register_automate_domains(engine)
Dir.glob(engine.root.join("content", "automate", "*")).each do |domain_directory|
@registered_automate_domains << AutomateDomain.new(domain_directory)
def provider_plugin_names
Copy link
Member

Choose a reason for hiding this comment

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

We usually implement something like this as:

def provider_plugin_names(refresh = false)
  @provider_plugin_names = nil if refresh
  @provider_plugin_names ||= .....
end

def register_provider_plugin(engine)
if engine.class.name.start_with?("ManageIQ::Providers::")
@registered_provider_plugins << engine
@provider_plugins_names = nil
Copy link
Member

Choose a reason for hiding this comment

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

It feels strange to wipe this cache in this method. Based on the comment above, you could instead call provider_plugin_names(true)

@registered_automate_domains << AutomateDomain.new(domain_directory)
def provider_plugin_names
@provider_plugins_names ||= registered_provider_plugins.collect do |plugin|
ManageIQ::Providers::Inflector.provider_name(plugin).downcase.to_sym
Copy link
Member

Choose a reason for hiding this comment

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

Should this be underscore rather than downcase to deal with things like AnsibleTower in the future?

@durandom
Copy link
Member Author

@bdunne addressed your feedback - thanks for catching the underscore issue
also changed to storing the provider plugins in a hash to circumvent the cache invalidation issue

@miq-bot
Copy link
Member

miq-bot commented Feb 20, 2017

Checked commits durandom/manageiq@edc86e2~...9e6d72f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍰

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.

👍 LGTM. @Fryguy Any concerns?

@Fryguy Fryguy merged commit 8f883cd into ManageIQ:master Feb 20, 2017
@Fryguy Fryguy added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 20, 2017
@durandom durandom deleted the vmdb_plugins_add_registered_providers branch February 20, 2017 19:06
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