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

DB zip files are not removed from the /tmp directory. #49

Closed
cthorne91 opened this issue Oct 28, 2015 · 8 comments
Closed

DB zip files are not removed from the /tmp directory. #49

cthorne91 opened this issue Oct 28, 2015 · 8 comments

Comments

@cthorne91
Copy link

I'm not sure If I just haven't set up my configuration properly, but I wanted to raise a potential issue / see if I could get some help with this problem I'm having.

I'm using the s3 file storage system, and running the db only backup command on a cron. All of the temporary zip files stored in the system's /tmp directory are never removed.

Seems like line 52 of BackupCommand.php could use a call to delete the $backupZipFile?
I wanted to check if you guys see my problem also.

Thanks!

@groenewege
Copy link

See issue #35 for an example console task to clean up the tmp directory.

@cthorne91
Copy link
Author

Got it, the linux system should be in charge of clearing the tmp directory on reboot. However, I'll need to clear these files much more often than on reboot, so I'll use the command via a cron job.

Here is the slightly modified command that works for me if this helps any body else.

find /tmp -name laravel-backup-* -and -exec rm {} ';'

Thanks @groenewege

@freekmurze
Copy link
Member

From v2.8.0 onwards the package will explicitly delete the backup file in the temp directory.

@cthorne91
Copy link
Author

Thanks @freekmurze, I do notice that now with v2.8.0 the zip files are being deleted.

However, upon further inspection it looks like the database is being dumped more than once to the temp directory and none of those files are being removed either.

Although I'd love to submit a pull request to fix the issue, it might just be easier if I bring it to your attention considering you knowledge of the code base.

It seems to me the following simple changes would be necessary:

  1. Line 262 of BackupCommand.php should use the variable from line 254 instead of calling getFilesToBeBackedup again. (Which commits the dump to the temp directory - hence the many files in the tmp directory)
  2. Line 109 of BackupCommand.php should loop through all the files that were just closed into the zip and unlink them.

If I'm not making sense let me know.
Thanks.

@cthorne91 cthorne91 reopened this Nov 6, 2015
@freekmurze
Copy link
Member

@dirtbikerdude91 You're right that the original db should also be deleted. I'll fix this in the future.

You're not right on deleting all files after the loop of line 109, this would remove the original files and would make all users of this package incredibly unhappy :-)

@cthorne91
Copy link
Author

Ok, that makes sense. Thanks for the update @freekmurze.

A follow up question then.
If the original db dump will be left in the tmp directory on purpose, and I personally would rather it be deleted, since I'm uploading the zip files to AWS and off of the server any way. Is the recommended way to remove the DB file through a cron command to clear the tmp directory?
Or could we add a configuration flag to the laravel-backup.php to remove the db on line 109 conditionally?

Again, I'm still learning about this code base and the intentions of it, so maybe what I'm suggesting is not what this package aims to do.

Thanks for your time!

@freekmurze
Copy link
Member

The OS is responsible for cleaning up the tmp directory. On ubuntu the directory is emptied on every boot. I'm pretty sure you can configure every OS to periodically empty the folder. Google a bit for instructions for your OS.

Reading other people's code is a good way to learn stuff. If you have any questions, let me know.

@cthorne91
Copy link
Author

Awesome, thanks for your help @freekmurze!

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

No branches or pull requests

3 participants