Adding switches support for physical infra #16948

saulotoledo · 2018-02-05T13:30:36Z

This PR adds switches support for physical infra.

Depends on:

Updating switches data manageiq-schema#160 [Merged]

saulotoledo · 2018-02-16T20:52:18Z

@Fryguy, could you take a look at this PR?

bdunne · 2018-02-28T16:12:20Z

app/models/switch.rb

+  has_one :hardware, :dependent => :destroy
+
+  belongs_to :ext_management_system, :foreign_key => :ems_id, :inverse_of => :switches,
+    :class_name => "ManageIQ::Providers::PhysicalInfraManager"


I think this :class_name is too specific for the base Switch model.

Hi @bdunne. Sorry for the delay. I am moving that to the provider and changing here to point to "ExtManagementSystem".

@bdunne, I've updated the code. What do you think about?

agrare · 2018-03-21T12:54:55Z

app/models/switch.rb

+  has_one :hardware, :dependent => :destroy
+
+  belongs_to :ext_management_system, :foreign_key => :ems_id, :inverse_of => :switches,
+    :class_name => "ExtManagementSystem"


I don't think class_name is needed here

agrare · 2018-03-21T12:56:23Z

app/models/manageiq/providers/physical_infra_manager.rb

@@ -2,7 +2,10 @@ module ManageIQ::Providers
  class PhysicalInfraManager < BaseManager
    include SupportsFeatureMixin

+    has_many :switches, :foreign_key => "ems_id", :dependent => :destroy, :inverse_of => :ext_management_system


@Fryguy didn't want to have two providers with the same named association InfraManager.switches actually representing two different things, can you name this something like physical_switches and add a :class_name => "Switch"?

Yes, I think I can do something like that. I am preparing some changes according to the comment below. I will do that change and will update this thread.

saulotoledo · 2018-03-21T12:59:26Z

@bdunne, @agrare, @rodneyhbrown7, a PR changing the way we consider asset_details will be merged soon, the schema is already ready for physical servers was merged. Since switches is a new concept here, I will do the same changes here and in the parser before we merge.

Also, I am considering to add a new model called PhysicalSwitches. Thus, it is possible to have a different controller for the physical infra switches. I will remove the WIP prefix as soon I have updates here and I will let you know for review.

saulotoledo · 2018-03-28T00:04:02Z

@bdunne, @agrare, @rodneyhbrown7, this is ok for review.

I can run without ManageIQ/manageiq-schema#180. Can you help me to confirm if it is a dependency?

agrare · 2018-03-29T12:29:06Z

@saulotoledo I don't think ManageIQ/manageiq-schema#180 is a dependency of this, it looks like you have new vcrs in the -lenovo parsing PR, I'll test this out in the next day or two.

miq-bot · 2018-04-04T18:31:10Z

This pull request is not mergeable. Please rebase and repush.

agrare · 2018-04-05T19:51:44Z

app/models/hardware.rb

@@ -4,6 +4,7 @@ class Hardware < ApplicationRecord
  belongs_to  :miq_template,  :foreign_key => :vm_or_template_id
  belongs_to  :host
  belongs_to  :computer_system
+  belongs_to  :physical_switch


do you need a :foreign_key => :switch_id here?

I've added that in the PhysicalSwitch object. Do not should be there? Also, since I am using :foreign_key, it will work if I add the :inverse_of as requested by the RuboCop?

I get this error without it

>> hardware.physical_switch = physical_switch ActiveModel::MissingAttributeError: can't write unknown attribute `physical_switch_id` from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/attribute.rb:182:in `with_value_from_database' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/attribute_set.rb:53:in `write_from_user' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/attribute_methods/write.rb:50:in `write_attribute_with_type_cast' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/attribute_methods/write.rb:32:in `write_attribute' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/attribute_methods.rb:361:in `[]=' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/associations/belongs_to_association.rb:76:in `replace_keys' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/associations/belongs_to_association.rb:14:in `replace' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/associations/singular_association.rb:22:in `writer' from /home/agrare/.gem/ruby/2.4.3/gems/activerecord-5.0.7/lib/active_record/associations/builder/association.rb:119:in `physical_switch=' from (irb):4

It needs to work from both sides of the association

You're right. Thanks! It is fixed now.

miq-bot · 2018-04-06T13:01:01Z

Checked commit saulotoledo@b963a8f with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
6 files checked, 0 offenses detected
Everything looks fine. ⭐

miq-bot added the wip label Feb 5, 2018

saulotoledo force-pushed the add_physical_switch_support branch 6 times, most recently from 39ada3d to 41099c9 Compare February 7, 2018 15:07

saulotoledo force-pushed the add_physical_switch_support branch from 41099c9 to e6f86d7 Compare February 16, 2018 17:25

