Physical Server details #14275

walteraa · 2017-03-10T18:37:25Z

This PR contains a migration to add the follow informations in physical server table:

Support contact: String
Description: String
Location: String
Room ID: String (I am not sure if this information can be a String or a Integer)
Rack name: String
Lowest rack unit: String (I am not sure about this data)

walteraa · 2017-03-10T18:39:32Z

@blomquisg @juliancheal @rodneyhbrown7 Can you guys comment about the properties data types which has comments?

juliancheal · 2017-03-10T19:14:39Z

Which editors do you use? Some like Atom & Sublime (I think) can run Rubocop inline.

Also might be worth if your editor can't run Rubocop using https://github.com/m4i/rubocop-git to just run Rubocop on the diffs.

Might save having to do a second commit to remove things like the extra space above. 🍰

juliancheal · 2017-03-10T19:38:40Z

Support contact: String
Description: String
Location: String
Room ID: String (I am not sure if this information can be a String or a Integer)
Rack name: String
Lowest rack unit: String (I am not sure about this data)

Actually thinking about it, I'm not sure if these details should be in PhysicalServer as looking at their data types some of them should probably be :text fields as they're probably longer than 255 chars long, and I think having lots of text fields in a table might not be the most efficient, seeing as the main purpose of PhysicalServer is to be a server. Where it is and who to call when it brakes is secondary almost.

Also are description and location the location and description of the server or the rack?
rack_name should probably be rack_id to a rack object eventually.

I'm looking to see if we have existing tables these details can be stored in, or if we should extract it to a separate table. As I imagine if we in the future create tables like PhysicalStorage, etc they would want to share such attributes.

Is this data already in XClarity? How much of this do we need to store in ManageIQ? Things like if a server requires maintenance, a warning might come from ManageIQ, but a technician would could get that data via an API call to XClairty, or the XClairty app?

rodneyhbrown7 · 2017-03-13T00:10:57Z

I had a thought about creating a separate tables for contact information and maybe a second one for location details. The description is meant to be more of a free form string the user can use to provide additional information about the server, location is typically the location of the rack and may be an address, the rack-name identifies the rack at that location.

These details are availabe in Xclarity today. One of the automations/services that we might include is to identify failing hardware, relocate vms and then notify an admin that hardware requires attention. I will also note that some of the location details may be related to Region/Zone information.

walteraa · 2017-03-13T16:57:16Z

@rodneyhbrown7 @juliancheal the VM has a column named location, so I think the location should be in physical_servers table or we can add a table containing only three columns(id, location and room_id) instead.

About the support_contact the XClarity REST API has a string property named "contact", I think don't sonds good create a table to save only two columns(ID and support_contact).

About the rack informations, are you sure that it should be an other table? So, there are only three columns in this table(id, rack_name and lowest_rack_unit), right?

For now I'll remove this columns from migration and I will let only description and contact in migration. Once you guys answer, I do the other changes.

skovic · 2017-03-13T17:38:40Z

We discussed this offline, and we decided that instead of creating two tables, we'll create a single new table, named assets, that contains: description, location, room_id, rack_name, and lowest_rack_unit.

skovic · 2017-03-13T19:23:27Z

@walteraa Just a suggestion, if you have time, consider changing assets_details to asset_details

walteraa · 2017-03-13T19:24:33Z

For sure, @skovic

rodneyhbrown7 · 2017-03-13T19:33:31Z

@walteraa can you clean up the rubocop messages?

miq-bot · 2017-03-14T17:36:35Z

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

blomquisg · 2017-03-14T19:29:14Z

db/migrate/20170313203319_add_assets_details_ref_to_physical_server.rb

@@ -0,0 +1,5 @@
+class AddAssetsDetailsRefToPhysicalServer < ActiveRecord::Migration[5.0]
+  def change
+    add_reference :physical_servers, :asset_details, index: true, foreign_key: true


Two things here:

Remove the foreign_key constraint

add type: :bigint

Also, @Fryguy will probably be happier if you use Hash Rockets instead. I.e.,

add_reference :physical_servers, :asset_details, :index => true, :type => :bigint

blomquisg · 2017-03-14T19:31:59Z

db/migrate/20170313203052_create_asset_details.rb

+      t.text  :location
+      t.text  :room
+      t.text  :rack_name
+      t.text  :lowest_rack_unit


not sure this is needed, but you might want to add t.timestamps to this table to track created and last updated dates.

walteraa · 2017-03-17T13:20:20Z

I should to change the ref between asset details and Physical server.

Add a migration to create asset_details table.

Add properties to schema.yml referring to the new table(asset_details).

walteraa · 2017-04-25T20:59:10Z

@blomquisg Can you review this PR and #14827, #14749 as well?

Fryguy · 2017-05-02T16:27:10Z

db/migrate/20170425185231_create_asset_details.rb

