ttionya / vaultwarden-backup

Backup vaultwarden (formerly known as bitwarden_rs) SQLite3/PostgreSQL/MySQL/MariaDB database by rclone. (Docker)
MIT License
956 stars 113 forks source link

Add ntfy.sh push notification #113

Closed bastouf closed 1 year ago

ttionya commented 1 year ago

@bastouf Thank you for your pull request, and I sincerely apologize for not being able to review and address it promptly due to my busy schedule.

Regarding the addition of a new notification channel, I believe it is necessary to discuss its necessity.

Email notification is a fundamental means of notification that sends emails upon success and failure. Users have the autonomy to decide when to send emails. Opting to receive only failure emails is a good choice as it prevents receiving a large number of insignificant success emails. You only need to investigate the reason for failure when receiving a failure email. However, receiving success emails as well can provide additional peace of mind, depending on your preferences.

While email notification is useful, it falls short if the container stops due to certain reasons. In such cases, incorporating the notification method through healthcheck.io (using the PING_URL environment variable) can effectively address this issue. After each successful backup, a request is made to the healthcheck.io address. If a health check request fails to be sent, healthcheck.io will notify you through the means you have set.

Regarding ntfy.sh, it only notifies you upon successful backups, leaving you to determine whether the backup was successful or not. Personally, I find this less meaningful. If indeed an additional notification method is desired, I lean towards introducing a more versatile webhooks notification, where requests are sent to a designated address regardless of success or failure. This would also ensure compatibility with ntfy.sh, wouldn't it?

I look forward to your response. Let's continue the discussion for now, focusing on ideas rather than rushing into code. Once again, I apologize for the delayed response to the pull request.

ttionya commented 1 year ago

@bastouf It appears that you maintain your own fork, and this pull request is no longer relevant. I will close it.

If you believe it is necessary to discuss the webhook issue, please open a new issue.