This was referenced Feb 16, 2018

Updating switches data ManageIQ/manageiq-schema#160

Merged

Adding parsing steps for switches ManageIQ/manageiq-providers-lenovo#132

Merged

saulotoledo changed the title ~~[WIP] Adding switches support for physical infra~~ Adding switches support for physical infra Feb 16, 2018

miq-bot removed the wip label Feb 16, 2018

saulotoledo force-pushed the add_physical_switch_support branch from e6f86d7 to c8eee09 Compare February 27, 2018 12:29

bdunne reviewed Feb 28, 2018

View reviewed changes

saulotoledo force-pushed the add_physical_switch_support branch from c8eee09 to e20cdc3 Compare March 6, 2018 19:21

bdunne requested a review from agrare March 20, 2018 15:49

saulotoledo force-pushed the add_physical_switch_support branch from e20cdc3 to 8f0b175 Compare March 20, 2018 18:55

saulotoledo changed the title ~~Adding switches support for physical infra~~ [WIP] Adding switches support for physical infra Mar 21, 2018

agrare reviewed Mar 21, 2018

View reviewed changes

miq-bot added the wip label Mar 21, 2018

saulotoledo mentioned this pull request Mar 26, 2018

Updating details for physical switches ManageIQ/manageiq-schema#182

Merged

saulotoledo force-pushed the add_physical_switch_support branch 2 times, most recently from 699bc8e to c2e510c Compare March 27, 2018 23:37

saulotoledo changed the title ~~[WIP] Adding switches support for physical infra~~ Adding switches support for physical infra Mar 28, 2018

saulotoledo force-pushed the add_physical_switch_support branch from c2e510c to 1b33d90 Compare March 28, 2018 00:13

saulotoledo mentioned this pull request Mar 28, 2018

Adding roles, filters and configurations for Physical Switches support #17216

Merged

miq-bot removed the wip label Mar 28, 2018

saulotoledo force-pushed the add_physical_switch_support branch from 1b33d90 to 8674019 Compare March 28, 2018 23:09

gtanzillo assigned agrare Apr 2, 2018

miq-bot added the unmergeable label Apr 4, 2018

saulotoledo force-pushed the add_physical_switch_support branch 2 times, most recently from 84d1bea to 224ebe5 Compare April 5, 2018 15:47

miq-bot removed the unmergeable label Apr 5, 2018

saulotoledo force-pushed the add_physical_switch_support branch 2 times, most recently from 61fb508 to 1d8a44f Compare April 5, 2018 18:33

agrare reviewed Apr 5, 2018

View reviewed changes

saulotoledo force-pushed the add_physical_switch_support branch 5 times, most recently from ddca623 to 576ced0 Compare April 6, 2018 12:54

Adding switches support for physical infra

b963a8f

saulotoledo force-pushed the add_physical_switch_support branch from 576ced0 to b963a8f Compare April 6, 2018 12:58

agrare approved these changes Apr 6, 2018

View reviewed changes

agrare merged commit fa45a97 into ManageIQ:master Apr 6, 2018

agrare added enhancement providers/physical labels Apr 6, 2018

agrare added this to the Sprint 83 Ending Apr 9, 2018 milestone Apr 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding switches support for physical infra #16948

Adding switches support for physical infra #16948

saulotoledo commented Feb 5, 2018 •

edited

Loading

saulotoledo commented Feb 16, 2018

bdunne Feb 28, 2018

saulotoledo Mar 20, 2018

saulotoledo Mar 20, 2018

agrare Mar 21, 2018

saulotoledo Mar 28, 2018

agrare Mar 21, 2018

saulotoledo Mar 23, 2018

saulotoledo Mar 28, 2018

saulotoledo commented Mar 21, 2018 •

edited

Loading

saulotoledo commented Mar 28, 2018 •

edited

Loading

agrare commented Mar 29, 2018

miq-bot commented Apr 4, 2018

agrare Apr 5, 2018

saulotoledo Apr 5, 2018

agrare Apr 5, 2018

saulotoledo Apr 5, 2018

miq-bot commented Apr 6, 2018

Adding switches support for physical infra #16948

Adding switches support for physical infra #16948

Conversation

saulotoledo commented Feb 5, 2018 • edited Loading

saulotoledo commented Feb 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saulotoledo commented Mar 21, 2018 • edited Loading

saulotoledo commented Mar 28, 2018 • edited Loading

agrare commented Mar 29, 2018

miq-bot commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Apr 6, 2018

saulotoledo commented Feb 5, 2018 •

edited

Loading

saulotoledo commented Mar 21, 2018 •

edited

Loading

saulotoledo commented Mar 28, 2018 •

edited

Loading