11
\$\begingroup\$

I've written a basic website status checker in Python/Flask which reads a list of URLs from a json file and cycles through them every x seconds to check they're online. It displays the results as a webpage:

Screenshot

It was written to help me learn Python (at the end of my third week) on my phone in my spare time rather than out of any real necessity so I'd love any feedback on improvements that could be made... both stylistically and programmatically :)

To keep things brief I won't include my very basic css but that's on the github repo: https://github.com/emojipeach/webpagestatuscheck

Files/Folders:

Project
 |
 +-- app.py
 +-- checkurls.json   
 +-- settings.py
 +-- unittests.py
 |    
 +-- templates
 |  |  
 |  +-- layout.html
 |  +-- returned_statuses.html 

app.py

import requests
import json
import threading
from socket import gaierror, gethostbyname
from multiprocessing.dummy import Pool as ThreadPool
from urllib.parse import urlparse
from flask import Flask, render_template, jsonify
from time import gmtime, strftime

from settings import refresh_interval, filename, site_down


def is_reachable(url):
    """ This function checks to see if a host name has a DNS entry
    by checking for socket info."""
    try:
        gethostbyname(url)
    except (gaierror):
        return False
    else:
        return True

def get_status_code(url):
    """ This function returns the status code of the url."""
    try:
        status_code = requests.get(url, timeout=30).status_code
        return status_code
    except requests.ConnectionError:
        return site_down

def check_single_url(url):
    """This function checks a single url and if connectable returns
    the status code, else returns UNREACHABLE."""
    if is_reachable(urlparse(url).hostname) == True:
        return str(get_status_code(url))
    else:
        return site_down

def check_multiple_urls():
    """ This function checks through urls specified in the checkurls.json file
    and returns their statuses as a dictionary every 60s."""
    statuses = {}
    temp_list_urls = []
    temp_list_statuses = []
    global returned_statuses
    global last_update_time
    t = threading.Timer
    t(refresh_interval, check_multiple_urls).start()
    for group, urls in checkurls.items():
        for url in urls:
            temp_list_urls.append(url)
    pool = ThreadPool(8)
    temp_list_statuses = pool.map(check_single_url, temp_list_urls)
    for i in range(len(temp_list_urls)):
        statuses[temp_list_urls[i]] = temp_list_statuses[i]
    last_update_time = strftime("%Y-%m-%d %H:%M:%S", gmtime())
    returned_statuses = statuses

app = Flask(__name__)

@app.route("/", methods=["GET"])
def display_returned_statuses():
    return render_template(
        'returned_statuses.html',
        returned_statuses = returned_statuses,
        checkurls = checkurls,
        last_update_time = last_update_time
        )

@app.route("/api", methods=["GET"])
def display_returned_api():
    return jsonify(
        returned_statuses
        ),200

with open(filename) as f:
    checkurls = json.load(f)
returned_statuses = {}
last_update_time = 'time string'

if __name__ == '__main__':
    check_multiple_urls()
    app.run() 

settings.py

# Interval to refresh status codes in seconds
refresh_interval = 60.0

# File containing groups ofurls to check in json format. See included example 'checkurls.json'
filename = 'checkurls.json'

# Message to display if sites are not connectable
site_down = 'UNREACHABLE' 

checkurls.json

{
"BBC": [
        "https://www.bbc.co.uk", 
        "http://www.bbc.co.uk",
        "https://doesnotexist.bbc.co.uk",
        "https://www.bbc.co.uk/sport",
        "https://www.bbc.co.uk/404",
        "https://www.bbc.co.uk"
        ],
"Google": [
        "https://www.google.com",
        "https://support.google.com",
        "http://localhost:8080"
        ]
} 

templates/layout.html

<!doctype html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <title>A Simple Website Status Checker</title>

    <link rel="stylesheet" href="https://unpkg.com/purecss@1.0.0/build/pure-min.css" integrity="sha384-nn4HPE8lTHyVtfCBi5yW9d20FjT8BJwUXyWZT9InLYax14RDjBj46LmSztkmNP9w" crossorigin="anonymous">

    <link rel="stylesheet" type="text/css" href="{{ url_for('static', filename='stylesheet.css') }}">

</head>
<body>

    {% block body %}{% endblock %}

</body>
</html> 

templates/returned_statuses.html

{% extends "layout.html" %}
{% block body %}

<div class="time_updated">Last updated: {{ last_update_time }} UTC</div>

{% for group, urls in checkurls.items() %}

    <h1 class="group">{{ group }}</h1>
    {% for url in urls %}
        {% if returned_statuses.get(url) == "200" %}
            <p class="good-url">{{ url }} <font color="green"> {{ returned_statuses.get(url) }}</font></p>
        {% endif %}
    {% endfor %}

    {% for url in urls %}
        {% if returned_statuses.get(url) == "200" %}
        {% else %}
            <p class="bad-url">{{ url }} <font color="red"> {{ returned_statuses.get(url) }}</font></p>
        {% endif %}
    {% endfor %}

{% endfor %}

{% endblock %}

unittests.py

import unittest

from test import is_reachable, get_status_code, check_single_url

