"--clean" flag not working as expected

[jerinpetergeorge]

Describe the bug

The --clean flag gracefully ignoring the DBBACKUP_CLEANUP_KEEP value

To Reproduce

# settings.py

DBBACKUP_STORAGE = 'django.core.files.storage.FileSystemStorage'
DBBACKUP_DEFAULT_SERVER_NAME = env("DBBACKUP_DEFAULT_SERVER_NAME", default="localhost")
def backup_filename(databasename, servername, datetime, extension, content_type):
    servername = DBBACKUP_DEFAULT_SERVER_NAME or servername
    directory = f"{DBBACKUP_PREFIX}/{databasename}"

    file_name = f"{servername}-{datetime}.{extension}"
    return f"{directory}/{file_name}"


DBBACKUP_FILENAME_TEMPLATE = backup_filename
DBBACKUP_CLEANUP_KEEP = 2
DBBACKUP_PREFIX = "test-prefix"

# terminal output

╰(generic-django-example-Kc6WXfau)─λ ls -al test-prefix/default/                                                                                                                                                         0 (0.864s) < 09:20:29
total 384
drwxrwxrwx 1 root root  4096 Apr 19 09:20 ./
drwxrwxrwx 1 root root     0 Apr 19 09:19 ../
-rwxrwxrwx 1 root root 74585 Apr 19 09:19 localhost-2023-04-19-034951.psql.bin*
-rwxrwxrwx 1 root root 74585 Apr 19 09:20 localhost-2023-04-19-035022.psql.bin*
-rwxrwxrwx 1 root root 74585 Apr 19 09:20 localhost-2023-04-19-035024.psql.bin*
-rwxrwxrwx 1 root root 74585 Apr 19 09:20 localhost-2023-04-19-035026.psql.bin*
-rwxrwxrwx 1 root root 74585 Apr 19 09:20 localhost-2023-04-19-035028.psql.bin*

Expected behavior

The test-prefix/default/ directory should only contain files equal to the value of DBBACKUP_CLEANUP_KEEP

Versions

Django-dbbackup

• django-dbbackup==4.0.2

External tools

• Python 3.9.11
• Django 3.2.18
• OS: Linux

[jerinpetergeorge]

The possible cause of this issue could be the DBBACKUP_FILENAME_TEMPLATE contains one or more slash (/); which makes sense.

If that is the case,

• How can we specify the directory in which the backup should store?
• How about adding a validation to the DBBACKUP_FILENAME_TEMPLATE so that it shouldn't contains any slash (/)?

[Archmonger]

The DBBACKUP_FILENAME_TEMPLATE setting is for the file name, not the output path.

In order to specify output path, that must be done when using the manage.py dbbackup -O "/my/path" option.

See the docs for more details.

[jerinpetergeorge]

-O OUTPUT_PATH, --output-path OUTPUT_PATH
Specify where to store "on local filesystem"

Unfortunately, the -O/--output-path won't work with "non-local storages (like S3)"

[jerinpetergeorge]

But, I was able to achieve something almost equal to the expected solution using DBBACKUP_STORAGE_OPTIONS with location

DBBACKUP_STORAGE = 'django.core.files.storage.FileSystemStorage'
DBBACKUP_DEFAULT_SERVER_NAME = env("DBBACKUP_DEFAULT_SERVER_NAME", default="localhost")
def backup_filename(databasename, servername, datetime, extension, content_type):
    return f"{servername}-{datetime}.{extension}"


DBBACKUP_FILENAME_TEMPLATE = backup_filename
DBBACKUP_CLEANUP_KEEP = 2
DBBACKUP_PREFIX = "test-prefix"
DBBACKUP_STORAGE_OPTIONS = {'location': 'abs/or/relative/path/to/backups'}

[Archmonger]

This might be good to document. Would you like to PR a note to this effect?

[jerinpetergeorge]

It seems to be the information is already in the docs. Technically, the DBBACKUP_STORAGE_OPTIONS accepts any kwargs that can be used while initializing the DBBACKUP_STORAGE. WDYT?

[Archmonger]

Yep, you're right. The existing documentation is solid.

[jerinpetergeorge]

apart from that, I think it is better to add a check to verify whether the DBBACKUP_FILENAME_TEMPLATE has any / (slash) in it -

[Archmonger]

Yup. How about a PR that adds a system check for this?