Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.x] & [3.x]: GeneralConfig::maxBackups ignores zipped SQL files #11241

Closed
rob-c-baker opened this issue May 16, 2022 · 2 comments
Closed

[4.x] & [3.x]: GeneralConfig::maxBackups ignores zipped SQL files #11241

rob-c-baker opened this issue May 16, 2022 · 2 comments

Comments

@rob-c-baker
Copy link

rob-c-baker commented May 16, 2022

What happened?

Same issue exists on 4.x and 3.x.

Description

As part of our deployment process we do a database backup command similar to:

./craft db/backup --zip=1

This correctly produces a *.sql.zip file in the storage/backups directory. We also have the GeneralConfig::maxBackups unset in config/general.php meaning it should be set at it's default of 20: https://craftcms.com/docs/3.x/config/config-settings.html#maxbackups.

The code responsible for limiting the number of backup files kept has this line:

$files = glob($backupPath . DIRECTORY_SEPARATOR . '*.sql');

This ignores the files with names ending .zip, so we have some old .sql files in that directory (20 of them!) but hundreds of .zip files.

Expected behavior

Zipped SQL files should be limited to the count defined in GeneralConfig::maxBackups, either treating .sql and .zip files as grouped together or as 2 separate groups.

Actual behavior

Only files ending .sql are counted.

Craft CMS version

3 or 4

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

No response

@brandonkelly
Copy link
Member

Agree this is probably a more desired behavior, but considering you have to explicitly opt into backups being stored as a zip file via the db/backup command, I don’t really consider this a bug. So I’ve made the change for Craft 4.1 instead of the next 4.0.x release (607d1e0).

@brandonkelly
Copy link
Member

Craft 4.1 is out with this change ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants