InventoryCollection Builder improvements #17621

slemrmartin · 2018-06-21T11:54:13Z

Issue: #17396

dependent : Persister: optimized InventoryCollection definitions manageiq-providers-amazon#456
dependent : Persister: optimized InventoryCollection definitions manageiq-providers-azure#271
dependent : Persister: optimized InventoryCollection definitions manageiq-providers-openstack#307
dependent: Persister: optimized InventoryCollection definitions manageiq-providers-ovirt#268
dependent: Persister: InventoryCollection building through add_collection() manageiq-providers-redfish#14
dependent: Persister: InventoryCollection building through add_collection() manageiq-providers-kubevirt#104
dependent: Persister: InventoryCollection building through add_collection() manageiq-providers-kubernetes#265
dependent: Persister: InventoryCollection building through add_collection() manageiq-providers-openshift#100

Renaming builder_params => default_values (partially)
Merged common definitions from provider specific

slemrmartin · 2018-06-25T12:46:02Z

cc @agrare

Renaming builder_params => default_values (partially) Merged common definitions from provider specific

Derives attributes correctly if you specify model_class as param of add_collection()

Added support for PhysicalInfra - Redfish

`

Kubernetes support

slemrmartin · 2018-07-02T13:22:34Z

Cc @Ladas

Ladas · 2018-07-04T11:56:13Z

app/models/manager_refresh/inventory_collection/builder.rb

@@ -28,7 +29,7 @@ def self.default_options

      # Entry point
      # Creates builder and builds data for inventory collection
-      # @param name [Symbol] InventoryCollection.association value
+      # @param name [Symbol || Array] InventoryCollection.association value. <name> method not called when Array


nit: I think the YARD syntax is [Symbol, Array]

Ladas · 2018-07-04T11:57:38Z

app/models/manager_refresh/inventory_collection/builder.rb

-              value
-            end
+      # Evaluates lambda blocks in @default_values and @dependency_attributes
+      # @param values[Hash]


nit: missing space values [Hash]

Ladas · 2018-07-04T12:02:04Z

app/models/manager_refresh/inventory_collection/builder/cloud_manager.rb

-              end
-            end
-          end
+          @options[:auto_inventory_attributes] = false


I am not much of fan of instance variable interface, could we have some setter that would set the options just for this 1 InventoryCollection? Otherwise there can be hard to find side effects

Ladas · 2018-07-04T12:03:17Z

app/models/manager_refresh/inventory_collection/builder/container_manager.rb

+  class InventoryCollection
+    class Builder
+      class ContainerManager < ::ManagerRefresh::InventoryCollection::Builder
+        # TODO: (agrare) Targeted refreshes will require adjusting the associations / arels. (duh)


@agrare I wonder what you've meant by this :-)

Ha I'm pretty sure I didn't write this, let me track down where it came from

Looks like it was @cben, I took his changes from #14337 and applied to the new repo after it was split out here ManageIQ/manageiq-providers-kubernetes#30

Link to the original comment: https://github.com/ManageIQ/manageiq/pull/14337/files#diff-65533e4742c2672e65b884331a5ad673R43

@agrare sorry I looked only to annotation, I'll change it

Ladas · 2018-07-04T12:05:19Z

app/models/manager_refresh/inventory_collection/builder/persister_helper.rb

@@ -19,7 +19,7 @@ module ManagerRefresh::InventoryCollection::Builder::PersisterHelper
  #     )
  #   )
  #
-  # @see ManagerRefresh::InventoryCollection::Builder
+  # @see documentation https://github.com/ManageIQ/guides/tree/master/providers/persister/inventory_collections.md


Ladas

Few nits, otherwise looks awesome.

For auto_inventory_attributes and without_model_class

miq-bot · 2018-07-04T12:47:44Z

Checked commits slemrmartin/manageiq@813af66~...f46fb65 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
10 files checked, 1 offense detected

app/models/manager_refresh/inventory_collection/builder/cloud_manager.rb

❗ - Line 156, Col 21 - Rails/SkipsModelValidations - Avoid using update_attribute because it skips validations.

agrare · 2018-07-10T19:56:45Z

app/models/manager_refresh/inventory_collection/builder.rb


      # Evaluates lambda blocks
      def evaluate_lambdas!(persister)
-        evaluate_builder_params_lambdas!(persister)
+        @default_values = evaluate_lambdas_on(@default_values, persister)
+        @dependency_attributes = evaluate_lambdas_on(@dependency_attributes, persister)


👍 this seems much nicer than evaluate_builder_params_lambdas!

agrare · 2018-07-10T20:08:31Z

@slemrmartin this looks good to me, I assume we'll need to merge this and the rest of the provider PRs together since at least the builder_params change is breaking?

Can you run the refresher specs for all of the providers locally before I merge this to make sure we don't run into any surprises when we kick the rest of the PRs?

slemrmartin · 2018-07-11T11:42:48Z

@agrare I added an alias to add_builder_params so providers specs will work before and after merge of providers PRs.
:builder_params symbol will be refactored later, it influences more than builder and definitions.

All providers tested locally without problems

agrare · 2018-07-11T12:13:27Z

@slemrmartin ah you're right I totally missed that alias, 👍

slemrmartin mentioned this pull request Jun 21, 2018

InventoryCollection Builder #17396

Closed

33 tasks

miq-bot added the wip label Jun 21, 2018

agrare self-assigned this Jun 25, 2018

slemrmartin mentioned this pull request Jun 25, 2018

Persister: optimized InventoryCollection definitions ManageIQ/manageiq-providers-ovirt#268

Merged

1 task

slemrmartin changed the title ~~[WIP] InventoryCollection Builder improvements~~ InventoryCollection Builder improvements Jun 25, 2018

miq-bot removed the wip label Jun 25, 2018

slemrmartin changed the title ~~InventoryCollection Builder improvements~~ [WIP] InventoryCollection Builder improvements Jun 25, 2018

miq-bot added the wip label Jun 25, 2018

slemrmartin added 8 commits July 2, 2018 10:00

IC Builder improvements

813af66

Renaming builder_params => default_values (partially) Merged common definitions from provider specific

Next builder_params => default_values renames

8d4487b

Fixes IC automatic inventory_object attributes

cc92de0

Derives attributes correctly if you specify model_class as param of add_collection()

IC Builder for Redfish

3f698f5

Added support for PhysicalInfra - Redfish

Link to documentation of add_collections()

f73fb10

`

