Alerting doc changes and sample files #15
Conversation
|
Coverage remained the same at 0.0% when pulling da76b1c on anmolbabu:Tendrl/specifications#40 into 9908e66 on Tendrl:master. |
|
TODO(@anmolbabu)
@Tendrl/tendrl-core Please review this WIP patch |
|
Coverage remained the same at 0.0% when pulling ac2bd14 on anmolbabu:Tendrl/specifications#40 into 9908e66 on Tendrl:master. |
|
Coverage remained the same at 0.0% when pulling 56238ff on anmolbabu:Tendrl/specifications#40 into 9908e66 on Tendrl:master. |
| except (ConfigNotFound, NotificationDispatchError) as ex: | ||
| raise NotificationDispatchError(str(ex)) | ||
|
|
||
| def get_mail_client(self): | ||
| if not self.admin_config: | ||
| raise NotificationDispatchError( | ||
| "Admin mail configuration is required for dispatching email" | ||
| "notification" | ||
| " notification" |
There was a problem hiding this comment.
Yes because, its a string concatenation of 2 lines "Admin mail configuration is required for dispatching email" and "notification" so if its not " notification", the resultant sentence appears as:
"Admin mail configuration is required for dispatching emailnotification"
|
|
||
| def run(self): | ||
| try: | ||
| app.run(host=self.host, port=5001) |
There was a problem hiding this comment.
Is the port configurable? If so externalize this as well in configuration. Default could be 5001 if not defined in configurations file.
There was a problem hiding this comment.
Currently it is not configurable. Making configurable now...
|
|
||
|
|
||
| @to_singleton | ||
| class EtcdInterface(object): |
There was a problem hiding this comment.
Why not call EtcdManager as you called ApiManager above
| alerts_arr.append(json.loads(child['value'])) | ||
| if filters is not None: | ||
| filtered_alerts = {} | ||
| for f in filters: |
There was a problem hiding this comment.
filter seems to be a keyword in python
| raise NotificationDispatchError(str(ex)) | ||
|
|
||
| def format_message(self, alert): | ||
| return "Subject: [Alert] %s, %s threshold breached\n\n%s" % ( |
There was a problem hiding this comment.
Should we provide option for formatting message as configurable one?
There was a problem hiding this comment.
No I don't think this needs to be configurable at-least that's not planned as of now
| ) | ||
| if ( | ||
| self.admin_config.get('auth') is not None and | ||
| self.admin_config['auth'] == 'ssl' |
anmolbabu
force-pushed
the
Tendrl/specifications#40
branch
from
5695567
to
e724c23
Compare
anmolbabu
changed the title
|
Coverage remained the same at 0.0% when pulling e724c23 on anmolbabu:Tendrl/specifications#40 into 9908e66 on Tendrl:master. |
| @@ -13,11 +13,7 @@ log_level = DEBUG | |||
|
|
|||
| [alerting] | |||
| # Path to log file | |||
| log_cfg_path = /etc/tendrl/logging.yaml | |||
| log_cfg_path = /etc/tendrl/alerting_logging.yaml | |||
There was a problem hiding this comment.
As I commented on the other patch, alert configuartion can be in a different file
Add rest apis + alert watcher + notification management framework
TODO:
Note: Python multiprocessing has been used here instead of greenlet
only to handle blocking calls.
tendrl-bug-id: #10,#11,#12,#14
Signed-off-by: anmolbabu anmolbudugutta@gmail.com