class IsReachableTestCase(unittest.TestCase):
    """Tests the is_reachable function."""

    def test_is_google_reachable(self):
        result = is_reachable('www.google.com')
        self.assertTrue(result)

    def test_is_nonsense_reachable(self):
        result = is_reachable('ishskbeosjei.com')
        self.assertFalse(result)

class GetStatusCodeTestCase(unittest.TestCase):
    """Tests the get_status_code function."""

    def test_google_status_code(self):
        result = get_status_code('https://www.google.com')
        self.assertEqual(result, 200)

    def test_404_status_code(self):
        result = get_status_code('https://www.bbc.co.uk/404')
        self.assertEqual(result, 404)

class CheckSingleURLTestCase(unittest.TestCase):
    """Tests the check_single_url function"""

    def test_bbc_sport_url(self):
        result = check_single_url('http://www.bbc.co.uk/sport')
        self.assertEqual(result, '200')

    def test_nonsense_url(self):
        result = check_single_url('https://ksjsjsbdk.ievrygqlsp.com')
        self.assertEqual(result, 'UNREACHABLE')

    def test_timeout_url(self):
        result = check_single_url('https://www.bbc.co.uk:90')
        self.assertEqual(result, 'UNREACHABLE')

    def test_connrefused_url(self):
        result = check_single_url('http://127.0.0.1:8080')
        self.assertEqual(result, 'UNREACHABLE')

unittest.main() 
\$\endgroup\$
1
\$\begingroup\$

This looks like a very good status page. Well done !


Review

from settings import refresh_interval, filename, site_down

I recommend that you use set of config classes pattern for Flask.

class Config(object):
    DEBUG = False
    TESTING = False
    DATABASE_URI = 'sqlite:///:memory:'

class ProductionConfig(Config):
    DATABASE_URI = 'mysql://user@localhost/foo'

class DevelopmentConfig(Config):
    DEBUG = True

class TestingConfig(Config):
    TESTING = True
  • You can then load the correct one based on environment. More info can be found here.

  • This allows you to have different configs, (Mock databases, mock services) for testing and for development. For production you can have the real stuff.

from multiprocessing.dummy import Pool as ThreadPool
  • Why are you using dummy pool?
  • Make the Pool size configurable.
import requests
import json
import threading
from socket import gaierror, gethostbyname
from multiprocessing.dummy import Pool as ThreadPool
from urllib.parse import urlparse
from flask import Flask, render_template, jsonify
from time import gmtime, strftime
  • Consider separating builtin modules, third party modules and your own modules with a new line.
global returned_statuses
global last_update_time
t = threading.Timer
  • Use a class when you want to maintain state. Globals and functions are error prone and harder to understand.
t(refresh_interval, check_multiple_urls).start()
  • Don't create timers that call's it self? There are good alternatives here.
with open(filename) as f:
    checkurls = json.load(f)
returned_statuses = {}
last_update_time = 'time string'
  • If you encapsulate the state in-to a class such as Status this can go in the __init__
def check_multiple_urls():
    """ This function checks through urls specified in the checkurls.json file
    and returns their statuses as a dictionary every 60s."""
    statuses = {}
    temp_list_urls = []
    temp_list_statuses = []
    global returned_statuses
    global last_update_time
    t = threading.Timer
    t(refresh_interval, check_multiple_urls).start()
    for group, urls in checkurls.items():
        for url in urls:
            temp_list_urls.append(url)
    pool = ThreadPool(8)
    temp_list_statuses = pool.map(check_single_url, temp_list_urls)
    for i in range(len(temp_list_urls)):
        statuses[temp_list_urls[i]] = temp_list_statuses[i]
    last_update_time = strftime("%Y-%m-%d %H:%M:%S", gmtime())
    returned_statuses = statuses
  • Add some logical new lines here to separate different things that you are doing. Better to break to multiple functions. This function violates the single responsibility principle.

Bonus:

  • You are missing the requests library in your requirements.txt in github.
\$\endgroup\$
0
\$\begingroup\$

I like the unittests part in your code, and learn something about it from you too.

I noticed that you used global in your check_multiple_urls which is really bad style Why are global variables evil

def check_multiple_urls():
    ...
    global returned_statuses
    global last_update_time

Also it is bad practice to open a file and not call close on it.

with open(filename) as f:
    checkurls = json.load(f)
\$\endgroup\$
  • 2
    \$\begingroup\$ A with open doesn't require closing the file descriptor. \$\endgroup\$ – hjpotter92 Jul 25 '18 at 10:32
  • \$\begingroup\$ @hjpotter92 thank you for your commit, I did some search, with open will automatically invokes the close() method as soon as you leave this code block. \$\endgroup\$ – Aries_is_there Jul 25 '18 at 15:44
  • \$\begingroup\$ Hi, thanks for the insight. The unit tests were problematic as ideally they shouldn't rely on connections to third party servers. I've read httpretty was the way to go but sadly I couldn't get that module to install/import on my iPhone so just ran with it. Writing them was useful because it made me explicitly think about error handling I hadn't given much thought to before. Worth doing even for such a simple app. I know globals are frowned upon but if I'm honest I couldn't think of an easier way to achieve my goal so ran with it. That's probably because I just started with python... \$\endgroup\$ – emojipeach Jul 25 '18 at 16:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.