Python Back-Up Script

Question

I wrote this script to backup some important files. It backs up the files to a local folder as well as to an external hard drive. It creates a new subdirectory which has its name constructed with the current date and time. Works perfectly, but I'm just wondering if there are any Python best-practices I'm missing or if there's any way I could've made it better!

import datetime
import os
import shutil
GOOGLE_DRIVE_DIRECTORY = 'C:\\Users\\Jeff\\Google Drive\\Manifest_Destiny'
MAIN_BACKUP_DIRECTORY = 'C:\\Users\\Jeff\\Desktop\\Manifest_Destiny_Backups\\md_backup_{0}'
EXTERNAL_DRIVE_DIRECTORY = 'F:\\My Files\\Manifest_Destiny_Backups\\md_backup_{0}'
def get_backup_directory(base_directory):
 date = str(datetime.datetime.now())[:16]
 date = date.replace(' ', '_').replace(':', '')
 return base_directory.format(date)
def copy_files(directory):
 for file in os.listdir(GOOGLE_DRIVE_DIRECTORY):
 file_path = os.path.join(GOOGLE_DRIVE_DIRECTORY, file)
 if os.path.isfile(file_path):
 shutil.copy(file_path, directory)
def perform_backup(base_directory):
 backup_directory = get_backup_directory(base_directory)
 os.makedirs(backup_directory)
 copy_files(backup_directory)
def main():
 perform_backup(MAIN_BACKUP_DIRECTORY)
 perform_backup(EXTERNAL_DRIVE_DIRECTORY)
if __name__ == '__main__':
 main()

• user94603: Check out sourceforge.net/projects/pycompry You can schedule it with crontab and use it for multiple directories, instead writing more lines mentioning directories in the code.

Answer 1

You can simplify paths using forward slashes:

GOOGLE_DRIVE_DIRECTORY = 'C:/Users/Jeff/Google Drive/Manifest_Destiny'
MAIN_BACKUP_DIRECTORY = 'C:/Users/Jeff/Desktop/Manifest_Destiny_Backups/md_backup_{0}'
EXTERNAL_DRIVE_DIRECTORY = 'F:/My Files/Manifest_Destiny_Backups/md_backup_{0}'

Instead of converting a date to a string, which may be locale dependent too, and then replacing characters in it, better to use strftime to generate exactly the format that you want:

def get_backup_directory(base_directory):
 date = datetime.datetime.now().strftime('%Y-%m-%d_%H%M')
 return base_directory.format(date)

If a variable contains a format, I would suffix it with _fmt or _format to clarify.

I don't really like global constants inside methods, so I would split copy_files like this:

def copy_files_to(srcdir, dstdir):
 for file in os.listdir(srcdir):
 file_path = os.path.join(srcdir, file)
 if os.path.isfile(file_path):
 shutil.copy(file_path, dstdir)
def copy_files(dstdir):
 copy_files_to(GOOGLE_DRIVE_DIRECTORY, dstdir)

The advantage of this is that copy_files_to is easier to unit test. I think it's also good to use more specific names for the directory variables to clarify which one is a source and destination.

Answer 2

I typically create wrapper methods around anything that is specific to the target platform [aka, WindowsTM, MacTM, LinuxTM, AndroidTM, etc]. I might use method names that look like setGoogleDriveDir( ), getExternDriveDir( ), learnBackupDir( ), etc. These methods may simply return the platform specific constants, but they may also do something complex to supply the needed value using platform specific components. For example, there might discover the specific platform using system calls and then use switch-case logic to compute appropriate return values.

All of this presumes that you anticipate the requirement to run your scripts on multiple platforms in future. Remember, "... No one will ever use more than 640 Kb of RAM ..."

• Legato: Welcome to CodeReview. Would you add more context to this answer? Right now it's somewhat unclear what you're actually suggesting.
• JaDogg: Those method names don't follow PEP8