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 relationship between [physical switch and physical chassis] and event stream #17661

Merged
merged 1 commit into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/models/event_stream.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class EventStream < ApplicationRecord

belongs_to :middleware_server, :foreign_key => :middleware_server_id
belongs_to :physical_server
belongs_to :physical_chassis, :inverse_of => :event_streams
belongs_to :physical_switch, :inverse_of => :event_streams
Copy link
Member

Choose a reason for hiding this comment

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

I think I'm missing something here.

I didn't find physical_switch_id or physical_chassis_id in event_streams table.

Is it through physical_server?

(Not sure how many fields we can add to event_streams without things going seriously downhill)

Copy link
Member Author

@felipedf felipedf Jul 12, 2018

Choose a reason for hiding this comment

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

My bad here @kbrock, it is depended on another PR, added it to the description

Copy link
Member Author

Choose a reason for hiding this comment

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

(Not sure how many fields we can add to event_streams without things going seriously downhill)

Agree, I guess it was decided to keep things this way while we design a better architecture for events.
@agrare am I right?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah the proposed solution was worse than the problem IMO and if this really becomes an issue we can always delete the middleware relationship keys.


virtual_column :group, :type => :string
virtual_column :group_level, :type => :string
Expand Down
8 changes: 8 additions & 0 deletions app/models/physical_chassis.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
class PhysicalChassis < ApplicationRecord
include SupportsFeatureMixin
Copy link
Member

Choose a reason for hiding this comment

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

Is this related to adding the relationship to ems_events?

Copy link
Member Author

@felipedf felipedf Jul 24, 2018

Choose a reason for hiding this comment

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

It is, as EventMixin tries to execute some method from SupportsFeatureMixin on its super class, in that context PhysicalChassis

https://github.com/ManageIQ/manageiq/blob/master/app/models/mixins/event_mixin.rb#L6

Copy link
Member

Choose a reason for hiding this comment

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

Ahh okay thanks @felipedf

include EventMixin

acts_as_miq_taggable

belongs_to :ext_management_system, :foreign_key => :ems_id, :inverse_of => :physical_chassis,
:class_name => "ManageIQ::Providers::PhysicalInfraManager"
belongs_to :physical_rack, :foreign_key => :physical_rack_id, :inverse_of => :physical_chassis

has_many :event_streams, :inverse_of => :physical_chassis, :dependent => :nullify
has_many :physical_servers, :dependent => :destroy, :inverse_of => :physical_chassis

has_one :computer_system, :as => :managed_entity, :dependent => :destroy, :inverse_of => false
Expand All @@ -17,6 +21,10 @@ def my_zone
ems ? ems.my_zone : MiqServer.my_zone
end

def event_where_clause(assoc = :ems_events)
["#{events_table_name(assoc)}.physical_chassis_id = ?", id]
end

def refresh_ems
unless ext_management_system
raise _("No Provider defined")
Expand Down
8 changes: 8 additions & 0 deletions app/models/physical_switch.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
class PhysicalSwitch < Switch
include SupportsFeatureMixin
Copy link
Member

Choose a reason for hiding this comment

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

Same

include EventMixin

belongs_to :ext_management_system, :foreign_key => :ems_id, :inverse_of => :physical_switches,
:class_name => "ManageIQ::Providers::PhysicalInfraManager"

has_one :asset_detail, :as => :resource, :dependent => :destroy, :inverse_of => :resource
has_one :hardware, :dependent => :destroy, :foreign_key => :switch_id, :inverse_of => :physical_switch
has_many :physical_network_ports, :dependent => :destroy, :foreign_key => :switch_id
has_many :event_streams, :inverse_of => :physical_switch, :dependent => :nullify

def my_zone
ems = ext_management_system
Expand All @@ -25,6 +29,10 @@ def refresh_ems
EmsRefresh.queue_refresh(ext_management_system)
end

def event_where_clause(assoc = :ems_events)
["#{events_table_name(assoc)}.physical_switch_id = ?", id]
end

def self.display_name(number = 1)
n_('Physical Switch', 'Physical Switches', number)
end
Expand Down