Support for Kubevirt's persister#add_collection()

a459dcf

Added IC definition from Ovirt

ca4076d

Added support for Container persisters

cefcc34

slemrmartin force-pushed the ic-builder-optimalization branch from 4f286cb to 1aac036 Compare July 2, 2018 12:56

slemrmartin mentioned this pull request Jul 2, 2018

Persister: InventoryCollection building through add_collection() ManageIQ/manageiq-providers-kubernetes#265

Merged

1 task

Allowed IC association name as Array

5e18027

Kubernetes support

slemrmartin force-pushed the ic-builder-optimalization branch from 1aac036 to 5e18027 Compare July 2, 2018 13:21

slemrmartin changed the title ~~[WIP] InventoryCollection Builder improvements~~ InventoryCollection Builder improvements Jul 2, 2018

miq-bot removed the wip label Jul 2, 2018

slemrmartin changed the title ~~InventoryCollection Builder improvements~~ [WIP] InventoryCollection Builder improvements Jul 2, 2018

miq-bot added the wip label Jul 2, 2018

slemrmartin mentioned this pull request Jul 3, 2018

Persister: InventoryCollection building through add_collection() ManageIQ/manageiq-providers-openshift#100

Merged

2 tasks

slemrmartin changed the title ~~[WIP] InventoryCollection Builder improvements~~ InventoryCollection Builder improvements Jul 3, 2018

slemrmartin mentioned this pull request Jul 3, 2018

ContainerImage deleting #17664

Merged

1 task

miq-bot removed the wip label Jul 3, 2018

OpenShift IC definition fixes

86d29e8

Ladas reviewed Jul 4, 2018

View reviewed changes

Ladas approved these changes Jul 4, 2018

View reviewed changes

Yard fixes

d9ed848

slemrmartin force-pushed the ic-builder-optimalization branch from ef78119 to d9ed848 Compare July 4, 2018 12:12

IC Builder options skip methods

f46fb65

For auto_inventory_attributes and without_model_class

agrare reviewed Jul 10, 2018

View reviewed changes

agrare added enhancement providers/inventory gaprindashvili/no labels Jul 11, 2018

agrare approved these changes Jul 11, 2018

View reviewed changes

agrare merged commit 9036816 into ManageIQ:master Jul 11, 2018

agrare added this to the Sprint 90 Ending Jul 16, 2018 milestone Jul 11, 2018

jameswnl mentioned this pull request Jul 11, 2018

automation_manager should use :manager(_id) instead of :ems_id #17694

Merged

slemrmartin deleted the ic-builder-optimalization branch July 26, 2018 08:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

InventoryCollection Builder improvements #17621

InventoryCollection Builder improvements #17621

slemrmartin commented Jun 21, 2018 •

edited

Loading

slemrmartin commented Jun 25, 2018

slemrmartin commented Jul 2, 2018

Ladas Jul 4, 2018 •

edited

Loading

Ladas Jul 4, 2018 •

edited

Loading

Ladas Jul 4, 2018

Ladas Jul 4, 2018

agrare Jul 10, 2018

agrare Jul 10, 2018

slemrmartin Jul 11, 2018

Ladas Jul 4, 2018

Ladas left a comment

miq-bot commented Jul 4, 2018

agrare Jul 10, 2018

agrare commented Jul 10, 2018

slemrmartin commented Jul 11, 2018

agrare commented Jul 11, 2018

InventoryCollection Builder improvements #17621

InventoryCollection Builder improvements #17621

Conversation

slemrmartin commented Jun 21, 2018 • edited Loading

slemrmartin commented Jun 25, 2018

slemrmartin commented Jul 2, 2018

Ladas Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Ladas Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Ladas Jul 4, 2018

Choose a reason for hiding this comment

Ladas Jul 4, 2018

Choose a reason for hiding this comment

agrare Jul 10, 2018

Choose a reason for hiding this comment

agrare Jul 10, 2018

Choose a reason for hiding this comment

slemrmartin Jul 11, 2018

Choose a reason for hiding this comment

Ladas Jul 4, 2018

Choose a reason for hiding this comment

Ladas left a comment

Choose a reason for hiding this comment

miq-bot commented Jul 4, 2018

agrare Jul 10, 2018

Choose a reason for hiding this comment

agrare commented Jul 10, 2018

slemrmartin commented Jul 11, 2018

agrare commented Jul 11, 2018

slemrmartin commented Jun 21, 2018 •

edited

Loading

Ladas Jul 4, 2018 •

edited

Loading

Ladas Jul 4, 2018 •

edited

Loading