Simple Nagios Scraper - version 2

Question

Version 1 - Beginner web scraper for Nagios

Current version changes:

• Moved NAGIOS_DATA dictionary to separate file (and added to .gitignore)
• Used functions with DOCSTRINGS
• Removed the multiple redundant print() statements
• Actually read the PEP8 standards, and renamed variables to match the requirements

Again, beginner Python programmer. I appreciate the feedback!

import requests
from scraper import NAGIOS_DATA
from bs4 import BeautifulSoup
from requests.auth import HTTPBasicAuth, HTTPDigestAuth


def get_url_response(url, user, password, auth_type):
    """Get the response from a URL.

    Args:
        url (str): Nagios base URL
        user (str): Nagios username
        password (str): Nagios password
        auth_type (str): Nagios auth_type - Basic or Digest

    Returns: Response object
    """

    if auth_type == "Basic":
        return requests.get(url, auth=HTTPBasicAuth(user, password))
    return requests.get(url, auth=HTTPDigestAuth(user, password))


def main():
    """
    Main entry to the program
    """

    # for nagios_entry in ALL_NAGIOS_INFO:
    for url, auth_data in NAGIOS_DATA.items():
        user, password, auth_type = auth_data["user"], auth_data["password"], \
            auth_data["auth_type"]
        full_url = "{}/cgi-bin/status.cgi?host=all".format(url)
        response = get_url_response(full_url, user, password, auth_type)
        if response.status_code == 200:
            html = BeautifulSoup(response.text, "html.parser")
            for i, items in enumerate(html.select('td')):
                if i == 3:
                    hostsAll = items.text.split('\n')
                    hosts_up = hostsAll[12]
                    hosts_down = hostsAll[13]
                    hosts_unreachable = hostsAll[14]
                    hosts_pending = hostsAll[15]
                    hosts_problems = hostsAll[24]
                    hosts_types = hostsAll[25]
                if i == 12:
                    serviceAll = items.text.split('\n')
                    service_ok = serviceAll[13]
                    service_warning = serviceAll[14]
                    service_unknown = serviceAll[15]
                    service_critical = serviceAll[16]
                    service_problems = serviceAll[26]
                    service_types = serviceAll[27]
                # print(i, items.text) ## To get the index and text
            print_stats(
                user, url, hosts_up, hosts_down, hosts_unreachable,
                hosts_pending, hosts_problems, hosts_types, service_ok,
                service_warning, service_unknown, service_critical,
                service_problems, service_types)

    # print("Request returned:\n\n{}".format(html.text))
    # To get the full request


def print_stats(
        user, url, hosts_up, hosts_down, hosts_unreachable, hosts_pending,
        hosts_problems, hosts_types, service_ok, service_warning,
        service_unknown, service_critical, service_problems, service_types):
    print("""{}@{}:
                Hosts
    Up\tDown\tUnreachable\tPending\tProblems\tTypes
    {}\t{}\t{}\t\t{}\t{}\t\t{}
                Services
    OK\tWarning\tUnknown\tCritical\tProblems\tTypes
    {}\t{}\t{}\t{}\t\t{}\t\t{}""".format(
        user, url, hosts_up, hosts_down, hosts_unreachable, hosts_pending,
        hosts_problems, hosts_types, service_ok, service_warning,
        service_unknown, service_critical, service_problems, service_types))

if __name__ == '__main__':
    main()

scraper.py source:

NAGIOS_DATA = {
    'http://192.168.0.5/nagios': {
        'user': 'nagiosadmin',
        'password': 'PasswordHere1',
        'auth_type': 'Basic'
    },
    'https://www.example.com/nagios': {
        'user': 'exampleuser',
        'password': 'P@ssw0rd2',
        'auth_type': 'Digest'
    },
}

Answer 1

There are still a couple of rouge non-PEP8-compliant variable names: serviceAll and hostsAll.

This is a minor detail, but to avoid too much nesting I would suggest inverting this condition if response.status_code == 200:. Then you can write it like this:

if response.status_code != 200:
    continue # or raise an exception

html = BeautifulSoup(response.text, "html.parser")

IMO, such code is much easier to read. These kind of checks are also called guards (https://en.wikipedia.org/wiki/Guard_(computer_science)).

Instead of iterating through all the td tags, I would store them in a list and then extract the necessary elements with an index:

td_elements = list(html.select('td'))
hosts_all = td_elements[3].text.split('\n')
service_all = td_elements[12].text.split('\n')

Next, I would like to focus on the print_stats function. It takes way to many parameters and has become tough to work with. I suggest storing all variables you extract from the HTML in a dictionary, which you can then pass to the print_stats function.

extracted_information = {
    'hosts_up': hosts_all[12],
    'hosts_down': hosts_all[13],
    'hosts_unreachable': hosts_all[14],
    'hosts_pending': hosts_all[15],
    'hosts_problems': hosts_all[24],
    'hosts_types': hosts_all[25],
    'service_ok': service_all[13],
    'service_warning': service_all[14],
    'service_unknown': service_all[15],
    'service_critical': service_all[16],
    'service_problems': service_all[26],
    'service_types': service_all[27],
}

Then you would call the print_stats function like this: print_stats(user, url, extracted_information).

Of course, we now have to rewrite the print_stats function itself. The Python format function can also take named parameters. For example: "{param1} and {param2}".format(param1="a", param2="b") would return string "a and b". Using this we can rewrite the template string and pass the "unpacked" extracted_information dictionary to the format function.

def print_stats(user, url, extracted_information):
    template = """{user}@{url}:
                Hosts
    Up\tDown\tUnreachable\tPending\tProblems\tTypes
    {hosts_up}\t{hosts_down}\t{hosts_unreachable}\t\t{hosts_pending}\t{hosts_problems}\t\t{hosts_types}
                Services
    OK\tWarning\tUnknown\tCritical\tProblems\tTypes
    {service_ok}\t{service_warning}\t{service_unknown}\t{service_critical}\t\t{service_problems}\t\t{service_types}"""

    print(template.format(user=user, url=url, **extracted_information))