DeliciousSoda: A simple parser for robots.txt files

Question

This is my first big project that I've decided to release publically. It's available on my Github, and through pip3: pip3 install delicioussoda.

This is a simple parser for robots.txt files for a website. And yes, the name "DeliciousSoda" is a play off of "BeautifulSoup".

As I intended to keep developing and adding more features to this module, I would really appreciate some feedback on every part of my code. Here are a couple main things:

• How is my handling of ensuring the url is valid, and that the site has a robots.txt file?
• How is my use of custom exceptions?
• I've decided to use __ before some methods only used within the class. I also do this for any variables. Is this unnecessary?
• get_allowed and get_disallowed both seem unnecessarily complicated. I feel like I can accomplish the same thing with less code. Is this possible?
• How is my example usage? I use this to test the module without writing another file. Should I implement something like unittest?

Appreciate any and all feedback provided. Thank you in advance.

"""
DeliciousSoda
~~~~~~~~~~~~~

DeliciousSoda is a webscraper for robots.txt files. Basic usage:

>>> from delicioussoda import DeliciousSoda
>>> s = DeliciousSoda('https://www.google.com')
>>> s.get_allowed()
['Allow: /search/about', 'Allow: /search/static', ...]

:copyright: (c) 2019 by Ben Antonellis.
:license: MIT, see LICENSE for more details. 

"""

__all__ = [
    "DeliciousSoda"
]

# Standard Library Imports
import os
from typing import List, Dict, Union
import urllib.request
from urllib.error import HTTPError

# Install Requires
import requests
from requests.exceptions import SSLError

class DeliciousSoda():
    """
    A custom robots.txt parser. Assumes ' User-agent: * '.
    """

    def __init__(self, url=None):
        self.__url = url
        self.__robots_file = None

        if self.__url is not None:
            self.set_url(url)
            self.__validate_url()
            self.__content = self.__download_robots()

    def __str__(self) -> str:
        return f"DeliciousSoda Parsing: {self.__url}"

    def set_url(self, url: str) -> None:
        """
        Sets DeliciousSoda to retrieve robots.txt from this url.
        :param url -> str: Url
        :return: None
        """
        if not url.endswith("robots.txt"):
            if url.endswith("/"):
                self.__url = f"{url}robots.txt"
            else:
                self.__url = f"{url}/robots.txt"
        else:
            self.__url = url
        self.__content = self.__download_robots()

    def __validate_url(self) -> None:
        """
        Checks current url's validity.
        :return: None
        """
        try:
            response = requests.get(self.__url, timeout=5)
        except SSLError:
            raise InvalidUrlException("\n\nInvalidUrlException: Invalid Url.\n")

    def __download_robots(self) -> List[str]:
        """
        Downloads robots.txt file from url, loads the content from it, and returns a list of lines in the file.
        :return List[str]: Lines in the file
        """
        if self.__url is not None:
            # Ensure url is valid #
            self.__validate_url()

            # Ensure file exists #
            try:
                self.__robots_file, _ = urllib.request.urlretrieve(self.__url, filename="page.html")
            except HTTPError as error:
                if error.code == 404:
                    raise InvalidUrlException(f"\n\nInvalidUrlException: Site does not have a robots.txt file.\n")

            # Load lines and return list #
            with open(self.__robots_file, "r") as file:
                content = [line for line in file]

            # Remove file and return lines #
            os.remove(self.__robots_file)
            return content

        raise InvalidUrlException("\n\nInvalidUrlException: Invalid Url.\n")

    def get_sitemap(self) -> Union[str, None]:
        """
        Returns the sitemap of the webiste, None if it doesn't exist.
        :return Union[str, None]: Either the sitemap of robots.txt, or None
        """
        for line in self.__content:
            if line.startswith("Sitemap:"):
                return line[9:].rstrip()
        return None

    def get_allowed(self) -> List[str]:
        """
        Gets all allowed directories for the url.

        :return List[str]: Allowed directories
        """
        if self.__url is not None:
            allowed = []
            for line in self.__content:
                if line == "\n":
                    break
                if line.startswith("Allow:"):
                    allowed.append(line.rstrip()[7:])
            return allowed
        raise InvalidUrlException("\n\nInvalidUrlException: Invalid Url.\n")

    def get_disallowed(self) -> List[str]:
        """
        Gets all disallowed directories for the url.
        :return List[str]: Disallowed directories
        """
        if self.__url is not None:
            disallowed = []
            for line in self.__content:
                if line == "\n":
                    break
                if line.startswith("Disallow:"): 
                    disallowed.append(line.rstrip()[10:])
            return disallowed
        raise InvalidUrlException("\n\nInvalidUrlException: Invalid Url.\n")

    def get_all(self) -> Dict[str, List[str]]:
        """
        Returns a dict of both allowed and disallowed directories under User-agent: *.
        :return Dict[str, List[str]]: Dictionary of allowed and disallowed directories under User-agent: *
        """
        if self.__url is not None:
            return {
                "Allow": self.get_allowed(),
                "Disallow": self.get_disallowed()
            }
        raise InvalidUrlException("\n\nInvalidUrlException: Invalid Url.\n")

class InvalidUrlException(Exception):
    """ Url passed to DeliciousSoda object is `Invalid`. """
    pass

# Example Usage

if __name__ == "__main__":
    __robot = DeliciousSoda("https://www.google.com")
    __sitemap = __robot.get_sitemap()
    __allowed = __robot.get_allowed()
    __disallowed = __robot.get_disallowed()
    __both = __robot.get_all()  

    # print(__robot)
    # print(__sitemap)
    # print(__allowed)
    # print(__disallowed)
    # print(__both)

Answer 1

