Create SQLite backups

Question

I have this script for creating SQLite backups, and I was wondering whether you'd have any suggestions on how to improve this. I was thinking that maybe it should create the backup dir if it doesn't exist, but not sure if it's necessarily better.

Besides functionality/performance tips, any styling/formatting tips are also much appreciated.

#!/usr/bin/env python
"""
This script creates a timestamped database backup,
and cleans backups older than a set number of dates

"""    

from __future__ import print_function
from __future__ import unicode_literals

import argparse
import sqlite3
import shutil
import time
import os

DESCRIPTION = """
              Create a timestamped SQLite database backup, and
              clean backups older than a defined number of days
              """

# How old a file needs to be in order
# to be considered for being removed
NO_OF_DAYS = 7

def sqlite3_backup(dbfile, backupdir):
    """Create timestamped database copy"""

    if not os.path.isdir(backupdir):
        raise Exception("Backup directory does not exist: {}".format(backupdir))

    backup_file = os.path.join(backupdir, os.path.basename(dbfile) +
                               time.strftime("-%Y%m%d-%H%M%S"))

    connection = sqlite3.connect(dbfile)
    cursor = connection.cursor()

    # Lock database before making a backup
    cursor.execute('begin immediate')
    # Make new backup file
    shutil.copyfile(dbfile, backup_file)
    print ("\nCreating {}...".format(backup_file))
    # Unlock database
    connection.rollback()

def clean_data(backup_dir):
    """Delete files older than NO_OF_DAYS days"""

    print ("\n------------------------------")
    print ("Cleaning up old backups")

    for filename in os.listdir(backup_dir):
        backup_file = os.path.join(backup_dir, filename)
        if os.stat(backup_file).st_ctime < (time.time() - NO_OF_DAYS * 86400):
            if os.path.isfile(backup_file):
                os.remove(backup_file)
                print ("Deleting {}...".format(ibackup_file))

def get_arguments():
    """Parse the commandline arguments from the user"""

    parser = argparse.ArgumentParser(description=DESCRIPTION)
    parser.add_argument('db_file',
                        help='the database file that needs backed up')
    parser.add_argument('backup_dir',
                         help='the directory where the backup'
                              'file should be saved')
    return parser.parse_args()

if __name__ == "__main__":
    args = get_arguments()
    sqlite3_backup(args.db_file, args.backup_dir)
    clean_data(args.backup_dir)

    print ("\nBackup update has been successful.")

Note: Code works in both Python 2 and Python 3.

Answer 1

This looks like a pretty nice script to me.

I was thinking that maybe it should create the backup dir if it doesn't exist, but not sure if it's necessarily better.

I don't think there's a universal answer to that, it's your call how you want the script to behave. It depends on your use whether you need the script to create a backup directory if it doesn't exist, or if you want to avoid it creating directories at unintended locations. A compromise might be to add a -p flag like mkdir has to "create parents".

Not that it really matters, but I think it should be slightly more efficient, and more natural to flip these conditions:

    if os.stat(backup_file).st_ctime < (time.time() - NO_OF_DAYS * 86400):
        if os.path.isfile(backup_file):

That is:

    if os.path.isfile(backup_file):
        if os.stat(backup_file).st_ctime < (time.time() - NO_OF_DAYS * 86400):

You seem to be following PEP8 for the most part, with few exceptions:

• Don't put a space before parentheses, as in print (something)
• Put two empty lines in front of function definitions

Answer 2

There's no need to repeat yourself by defining DESCRIPTION (and in fact your strings have gotten out of sync). Just reuse the program's docstring:

def get_arguments():
    """Parse the commandline arguments from the user"""    
    parser = argparse.ArgumentParser(description=__doc__)
    …

Answer 3

1. Found a little typo in your code when executing it in a test project:

   print ("Deleting {}...".format(ibackup_file))

   should be

   print("Deleting {}...".format(backup_file))

2. Also you should use a consistence naming (e.g. backupdir vs. backup_dir). If got the feeling you made decision based on avoiding over 80 (using the underscore in the variable name ends in a line with 81 letters). Try to avoid doing changes like this. Instead make a new line for the parameters of a function or something like this.

3. I would suggest to store the elapse time in an own variable:

   elapse_time = time.time() - NO_OF_DAYS * 86400

   and

   if os.stat(backup_file).st_ctime < elapse_time: