Pluggable automate domains #11083

bdunne · 2016-09-07T21:04:29Z

Allow provider plugins to carry their own automate domains. When resetting domains, those should be included in the reset.

If the plugin doesn't start with "ManageIQ::Providers::", the engine.rb should include:

def vmdb_plugin?
  true
end

Fryguy · 2016-09-07T21:21:02Z

@durandom Please also review.

Fryguy · 2016-09-07T21:22:23Z

lib/miq_automation_engine/models/miq_ae_datastore.rb

+  def self.reset_domains_from_provider_gems
+    provider_gems = Vmdb::Plugins.instance.provider_plugins
+    provider_gems.each do |provider_gem|
+      domain_bucket = provider_gem.root.join("db", "fixtures", "ae_datastore")


👍 on convention over configuration here.

Wondering if this should be moved out to a common method, since the "built-in" domains are also in the same path (thus duplicating the path generation code)

Fryguy · 2016-09-07T21:24:42Z

@durandom Please review with respect to the plugins module added here...I think that might overlap a bit with what you were doing in #10944 with the same end goal of not having the plugin itself specify the descendat_loader paths.

chessbyte · 2016-09-07T21:36:11Z

/cc @blomquisg @gmcculloug

durandom · 2016-09-08T14:24:50Z

@bdunne have you checked my approach in #10944 ?

I find it more intuitive by asking a Provider subclass if its an engine and then getting that engine by e.g. Amazon::Provider.engine . That also enforces gems to subclass the Provider class, which now somewhat optional :)

Also see my comment here I think its safe to assume all providers are under that namespace, so it's one method less to define.

bdunne · 2016-09-08T14:30:33Z

@durandom The only problem that I see with that approach is if we want to have plugins that provide specific features (like an automate domain and some models), but not a provider subclass, you won't find the plugin in the same way.

durandom · 2016-09-08T14:58:51Z

plugins that provide specific features, but not a provider subclass

Is that going to happen in the near term? If not, I would go with simpler approach and with adding less boilerplate code baggage to the provider gems.

bdunne · 2016-09-08T15:05:41Z

I have two current use cases where I would like to see that happen soon.

Extract the default ManageIQ automate domain to its own repo
Downstream productization (provide an automate domain and a few non-provider model subclasses)

durandom · 2016-09-09T09:01:21Z

config/initializers/register_provider_plugins.rb

@@ -0,0 +1,3 @@
+Rails.application.railties.each do |railtie|
+  Vmdb::Plugins.instance.register_provider_plugin(railtie) if railtie.try(:manageiq_provider_plugin?)


How about adding a convention like approach here too:

if railtie.class.name.start_with? "ManageIQ::Providers::" || railtie.try(:manageiq_provider_plugin?)

this way all engines under this namespace get registered automatically

durandom · 2016-09-09T09:06:53Z

@bdunne ok, then the naming of Vmdb::Plugins also makes sense, because its not only providers that are plugged.

So I favor this approach over mine in #10944
It would be great if you add the addition based on the namespace, too. Because I want to it to be super easy to create a provider gem :)

durandom · 2016-09-15T14:04:34Z

@bdunne wdy think about my #11083 (comment) ?
I'd like to get this in soon, to continue working on moving settings.yml over to the provider repo.

durandom

+1 for the Vmdb::Plugins idea. Would be great to auto-registers Rails engines under the ManageIQ::Providers Namespace, but I can do this in a follow up PR too.

I cant comment on the reset_domains.. part though :(

bdunne · 2016-09-16T16:28:49Z

I added the change @durandom suggested to reduce engine setup knowledge and changed to register_vmdb_plugin since it is not necessarily a provider

durandom

looks good to me.

durandom · 2016-09-20T12:57:27Z

lib/miq_automation_engine/models/miq_ae_datastore.rb

    restore_attrs_for_domains(saved_attrs)
    reset_default_namespace
  end

+  def self.reset_domains_from_provider_gems
+    provider_gems = Vmdb::Plugins.instance.vmdb_plugins


only minor, but maybe bring this and the method name in line with vmdb_plugins which are not necessarily providers?

durandom · 2016-09-20T12:59:44Z

@miq-bot assign Fryguy

@Fryguy merge?

gmcculloug · 2016-10-04T00:16:32Z

cc @mkanoor

durandom · 2016-10-04T10:19:04Z

@bdunne do you want this in euwe?

bdunne · 2016-10-06T13:52:57Z

🐢

Fryguy · 2016-10-26T21:04:50Z

#12221 is merged.

miq-bot · 2016-10-26T21:07:09Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

miq-bot · 2016-12-13T14:22:06Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

durandom · 2016-12-15T09:14:54Z

lib/vmdb/plugins/automate_domain.rb

@@ -12,6 +12,10 @@ def initialize(path)
        @name            = config.fetch_path("object", "attributes", "name")
      end

+      def system?
+        @system ||= config.fetch_path("object", "attributes", "source") == "system"


not necessarily bad, but if this evaluates to false it'll be evaluated next time it's called - making the memoization useless. Maybe just drop @system

hmm, looks like this commit isnt in the PR anymore, so ignore :)

miq-bot · 2016-12-15T16:31:59Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

Extra domains may cause test failures because of changed behavior

miq-bot · 2016-12-15T21:23:37Z

Checked commits bdunne/manageiq@633197d~...8ada346 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
8 files checked, 0 offenses detected
Everything looks good. 🍰

Fryguy reviewed Sep 7, 2016
View reviewed changes

chessbyte added enhancement automate providers labels Sep 7, 2016

chessbyte assigned durandom Sep 7, 2016

blomquisg added the pluggability/providers label Sep 8, 2016

