patternpythonModerate
Python Back-Up Script
Viewed 0 times
scriptbackpython
Problem
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()Solution
You can simplify paths using forward slashes:
Instead of converting a date to a string, which may be locale dependent too, and then replacing characters in it, better to use
If a variable contains a format, I would suffix it with
I don't really like global constants inside methods, so I would split
The advantage of this is that
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.Code Snippets
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 = datetime.datetime.now().strftime('%Y-%m-%d_%H%M')
return base_directory.format(date)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)Context
StackExchange Code Review Q#49351, answer score: 16
Revisions (0)
No revisions yet.