Simple backup script with file removal after 7 days

Question

Here is the script that I would like reviewed for the following:

1. Best practices and design pattern usage
2. Correctness in unanticipated cases
3. Possible error checking

Any streamline updates/ideas are welcome!

import os
import tarfile
import time
# Create backup filename
timestr = time.strftime("%Y-%m-%d:%H%M%S")
output_filename = "minecraft-backup-" +timestr
backup_filename = output_filename + '.tar.gz'
# Locataion of final backup files
destination_dir = "/Users/username/Desktop/backup/"
destination_file = destination_dir + backup_filename
# Directory to backup
backup_dir = '/Users/username/.config/'
# Backup files
with tarfile.open(destination_file, "w:gz") as tar:
 for fn in os.listdir(backup_dir):
 p = os.path.join(backup_dir, fn)
 tar.add(p, arcname=fn)
# Remove files older than 7 days
current_time = time.time()
daysToDelete = 7
for dirpath,_,filenames in os.walk(destination_dir):
 for f in filenames:
 fileWithPath = os.path.abspath(os.path.join(dirpath, f))
 creation_time = os.path.getctime(fileWithPath)
 print("file available:",fileWithPath)
 if (current_time - creation_time) // (24 * 3600) >= daysToDelete:
 os.unlink(fileWithPath)
 print('{} removed'.format(fileWithPath))
 print("\n")
```

• Zachary Vance: Clearly explain what this does in a sentence or two. It's generally not possible to say if code is working correctly otherwise.

Answer 1

For many of your file manipulation operations prefer pathlib over os.

You can pull in your filename formatting including time to one f-string. Prefer datetime over time, and prefer timedelta over a day integer.

Note that the colon in your filename is not Windows-compatible. If you're able, use something like an underscore instead.

tar has built-in support for recursive directory archiving, so you should probably just do that instead of looping.

Suggested

import os
import tarfile
from datetime import datetime, timedelta
from pathlib import Path
now = datetime.now()
backup_filename = f'minecraft-backup-{now:%Y-%m-%d_%H%M%S}.tar.gz'
backup_dir = Path('~/.config').expanduser()
destination_dir = Path('~/Desktop/backup').expanduser()
destination_dir.mkdir(exist_ok=True)
destination_file = destination_dir / backup_filename
with tarfile.open(destination_file, 'w:gz') as tar:
 tar.add(backup_dir)
# Remove files older than 7 days
time_to_delete = timedelta(days=7)
for dirname, _, filenames in os.walk(destination_dir):
 dirpath = Path(dirname)
 for f in filenames:
 file_with_path = dirpath / f
 print('file available:', file_with_path)
 creation_time = datetime.fromtimestamp(
 os.path.getctime(file_with_path)
 )
 if now - creation_time >= time_to_delete:
 # file_with_path.unlink()
 print(file_with_path, 'removed')