durandom reviewed Sep 9, 2016
View reviewed changes

durandom mentioned this pull request Sep 9, 2016

[WIP] Settings for providers #10944

Merged

durandom approved these changes Sep 15, 2016

View reviewed changes

bdunne force-pushed the pluggable_automate branch 2 times, most recently from ebefd4f to 9af98a0 Compare September 16, 2016 16:20

durandom approved these changes Sep 20, 2016

View reviewed changes

miq-bot assigned Fryguy and unassigned durandom Sep 20, 2016

durandom mentioned this pull request Sep 21, 2016

Extract factories to provider gems #11413

Merged

bdunne added the euwe/yes label Oct 4, 2016

durandom mentioned this pull request Dec 2, 2016

Move from MIQ repo to provider repo. ManageIQ/manageiq-providers-azure#16

Merged

bdunne force-pushed the pluggable_automate branch from 9af98a0 to 048972a Compare December 12, 2016 21:47

miq-bot added the dependencies label Dec 12, 2016

miq-bot added the unmergeable label Dec 13, 2016

bdunne force-pushed the pluggable_automate branch from a2a0ea8 to 012e5ac Compare December 14, 2016 19:05

miq-bot removed the unmergeable label Dec 14, 2016

bdunne force-pushed the pluggable_automate branch 2 times, most recently from c32b309 to 9533b10 Compare December 14, 2016 23:02

Fryguy approved these changes Dec 14, 2016

View reviewed changes

durandom reviewed Dec 15, 2016

View reviewed changes

miq-bot added the unmergeable label Dec 15, 2016

bdunne force-pushed the pluggable_automate branch from 9533b10 to 8339493 Compare December 15, 2016 21:04

miq-bot removed the unmergeable label Dec 15, 2016

bdunne added 6 commits December 15, 2016 16:19

Remove the ManageIQ automate domain

633197d

Leverage extracted ManageIQ automate domain from manageiq-content

5250e1c

When testing, we only want to reset specific automate domains

50522cf

Extra domains may cause test failures because of changed behavior

Remove Automation test suite

eac1f02

Fix finding the default_automate_domains

2f01cf9

Fix support for legacy DATASTORE_DIRECTORY

8ada346

bdunne force-pushed the pluggable_automate branch from 8339493 to 8ada346 Compare December 15, 2016 21:19

Fryguy merged commit 44bfe35 into ManageIQ:master Dec 16, 2016

Fryguy added euwe/no and removed euwe/yes labels Dec 16, 2016

Fryguy added this to the Sprint 51 Ending Jan 2, 2017 milestone Dec 16, 2016

bdunne deleted the pluggable_automate branch December 16, 2016 16:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pluggable automate domains #11083

Pluggable automate domains #11083

bdunne commented Sep 7, 2016 •

edited

Loading

Fryguy commented Sep 7, 2016

Fryguy Sep 7, 2016

Fryguy Sep 7, 2016

Fryguy commented Sep 7, 2016

chessbyte commented Sep 7, 2016

durandom commented Sep 8, 2016

bdunne commented Sep 8, 2016 •

edited

Loading

durandom commented Sep 8, 2016

bdunne commented Sep 8, 2016

durandom Sep 9, 2016 •

edited

Loading

durandom commented Sep 9, 2016

durandom commented Sep 15, 2016

durandom left a comment

bdunne commented Sep 16, 2016

durandom left a comment

durandom Sep 20, 2016

durandom commented Sep 20, 2016

gmcculloug commented Oct 4, 2016

durandom commented Oct 4, 2016

bdunne commented Oct 6, 2016

Fryguy commented Oct 26, 2016

miq-bot commented Oct 26, 2016

miq-bot commented Dec 13, 2016

durandom Dec 15, 2016

durandom Dec 15, 2016

miq-bot commented Dec 15, 2016

miq-bot commented Dec 15, 2016

		@@ -0,0 +1,3 @@
		Rails.application.railties.each do \|railtie\|
		Vmdb::Plugins.instance.register_provider_plugin(railtie) if railtie.try(:manageiq_provider_plugin?)

Pluggable automate domains #11083

Pluggable automate domains #11083

Conversation

bdunne commented Sep 7, 2016 • edited Loading

Fryguy commented Sep 7, 2016

Fryguy Sep 7, 2016

Choose a reason for hiding this comment

Fryguy Sep 7, 2016

Choose a reason for hiding this comment

Fryguy commented Sep 7, 2016

chessbyte commented Sep 7, 2016

durandom commented Sep 8, 2016

bdunne commented Sep 8, 2016 • edited Loading

durandom commented Sep 8, 2016

bdunne commented Sep 8, 2016

durandom Sep 9, 2016 • edited Loading

Choose a reason for hiding this comment

durandom commented Sep 9, 2016

durandom commented Sep 15, 2016

durandom left a comment

Choose a reason for hiding this comment

bdunne commented Sep 16, 2016

durandom left a comment

Choose a reason for hiding this comment

durandom Sep 20, 2016

Choose a reason for hiding this comment

durandom commented Sep 20, 2016

gmcculloug commented Oct 4, 2016

durandom commented Oct 4, 2016

bdunne commented Oct 6, 2016

Fryguy commented Oct 26, 2016

miq-bot commented Oct 26, 2016

miq-bot commented Dec 13, 2016

durandom Dec 15, 2016

Choose a reason for hiding this comment

durandom Dec 15, 2016

Choose a reason for hiding this comment

miq-bot commented Dec 15, 2016

miq-bot commented Dec 15, 2016

bdunne commented Sep 7, 2016 •

edited

Loading

bdunne commented Sep 8, 2016 •

edited

Loading

durandom Sep 9, 2016 •

edited

Loading