-
Notifications
You must be signed in to change notification settings - Fork 261
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
Notification emails merging and localization #798
Notification emails merging and localization #798
Conversation
909da9c
to
82f0ea3
Compare
bad75a1
to
d996d30
Compare
/ost |
d996d30
to
76a07d3
Compare
/ost |
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.
just a casual review, but it has tests and if automation passes it LGTM
/ost |
2 similar comments
/ost |
/ost |
76a07d3
to
fca4703
Compare
/ost |
lgtm except the properties file that needs to be done in zanata |
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.
approval just for the sake of OST
/ost |
eh, builds need to finish first:/ |
...and for that it needs ansible fixes(or at elst disabling the linting) first |
Signed-off-by: Stepan Ermakov <sermakov@orionsoft.ru>
fca4703
to
6db8b10
Compare
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.
approve for OST
/ost |
2 similar comments
/ost |
/ost |
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.
+1
Hi, I would like to introduce couple email notification enhancements.
Why do we need these enhancements?
ovirt-engine has a separate service ovirt-engine-notifier. This service monitors events at the audit_log table and sends the notifications to different recipients (amqp, snmp, and smtp).
We are mostly interested in email notifications about engine backup events (ENGINE_BACKUP_STARTED, ENGINE_BACKUP_FAILED, ENGINE_BACKUP_COMPLETED).
So, first, when the engine-backup.sh script notifies ovirt-engine it sends multiple events in a row for each backup scope (files, db, dwhdb, cinderlib, grafanadb). Finally, If you start one backup then email recipients would receive 10 email messages (5 messages (one per scope item) about the backup started and 5 (one per scope item) messages about the backup completed or failed). We want to combine these messages in a way where the recipient receives only two emails: one for the "backup started" (with all scope items inside) and one for the "backup completed/failed" (with all scope items inside).
And second, currently, the template of the email messages is hard coded. And it is always in English. We need to provide the ability to localize the email contents to other languages.
What did we propose in this change?
For both topics explained above (email merging and email localization) we introduced two additional parameters (see changes at ovirt-engine-notifier.in). By default, both these parameters are empty, so the system would work as it is - nothing would be changed in the current behavior.
But if you specify a list of "log type names" for the MAIL_MERGE_LOG_TYPES parameter then the new "email merging" logic would be activated for the events of specified types.
If you specify some locale code (currently "en" or "ru" are supported, but welcome to help us to provide translations for other languages) for the MAIL_LOCALE parameter then the static fields of the notification email subject (such as "Time: ", "Message: ", etc) would be translated to the specified language.