patternpythonMinor
System backup on Linux - follow up
Viewed 0 times
systemfollowlinuxbackup
Problem
This is a follow-up to System backup on Linux. I've done a lot to my script since I asked my first question and I think I could improve a lot while doing this. The script still does the same, only a bit neater and cleaner. So let me explain what I changed, and sometimes why I've changed it.
-
At first I changed all the names of my variables and constants (I hope that I understood constants correctly, that these are values that stay always the same, no matter on which system/host this script is executed). They're all now in "snake_case" and the constants are also in capital case.
-
I started using the power of
-
I also created a function for every major task to get a better structure in my script, also it makes it a lot easier for me to debug it. I want to make one function out of the two functions for getting the metadata and the old backups because they're practically doing the same except for the file ending, but I'm not sure if that's a good/better idea as using two. Also I'm not sure about the return values.
-
The biggest change I've made in my backup function. I factored out the nesting to check the file age and add those files as a parameter to tar. The main reason for that was that I suddenly had around 120MB of parameters given to tar and somewhere below that lies the border of how many parameters tar can process. Now I'm using some nice tar features to make real incremental backups of my systems.
I'm really happy about what I did to my code, it looks pretty good and also works really well. But I guess there is still some space to improve it further, so I would appreciate any further feedback to my code!
```
import datetime
import os
import subprocess
import socket
import glob
i
-
At first I changed all the names of my variables and constants (I hope that I understood constants correctly, that these are values that stay always the same, no matter on which system/host this script is executed). They're all now in "snake_case" and the constants are also in capital case.
-
I started using the power of
os.path.join instead of string concatenation. But there is one problem, os.path.join doesn't set a final / on the filepath, so I had to add the DIR_PATH constant on every path that was using the backup_dir key. Still looking around for a better solution for that..-
I also created a function for every major task to get a better structure in my script, also it makes it a lot easier for me to debug it. I want to make one function out of the two functions for getting the metadata and the old backups because they're practically doing the same except for the file ending, but I'm not sure if that's a good/better idea as using two. Also I'm not sure about the return values.
-
The biggest change I've made in my backup function. I factored out the nesting to check the file age and add those files as a parameter to tar. The main reason for that was that I suddenly had around 120MB of parameters given to tar and somewhere below that lies the border of how many parameters tar can process. Now I'm using some nice tar features to make real incremental backups of my systems.
I'm really happy about what I did to my code, it looks pretty good and also works really well. But I guess there is still some space to improve it further, so I would appreciate any further feedback to my code!
```
import datetime
import os
import subprocess
import socket
import glob
i
Solution
One brief simple note, you could easily combine these functions:
They're identical apart from the file extention, so just make that a parameter.
This also fixes the inaccurate docstring for
def get_previous_backups(backup_dir):
"""Returns a list of tar.bz2 compressed backup files matching the date format"""
return [os.path.basename(x) for x in
glob.glob(backup_dir + DIR_PATH + host + '.[0-9][0-9][0-9][0-9]' +
'-[0-9][0-9]' + '-[0-9][0-9]' + '.tar.bz2')]
def get_metafile(backup_dir):
"""Returns the metadata file for incremental tar backups"""
return [os.path.basename(x) for x in
glob.glob(backup_dir + DIR_PATH + host + '.[0-9][0-9][0-9][0-9]' +
'-[0-9][0-9]' + '-[0-9][0-9]' + '.metadata')]They're identical apart from the file extention, so just make that a parameter.
def get_previous_backups(backup_dir, extention):
"""Returns a list of backup files matching the date format and file extention"""
return [os.path.basename(x) for x in
glob.glob(backup_dir + DIR_PATH + host + '.[0-9][0-9][0-9][0-9]' +
'-[0-9][0-9]' + '-[0-9][0-9]' + '.' + extention)]This also fixes the inaccurate docstring for
get_metafile that didn't make it clear that a list was returned.Code Snippets
def get_previous_backups(backup_dir):
"""Returns a list of tar.bz2 compressed backup files matching the date format"""
return [os.path.basename(x) for x in
glob.glob(backup_dir + DIR_PATH + host + '.[0-9][0-9][0-9][0-9]' +
'-[0-9][0-9]' + '-[0-9][0-9]' + '.tar.bz2')]
def get_metafile(backup_dir):
"""Returns the metadata file for incremental tar backups"""
return [os.path.basename(x) for x in
glob.glob(backup_dir + DIR_PATH + host + '.[0-9][0-9][0-9][0-9]' +
'-[0-9][0-9]' + '-[0-9][0-9]' + '.metadata')]def get_previous_backups(backup_dir, extention):
"""Returns a list of backup files matching the date format and file extention"""
return [os.path.basename(x) for x in
glob.glob(backup_dir + DIR_PATH + host + '.[0-9][0-9][0-9][0-9]' +
'-[0-9][0-9]' + '-[0-9][0-9]' + '.' + extention)]Context
StackExchange Code Review Q#107044, answer score: 2
Revisions (0)
No revisions yet.