• I tried using __ once and found it to produce more problems than it's worth. Take:

  def __foo():
      return 'foo'
  
  
  class Bar:
      def __init__(self):
          self.__baz = __foo()
  
  
  Bar()
  

  NameError: name '_Bar__foo' is not defined
  

  I'd suggest you follow PEP 8 and only use it for classes that will be subclassed and where you need to prevent name collisions. __robot in global scope isn't following the advice in PEP 8's Naming Conventions - Method Names and Instance Variables

  Python mangles these names with the class name: if class Foo has an attribute named __a, it cannot be accessed by Foo.__a. (An insistent user could still gain access by calling Foo._Foo__a.) Generally, double leading underscores should be used only to avoid name conflicts with attributes in classes designed to be subclassed.

  Note: there is some controversy about the use of __names (see below).

• Your docstrings aren't consistent, nor are they PEP 257 compliant.
  You should lint your code and use Sphinx.

• Your class seems to have a lot of noise in it because you've chosen to make it mutable. This is a poor design choice.
• You can url.rstrip('/') to remove the need for another if in _set_url.
• If url is passed to DeliciousSoda than you run __download_robots twice. This is a waste.
• Don't add \n to exceptions. If you really want to add this mutate the InvalidUrlException's __init__. But seriously, please don't.
• Don't add the name of the exception to the exception message. Python does this automagically.
• If I were a consumer, I would prefer to be able to distinguish between the errors your program raises. You should raise different errors when there's an invalid URL or the site doesn't have a robots.
• __download_robots shouldn't be validating the URL. You should do that when you set the URL. Furthermore if your class was immutable then you could ignore all of this. Including the self.__url is not None part.
• __download_robots seems overly engineered. You're downloading a file to your filesystem to then reading the entire file into memory. Clearly there's an unneeded step.
• You should make set_url, __validate_url and __download_robots become one single, download_robots function, not method. This function can also take an *args and **kwargs that delegates to requests.get.
• The rest of your code is how you parse the robots. At which point you may as well just make that a function and return a dictionary.

"""
:copyright: (c) 2019 by Ben Antonellis, Peilonrayz.
:license: CC BY-SA 4.0.
"""

from typing import List, Dict
import collections
import os

from requests.exceptions import SSLError
import requests


def _normalize_robots_url(url: str) -> str:
    url = url.rstrip('/')
    if not url.endswith('robots.txt'):
        url += "/robots.txt"
    return url


def _download_robots(url: str, *args, **kwargs) -> str:
    url = _normalize_robots_url(url)
    try:
        r = requests.get(url, *args, **kwargs)
    except SSLError:
        raise InvalidUrlException("Invalid Url")
    r.raise_for_status()
    return r.text


def _parse_robots(robots: str) -> Dict[str, List[str]]:
    info: Dict[str, List[str]] = collections.defaultdict(list)
    for line in robots.split('\n'):
        name, value, *_ = *line.split(': ', 1), None
        if name and not name.startswith('#'):
            info[name].append(value)
    return info


def delicious_soda(url: str, *args, **kwargs) -> Dict[str, List[str]]:
    robots = _download_robots(url, *args, **kwargs)
    return _parse_robots(robots)


class InvalidUrlException(Exception):
    """ Url passed to DeliciousSoda object is `Invalid`. """
    pass


# Example Usage
if __name__ == "__main__":
    robots = delicious_soda("https://www.google.com")
    print(robots.keys())
    print(robots['Sitemap'])
    print(robots['User-agent'])
    print(robots['Allow'])
    print(robots['Disallow'])

---

Given the above functions I still have some concerns.

1. I personally consider calling _normalize_robots_url inside _download_robots an anti-pattern. The normalizing in fetch anti-pattern if you will.

   You should normalize your data before you perform the fetch. If I'm asking for example.com/robots-new.txt then I should have a pretty good reason to. Your 'help' of then appending /robots.txt to the end of my valid url has caused me needless problems.

   I recommend exposing _normalize_robots_url as a function that people can call if they want that behavior.

2. I also find delicious_soda to be an anti-pattern. The fetch and parse anti-pattern if you will.

   This is bad as it needlessly locks users into one usage. What if I want to cache the fetch, what if I need to stream it, what if I need to handle user authentication. What if I need to do something with the fetch you can't think of? At that point I can't use your library.

   IMO a library provider always needs to provide fetching and parsing as two separate entities. The cost to most users is having to do parse(fetch()). If you want to expose a helper function too, that's cool, but not exposing both as separate things is just bad.

3. The function _parse_robots seems like the best part of the library. Whilst the way you implemented it in DeliciousSoda isn't great. It's the best part of the library.

   But I don't think it's great as a stand alone library, as I've implemented most of the features in a 7 line function, which I'd hope most Python programmers could mimic with ease.

In all currently the library is just a 7 line function - _parse_robots. I don't think _download_robots is great as part of the library. I'd elect to just use requests which is more powerful and I know it won't break when I enter valid urls.

I would suggest expanding on _parse_robots in a way that makes the library somewhat more usable. Say converting it into a class and adding a DS.allowed_url method.

Answer 2

Disclaimer: Don't know Python nor PEP 8.

In __download_robots(self) you could reverse the if condition and return early which saves one level of indentation.

if self.__url is None:
    raise InvalidUrlException("\n\nInvalidUrlException: Invalid Url.\n")  

..the remaining code  

this applies to get_allowed(self), get_disallowed(self) and get_all(self) as well.

In the except block of __download_robots(self) you only check for if error.code == 404: shouldn't you check for other error codes, like 401 Unauthorized as well?

---

get_disallowed(self) and get_allowed(self) could be combined in one method in which you pass a search_phrase containing either Allow: or Disallow: and using the len(search_phrase) + 1 as the array-index.