Closed ranapushpender closed 2 years ago
Thank you! I have another PR in the queue. I'll merge that first, hopefully you won't have to rebase, but I can't promise
@ranapushpender I'm sorry but there are some conflicts indeed. They look very minor but I'm unable to resolve them. Can you please rebase? Thanks
Done, please check.
I've added a comment, please check it out.
Obvious question: have you tested these changes? I don't use Discord.
Will update tommorow, I'm using both telegram and discord notifications right now from this script.
Ok looks good. Will merge later today and update the corresponding documentation, then I'll do some general testing. I want to "release" the version before the end of the year :)
@ranapushpender the newline change creates a problem with Healthchecks.io, where is not parsed successfully. I think this happens because Healthchecks does not "read" the output but just shows what it gets.
The space didn't create any problem with Telegram or Healthchecks. I'm reverting the change, let me know what happens with Discord
EDIT: I created a Discord account and enabled the integration, now I see the error. We need another approach because I want both to be working
Ok. Let me check with healthcheck.io and come up with a alternate implementation for it.
Last night I played with Discord but could not find a way to make it working. My plan was to format the message as code using Markdown formatting (like on Github) but Discord was still reading the space and would break. I'm sure there's a way to send this kind of information to Discord.
@ranapushpender the quick and dirty way to fix this is to send to Discord only the subject without the summary of files.
Example:
[COMPLETED] DIFF + SCRUB Jobs (SnapRAID on test)
We could still send the summary bu creating a new $NOTIFY_OUTPUT
just for Discord where there's a \n
instead of a new line. But I'm not sure if it's really elegant so I will not implement this for now, let me know your thoughts.
Sending the subject doesn't seem like a good solution since discord notifications will only provide partial information to user.
I have been experimenting with different methods to create the output string but if they work on healthcheks.io then they don't work on discord so finding a way to use a common string to POST to all notification services seems to be a bigger challenge than I thought.
I think instead of keeping a single variable containing the output, the subject and description should be combined in the notification handlers for different platforms. This can provide us more flexibility and allow us to format the output depending on the features provided by the platform.
For example I was able to use "embeds" in discord to get much nicer looking outputs by passing the subject and description in different keys in the JSON that I was sending.
I really like this result, this could be enabled for Telegram as well, which also support formatting and rich text. Healtchecks.io doesn't parse what you send, it will just display it.
My solution (already in code) is just a temporary patch to let everything work, although with limited information for Discord.
Based on your words, I would create individual variables like this:
OLD:
# Scrub ran but did not complete successfully so lets warn the user
SUBJECT="[WARNING] SCRUB job ran but did not complete successfully $EMAIL_SUBJECT_PREFIX"
NOTIFY_OUTPUT="$SUBJECT
SUMMARY: Equal [$EQ_COUNT] - Added [$ADD_COUNT] - Deleted [$DEL_COUNT] - Moved [$MOVE_COUNT] - Copied [$COPY_COUNT] - Updated [$UPDATE_COUNT]"
notify_warning
NEW
# Scrub ran but did not complete successfully so lets warn the user
NOTIFY_SUBJECT="[WARNING] SCRUB job ran but did not complete successfully $EMAIL_SUBJECT_PREFIX"
NOTIFY_SUMMARY="SUMMARY: Equal [$EQ_COUNT] - Added [$ADD_COUNT] - Deleted [$DEL_COUNT] - Moved [$MOVE_COUNT] - Copied [$COPY_COUNT] - Updated [$UPDATE_COUNT]"
Then adapt notify_warning
and notify_success
with platform specific formatting and codes.
Do you have a more elegant approach to this? My scripting capabilities are limited and many part of this script were not written by me. If you can, please open a new PR when ready, otherwise I'll try to do it in the future.
I'll work on this, also will try to come with a better approach for this if I can.
This adds discord notification integration for snapraid sync jobs using discord webhooks. It also replaces newline characters with new line escape sequence as it creates error with some api's.