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
Added support for web change alerts via pushover #18
Conversation
@@ -59,6 +59,7 @@ urls_txt = os.path.join(urlwatch_dir, 'urls.txt') | |||
cache_dir = os.path.join(urlwatch_dir, 'cache') | |||
scripts_dir = os.path.join(urlwatch_dir, 'lib') | |||
hooks_py = os.path.join(scripts_dir, 'hooks.py') | |||
pushover_txt = os.path.join(urlwatch_dir, 'pushover.conf') |
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.
This variable should probably be called pushover_conf
.
Added some comments, please have a look. You can run the |
Cheers for feedback Thomas, hopefully all fixed now in latest commit. |
@@ -388,10 +427,25 @@ if __name__ == '__main__': | |||
short_summary += '\n\n\n' | |||
if not options.quiet: | |||
print short_summary | |||
|
|||
if(options.pushover): |
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.
PEP-8:
if options.pushover:
Two more code style issues, apart from that I think this can go in. Can you also add a short usage example to the README file, so users can test this? |
Quick example added |
config = ConfigParser.RawConfigParser() | ||
config.read(pushover_conf) | ||
except ConfigParser: | ||
log.error('Failed to parse pushover config file %s' % options.pushover_file) |
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.
The logger takes care of formatting the string, so replace %
with ,
:
log.error('Failed to parse pushover config file %s', options.pushover_file)
Also, please squash your changes after you've integrated the fixes, then I think it should be good to go in. |
Sorry for delay, hopefully all ready now. |
if options.pushover: | ||
summary_txt = '%(type)s: <a href="%(url)s">%(url)s</a>' % {'type': type.upper(), 'url': str(url)} | ||
else: | ||
summary_txt = ': '.join((type.upper(), str(url))) |
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.
Indentation issue (should be 4 spaces).
Spotted two indentation issues, other than that looks good. Can you fixup those? |
@atiro: Can you look into the remaining issues so we can merge this? |
Apologies, totally missed this. Added now very late! |
You should now be able to write a Reporter subclass and just add any settings you need to the configuration (storage.py has a https://github.com/thp/urlwatch/blob/master/share/urlwatch/examples/hooks.py.example#L92 |
Default settings can be placed here, like with the e-mail reporter: https://github.com/thp/urlwatch/blob/master/lib/urlwatch/storage.py#L67 |
Seems easier to submit for the updated code so I'll close this PR and create a new one based on urlwatch2 |
Copied from the XMPP patch, support for Pushover API to send notifications about website changes. Pushover config file needs to have App Key & User Key (created with a pushover account)