+      t.bigint  :resource_id
+      t.string  :resource_type
+      t.timestamps
+      t.index %w(resource_id resource_type), :name => "index_asset_details_on_resource_id_and_resource_type", :using => :btree


You should not need the name nor using, as Rails will default those properly.

Fryguy

I had one comment on the index, but otherwise LGTM

juliancheal · 2017-05-02T16:30:10Z

LGTM 👍

Remove 'name' and 'using' from asset details index.

miq-bot · 2017-05-03T13:03:51Z

Checked commits https://github.com/lenovo/manageiq/compare/7069f6134f686c68d626faf2d10ded50c1a0cb7a~...33eecf1231055ea139a6148703969d04f2c7131a with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

walteraa · 2017-05-04T17:22:58Z

🎂🎂

miq-bot added wip sql migration labels Mar 10, 2017

miq-bot added unmergeable and removed unmergeable labels Mar 14, 2017

blomquisg reviewed Mar 14, 2017

View reviewed changes

walteraa changed the title ~~[WIP] Physical Server details~~ Physical Server details Mar 14, 2017

miq-bot removed the wip label Mar 14, 2017

chessbyte assigned Fryguy Mar 15, 2017

chessbyte added the providers/physical label Mar 15, 2017

walteraa changed the title ~~Physical Server details~~ [WIP]Physical Server details Mar 17, 2017

miq-bot added the wip label Mar 17, 2017

walteraa changed the title ~~[WIP]Physical Server details~~ Physical Server details Mar 21, 2017

miq-bot removed the wip label Mar 21, 2017

MaysaMacedo mentioned this pull request Apr 12, 2017

Create asset details object #14749

Merged

walteraa mentioned this pull request Apr 20, 2017

Method to save asset details #14827

Merged

walteraa added 3 commits April 25, 2017 16:20

Create a migration to asset details table

7069f61

Add a migration to create asset_details table.

Update db/schema.yml

648a357

Add properties to schema.yml referring to the new table(asset_details).

Fix Rubocop warnings

0a47b32

Fryguy reviewed May 2, 2017

View reviewed changes

Fryguy approved these changes May 2, 2017

View reviewed changes

Remove 'name' and 'using' from index

33eecf1

Remove 'name' and 'using' from asset details index.

blomquisg added the fine/no label May 4, 2017

blomquisg merged commit f2cb9cd into ManageIQ:master May 4, 2017

blomquisg added this to the Sprint 60 Ending May 8, 2017 milestone May 4, 2017

Fryguy added the enhancement label May 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Physical Server details #14275

Physical Server details #14275

walteraa commented Mar 10, 2017 •

edited by chessbyte

Loading

walteraa commented Mar 10, 2017

juliancheal commented Mar 10, 2017

juliancheal commented Mar 10, 2017

rodneyhbrown7 commented Mar 13, 2017

walteraa commented Mar 13, 2017 •

edited

Loading

skovic commented Mar 13, 2017

skovic commented Mar 13, 2017

walteraa commented Mar 13, 2017

rodneyhbrown7 commented Mar 13, 2017

miq-bot commented Mar 14, 2017

blomquisg Mar 14, 2017

blomquisg Mar 14, 2017

walteraa commented Mar 17, 2017

walteraa commented Apr 25, 2017 •

edited

Loading

Fryguy May 2, 2017

walteraa May 3, 2017

Fryguy left a comment

juliancheal commented May 2, 2017

miq-bot commented May 3, 2017

walteraa commented May 4, 2017

Physical Server details #14275

Physical Server details #14275

Conversation

walteraa commented Mar 10, 2017 • edited by chessbyte Loading

walteraa commented Mar 10, 2017

juliancheal commented Mar 10, 2017

juliancheal commented Mar 10, 2017

rodneyhbrown7 commented Mar 13, 2017

walteraa commented Mar 13, 2017 • edited Loading

skovic commented Mar 13, 2017

skovic commented Mar 13, 2017

walteraa commented Mar 13, 2017

rodneyhbrown7 commented Mar 13, 2017

miq-bot commented Mar 14, 2017

blomquisg Mar 14, 2017

Choose a reason for hiding this comment

blomquisg Mar 14, 2017

Choose a reason for hiding this comment

walteraa commented Mar 17, 2017

walteraa commented Apr 25, 2017 • edited Loading

Fryguy May 2, 2017

Choose a reason for hiding this comment

walteraa May 3, 2017

Choose a reason for hiding this comment

Fryguy left a comment

Choose a reason for hiding this comment

juliancheal commented May 2, 2017

miq-bot commented May 3, 2017

walteraa commented May 4, 2017

walteraa commented Mar 10, 2017 •

edited by chessbyte

Loading

walteraa commented Mar 13, 2017 •

edited

Loading

walteraa commented Apr 25, 2017 •

edited

Loading