HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Script to send push notifications using email, Pushbullet, or Pushover

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptemailnotificationspushpushbulletusingpushoversend

Problem

I have built a tiny script to send out push notifications. I would be glad to receive general suggestions / feedback.

My main problem is that the only security offered is coming from the OS filesystem settings (making the config file not readable to other users) and I would be very happy to improve my code and make it more secure.

In case anybody is much more comfortable with git than with copy/paste:

git clone https://github.com/ltpitt/python-simple-notifications.git


For the others here's the main code:

```
#
# ______________________
# / Simple Notifications \
# \ /
# ----------------------
# \ ^__^
# \(oo)\_______
# (__)\ )\/\
# ||-----w |
# || ||
#
# This script sends notification using
# Email, Pushbullet or Pushover
#
# Please put your data into configure.py before using this script

import notification_config
import requests
import json
import sys
import httplib, urllib
from email.mime.text import MIMEText
import smtplib

server = smtplib.SMTP()

def send_email(email_subject, notification_msg, email_recipients):
'''
This functions sends a notification using Email
Args:
email_subject (str) : Email Subject.
notification_msg (str) : Email Body.
email_recipients (str) : Email recipients.
'''
server.connect(notification_config.EMAIL_SERVER, notification_config.EMAIL_SERVER_PORT)
if notification_config.EMAIL_DEBUG_LEVEL == '1':
server.set_debuglevel(1)
recipients = [email_recipients]
msg = MIMEText(notification_msg)
msg['Subject'] = email_subject
msg['From'] = notification_config.EMAIL_SENDER
msg['To'] = ', '.join(recipients)
server.ehlo()
server.starttls()
server.ehlo
server.login(notification_config.EMAIL_SENDER, notification_config.EMAIL_PASSWORD)
server.sendmail(notification_config.EMAIL_SENDER, recipients, msg.as_string())
server.quit()

def send_pushover_notificatio

Solution

Since you’re willing to install third-party packages (I see a pip install -r requirements.txt step in your README), there are a couple of packages you can install that will probably be quite useful for your code.

For secure config: consider keyring

keyring provides an interface to the system keychain, so configuration is stored more securely than by mere filesystem access, and can only be accessed if the system keychain is unlocked.

So, for example, rather than hard-coding an access token, you could look it up like so:

import keyring

keyring.get_password('pushover', 'user_key')


This has the downside that it’s a bit more fiddly to set up – the user either has to use the OS’s keychain manager (if one exists), or run keyring.set_password(). You might consider providing a helper script that sets up the keychain, using getpass to ask for the password without dumping it to the console.

For argument parsing: consider docopt

You’re rolling your own argument parser, which is lots of extra code you have to write, debug and test. Within the standard library, there’s argparse, which makes this a little easier, but docopt makes it easier still. You just write the help message in a standard format, and let docopt do the fiddly business of option parsing.

I often use it with the of the module, like so:

#!/usr/bin/env python
# -*- encoding: utf-8 -*-
"""
Simple Notifications.

Usage:
  notifications.py email   
  notifications.py pushbullet  
  notifications.py pushover 
"""

import docopt

arguments = docopt.docopt(__doc__)
print(arguments)


And now anybody reading the code can easily see what the interface to the script is, and all that messy argument parsing code has been cleaned up. Additionally, docopt will do validation – for example, warning the user if they enter too many arguments, something your code doesn’t do.

For HTTPS: consider requests

I’m a bit confused as to why you use requests to make the POST request in send_pushbullet_notification(), but fall back to httplib and setting up an HTTPSConnection instance for send_pushover_notification().

Wouldn’t it be easier to just use requests.post() in both instances?

While I’m looking at these two functions, two additional comments:

-
Don’t print anything on success, just return True or exit quietly. If I want to use these functions as part of a longer pipeline, it’s really inconvenient to have to suppress this printing when your function completes. Better not to do anything like that within the function, and let the caller decide how they want to notify the end user.

-
Don’t throw away the HTTP status code in your exception messages; it can be very useful for debugging why something failed. There may be more useful information in the response; that’s the minimum you should be sending.

Code Snippets

import keyring

keyring.get_password('pushover', 'user_key')
#!/usr/bin/env python
# -*- encoding: utf-8 -*-
"""
Simple Notifications.

Usage:
  notifications.py email <subject> <message> <recipients>
  notifications.py pushbullet <title> <message>
  notifications.py pushover <message>
"""

import docopt

arguments = docopt.docopt(__doc__)
print(arguments)

Context

StackExchange Code Review Q#141887, answer score: 5

Revisions (0)

No revisions yet.