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")
```

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')