-
Notifications
You must be signed in to change notification settings - Fork 900
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 description to miq_alert_statuses #13653
Conversation
cc @simon3z @jeff-phillips-18 This is called "message" in the mockups |
@miq-bot add_label enhancement, wip |
b0688ec
to
2fd3d26
Compare
2fd3d26
to
a07ea2d
Compare
|
||
result | ||
end | ||
|
||
def add_status_post_evaluate(target, result) | ||
def add_status_post_evaluate(target, result, statue_description) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@moolitayer typo: statue_description
a07ea2d
to
18453d7
Compare
Checked commit moolitayer@18453d7 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 db/migrate/20170125141953_update_description_in_miq_alert_status.rb
|
miq_alert_statuses = Arel::Table.new('miq_alert_statuses') | ||
join_sql = miq_alerts.project(miq_alerts[:description]) | ||
.where(miq_alerts[:id].eq(miq_alert_statuses[:miq_alert_id])).to_sql | ||
MiqAlertStatus.update_all("description = (#{join_sql})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this one. We need someone to properly review it. Maybe @kbrock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, I would have done sql instead of areal but it looks good.
update miq_alert_statuses
set description = (
select miq_alerts.description
from miq_alerts
where miq_alerts.id = miq_alert_statuses.miq_alert_id
)
LGTM 👍
LGTM 👍 @gtanzillo can you check if this is OK for Alerts in general? @miq-bot assign gtanzillo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 @Fryguy, please review for SQL migrations.
Why cookie description from miq_alert to miq_alert_statuses?
We want to support generic alerting from external systems (hawkular).
What we get from it is only instances of an alert (==miq_alert_status) containing a description and there is no entity parallel to miq_alert.
The purpose of this change is to take the description from the MAS when it is available (container alerts from hawkular) or from the MA otherwise (all other providers including middleware alerts)
The piece adding the external description on mas will come in the event collection patch #12773