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

Adding Location LED name to asset_details table #231

Merged
merged 1 commit into from
Jul 24, 2018

Conversation

douglasgabriel
Copy link
Member

It is required to send LED action to physical components, once that the name of the LED can vary to each component.

@douglasgabriel
Copy link
Member Author

@miq-bot assign @agrare

@agrare
Copy link
Member

agrare commented Jul 9, 2018

@douglasgabriel I'd recommend calling this something other than _name, it sounds like this is more of a unique identifier sent to the API to target a specific LED which we would call an _ems_ref.

xClarity may use a name for this but other physical providers might use something else.

@douglasgabriel
Copy link
Member Author

Good point @agrare. Updated.

@agrare
Copy link
Member

agrare commented Jul 9, 2018

@miq-bot assign @Fryguy

@miq-bot miq-bot assigned Fryguy and unassigned agrare Jul 9, 2018
@miq-bot
Copy link
Member

miq-bot commented Jul 9, 2018

Checked commit douglasgabriel@5f703b0 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@Fryguy
Copy link
Member

Fryguy commented Jul 16, 2018

In general I'm ok with this, but I'm curious why this is not a foreign key reference to the table with the location_led itself instead?

@douglasgabriel
Copy link
Member Author

Do we have a table for location_led? What we do today is parse just the location led state into the component table (ex: physical_servers). The idea to parse the location_led_ems_ref inside the asset_details table is to share this column with all physical components avoiding to put this column on each component table.

@Fryguy
Copy link
Member

Fryguy commented Jul 24, 2018

Discussed with @douglasgabriel and the long term goal is to put location_led_status into asset_details, so that should keep them together and consistent, so I am good with this. 👍

@Fryguy Fryguy merged commit fa066ba into ManageIQ:master Jul 24, 2018
@Fryguy Fryguy added this to the Sprint 91 Ending Jul 30, 2018 milestone Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants