-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Add Apprise notifier #292
Add Apprise notifier #292
Conversation
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.
Hi @leonghui.
Thank you for your contribution. I didn't know Apprise till now. Seems to be a very useful project.
Please note my comments.
src/notifiers/apprise.py
Outdated
apobj = apprise.Apprise() | ||
apobj.add(self.url) | ||
apobj.notify( | ||
body=body |
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.
I didn't use apprise until now. But what about title
and tags
? These options could also be used with variables.
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.
Thanks, I have added title
and it works with Signal. From the docs, there seems to be two types of tags in Apprise - content tags which are only supported by a few platforms (ntfy, Boxcar, PagerTree, etc.) and URL tags which would be useful for filtering services to send to. The latter could be useful but it requires multiple Apprise URLs to be defined concurrently, along with other enhancements, e.g. notify X for 'meals' type of magic bags and notify Z for stores within a defined distance, etc.
src/notifiers/apprise.py
Outdated
log.debug("Apprise url: %s", url) | ||
if self.body: | ||
body = item.unmask(self.body) | ||
apobj = apprise.Apprise() |
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.
What about creating the Apprise
object during the initialization of the notifier and clearing it on shutdown? You can implement def stop(self)
which is called for every notifier when the scanner is shutting down.
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.
Done! Note that Apprise URLs are always appended to the Apprise object until the object is cleared.
src/notifiers/apprise.py
Outdated
self.enabled = config.apprise.get("enabled", False) | ||
self.body = config.apprise.get("body") | ||
self.url = config.apprise.get("url") | ||
self.timeout = config.apprise.get("timeout", 60) |
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.
You didn't implement apprise.timeout
in the config.py
.
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.
I have removed apprise.timeout
because Apprise has two timeout values - connect and read - and they can be configured via url
as global parameters.
Implements #291. Tested on Docker with https://github.com/bbernhard/signal-cli-rest-api.