-
Notifications
You must be signed in to change notification settings - Fork 236
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
Update slack notification formatting #34
base: master
Are you sure you want to change the base?
Changes from 8 commits
46ec29e
0f5f717
9223353
6fcf11a
58ac7e3
b3de6c0
30c088a
33e5c25
7e34a8a
e4b6af5
6cbb3c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
|
||
DATE_FORMAT = "%Y-%m-%d %H:%M:%S" | ||
|
||
|
||
def slack_sender(webhook_url: str, channel: str, user_mentions: List[str] = []): | ||
""" | ||
Slack sender wrapper: execute func, send a Slack notification with the end status | ||
|
@@ -21,7 +22,7 @@ def slack_sender(webhook_url: str, channel: str, user_mentions: List[str] = []): | |
`channel`: str | ||
The slack room to log. | ||
`user_mentions`: List[str] (default=[]) | ||
Optional users ids to notify. | ||
Optional usernames to notify. | ||
Visit https://api.slack.com/methods/users.identity for more details. | ||
""" | ||
|
||
|
@@ -30,6 +31,10 @@ def slack_sender(webhook_url: str, channel: str, user_mentions: List[str] = []): | |
"channel": channel, | ||
"icon_emoji": ":clapper:", | ||
} | ||
|
||
if user_mentions: | ||
user_mentions = ["@{}".format(user) for user in user_mentions if not user.startswith("@")] | ||
|
||
def decorator_sender(func): | ||
@functools.wraps(func) | ||
def wrapper_sender(*args, **kwargs): | ||
|
@@ -50,60 +55,174 @@ def wrapper_sender(*args, **kwargs): | |
master_process = True | ||
|
||
if master_process: | ||
contents = ['Your training has started 🎬', | ||
'Machine name: %s' % host_name, | ||
'Main call: %s' % func_name, | ||
'Starting date: %s' % start_time.strftime(DATE_FORMAT)] | ||
contents.append(' '.join(user_mentions)) | ||
dump['text'] = '\n'.join(contents) | ||
dump['icon_emoji'] = ':clapper:' | ||
notification = "Your training has started! 🎬" | ||
if user_mentions: | ||
notification = _add_mentions(notification) | ||
|
||
dump["blocks"] = _starting_message( | ||
func_name, host_name, notification, start_time | ||
) | ||
dump["text"] = notification | ||
dump["icon_emoji"] = ":clapper:" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might be missing something on the block builder web demo: I couldn't display this emoji (error with this line it seems). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I found some bad news about this on the slack webhook docs
I tried some workarounds for this, but it seems that updating the name/emoji are only possible using the new api, which would change the configuration of this sender. As a result, I removed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have difficulties understanding: where is your message sent then (on which channel)? is it encoded in the URL in an identifiable way? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this (https://api.slack.com/bot-users#installing-bot) related? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since Messages sent to this webhook are still posted as the bot, but only to one channel, and with the same username and profile image every time. This functionality looks like it has been deprecated for webhooks, and is only possible with the new API. I can update the slack sender to use the new API if you wish, but it will bring it further away from the refactoring you mentioned earlier There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @VictorSanh How would you like to proceed? I believe the choices are:
|
||
|
||
requests.post(webhook_url, json.dumps(dump)) | ||
|
||
try: | ||
value = func(*args, **kwargs) | ||
|
||
if master_process: | ||
end_time = datetime.datetime.now() | ||
elapsed_time = end_time - start_time | ||
contents = ["Your training is complete 🎉", | ||
'Machine name: %s' % host_name, | ||
'Main call: %s' % func_name, | ||
'Starting date: %s' % start_time.strftime(DATE_FORMAT), | ||
'End date: %s' % end_time.strftime(DATE_FORMAT), | ||
'Training duration: %s' % str(elapsed_time)] | ||
|
||
try: | ||
str_value = str(value) | ||
contents.append('Main call returned value: %s'% str_value) | ||
except: | ||
contents.append('Main call returned value: %s'% "ERROR - Couldn't str the returned value.") | ||
|
||
contents.append(' '.join(user_mentions)) | ||
dump['text'] = '\n'.join(contents) | ||
dump['icon_emoji'] = ':tada:' | ||
notification = "Your training is complete 🎉" | ||
if user_mentions: | ||
notification = _add_mentions(notification) | ||
|
||
dump["blocks"] = _successful_message( | ||
func_name, host_name, notification, start_time, value | ||
) | ||
dump["text"] = notification | ||
dump["icon_emoji"] = ":tada:" | ||
requests.post(webhook_url, json.dumps(dump)) | ||
|
||
return value | ||
|
||
except Exception as ex: | ||
end_time = datetime.datetime.now() | ||
elapsed_time = end_time - start_time | ||
contents = ["Your training has crashed ☠️", | ||
'Machine name: %s' % host_name, | ||
'Main call: %s' % func_name, | ||
'Starting date: %s' % start_time.strftime(DATE_FORMAT), | ||
'Crash date: %s' % end_time.strftime(DATE_FORMAT), | ||
'Crashed training duration: %s\n\n' % str(elapsed_time), | ||
"Here's the error:", | ||
'%s\n\n' % ex, | ||
"Traceback:", | ||
'%s' % traceback.format_exc()] | ||
contents.append(' '.join(user_mentions)) | ||
dump['text'] = '\n'.join(contents) | ||
dump['icon_emoji'] = ':skull_and_crossbones:' | ||
notification = "Your training has crashed ☠️" | ||
if user_mentions: | ||
notification = _add_mentions(notification) | ||
|
||
dump["blocks"] = _error_message( | ||
ex, func_name, host_name, notification, start_time | ||
) | ||
|
||
dump["text"] = notification | ||
dump["icon_emoji"] = ":skull_and_crossbones:" | ||
requests.post(webhook_url, json.dumps(dump)) | ||
raise ex | ||
|
||
def _error_message(ex, func_name, host_name, notification, start_time): | ||
"""Uses Slack blocks to create a formatted report of exception 'ex'.""" | ||
end_time = datetime.datetime.now() | ||
training_time = _format_train_time(end_time, start_time) | ||
return [ | ||
{"type": "section", "text": {"type": "mrkdwn", "text": notification}}, | ||
{"type": "divider"}, | ||
{ | ||
"type": "context", | ||
"elements": [ | ||
{ | ||
"type": "mrkdwn", | ||
"text": "*Machine name:* {}\n" | ||
"*Main call:* {}\n" | ||
"*Starting date:* {}\n" | ||
"*Crash date:* {}\n" | ||
"*Time elapsed before crash:* {}".format( | ||
host_name, | ||
func_name, | ||
start_time.strftime(DATE_FORMAT), | ||
end_time.strftime(DATE_FORMAT), | ||
training_time, | ||
), | ||
} | ||
], | ||
}, | ||
{"type": "divider"}, | ||
{ | ||
"type": "section", | ||
"text": {"type": "mrkdwn", "text": "*Error:* `{}`".format(ex)}, | ||
}, | ||
{ | ||
"type": "section", | ||
"text": { | ||
"type": "mrkdwn", | ||
"text": "*Traceback:*\n```{}```".format(traceback.format_exc()), | ||
}, | ||
}, | ||
] | ||
|
||
def _starting_message(func_name, host_name, notification, start_time): | ||
"""Uses Slack blocks to create an initial report of training.""" | ||
return [ | ||
{"type": "section", "text": {"type": "mrkdwn", "text": notification}}, | ||
{"type": "divider"}, | ||
{ | ||
"type": "context", | ||
"elements": [ | ||
{ | ||
"type": "mrkdwn", | ||
"text": "*Machine name:* {}\n" | ||
"*Main call:* {}\n" | ||
"*Starting date:* {}\n".format( | ||
host_name, func_name, start_time.strftime(DATE_FORMAT) | ||
), | ||
} | ||
], | ||
}, | ||
] | ||
|
||
def _successful_message(func_name, host_name, notification, start_time, value): | ||
"""Uses Slack blocks to report a successful training run with statistics.""" | ||
end_time = datetime.datetime.now() | ||
training_time = _format_train_time(end_time, start_time) | ||
blocks = [ | ||
{"type": "section", "text": {"type": "mrkdwn", "text": notification}}, | ||
{"type": "divider"}, | ||
{ | ||
"type": "context", | ||
"elements": [ | ||
{ | ||
"type": "mrkdwn", | ||
"text": "*Machine name:* {}\n" | ||
"*Main call:* {}\n" | ||
"*Starting date:* {}\n" | ||
"*End date:* {}\n" | ||
"*Training Duration:* {}".format( | ||
host_name, | ||
func_name, | ||
start_time.strftime(DATE_FORMAT), | ||
end_time.strftime(DATE_FORMAT), | ||
training_time, | ||
), | ||
} | ||
], | ||
}, | ||
] | ||
|
||
if value is not None: | ||
blocks.append({"type": "divider"}) | ||
try: | ||
str_value = str(value) | ||
dump["blocks"].append( | ||
{ | ||
"type": "section", | ||
"text": { | ||
"type": "mrkdwn", | ||
"text": "*Main call returned value:* {}".format( | ||
str_value | ||
), | ||
}, | ||
} | ||
) | ||
except Exception as e: | ||
blocks.append( | ||
"Couldn't str the returned value due to the following error: \n`{}`".format( | ||
e | ||
) | ||
) | ||
|
||
return blocks | ||
|
||
def _format_train_time(end_time, start_time): | ||
"""Returns a time delta in format HH:MM:SS""" | ||
elapsed_time = end_time - start_time | ||
hours, remainder = divmod(elapsed_time.seconds, 3600) | ||
minutes, seconds = divmod(remainder, 60) | ||
training_time = "{:2d}:{:02d}:{:02d}".format(hours, minutes, seconds) | ||
MetcalfeTom marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return training_time | ||
|
||
def _add_mentions(notification): | ||
notification = " ".join(user_mentions) + " " + notification | ||
MetcalfeTom marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return notification | ||
|
||
return wrapper_sender | ||
|
||
return decorator_sender | ||
|
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.
Do you need the notification here too? It seems to me that it's already contained in the
block
field.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.
Yes, removing it from here means that the message text will not display in desktop notifications