6
\$\begingroup\$

This is my script. I want it to run on a weekly basis on my linux server on a Raspberry Pi 4, to back up all files that may have changed. On the GPIO are just LEDs. It requires a program called notify_run, and a file called "BackupSettings.ini" in its own directory that looks like this:

[Sources]
Folder1=/home/pi/Desktop/Scripts/test/BackupTestEnvironment/Original Drive

[Destinations]
Folder1=/home/pi/Desktop/Scripts/test/BackupTestEnvironment/Backup Drive

...and can have multiple pairs of folders I would like to hear some suggestions to improve it, since im a bloody beginner, so please be patient with me :)

Here comes the main Code:

"""Import"""
import RPi.GPIO as GPIO
import shutil, os, hashlib, json, subprocess
from configparser import ConfigParser
from datetime import date
from operator import itemgetter


"""Init"""
config = ConfigParser()
config.read('BackupSettings.ini')
Sources = dict(config.items('Sources'))
Destinations = dict(config.items('Destinations'))
Indexnew = []
Indexold = {}
DeletedFiles = []
today = date.today().strftime("%Y_%m_%d")
GPIO.setmode(GPIO.BCM)
GPIO.setup(20, GPIO.OUT)
GPIO.setup(21, GPIO.OUT)



"""Def"""
def CreateNewIndex():
    global Indexnew
    for path, dirs, files in os.walk(Source):
        for file in files:
            filepath = path+"/"+file
            sha512_hash = hashlib.sha512()
            with open(filepath,"rb") as f:
                for byte_block in iter(lambda: f.read(4096),b""):
                    sha512_hash.update(byte_block)
                hashsum = sha512_hash.hexdigest()
            x, filepath = path.split(Source, 1)
            filepath = filepath+"/"
            data = {'Name': file, 'Path': filepath, 'Hashsum': hashsum}
            Indexnew.append(data)
    with open(Destination+"/"+today+".json", 'w+') as jsonout:
        json.dump(Indexnew,jsonout)

def ImportOldIndexes():
    global Indexold
    files = [f for f in os.listdir(Destination) if 
os.path.isfile(os.path.join(Destination,f))]
    if today+".json" in files:
        files.remove(today+".json")
    files.sort()
    for file in files:
        filepath = Destination+"/"+file
        Indexold[file] = json.load(open(filepath, "r"))

def Compare():
    keys = list(Indexold.keys())
    keys.sort()
    global Indexnew
    global DeletedFiles
    if Indexold:
        for x in Indexold[keys[-1]]:
            counter = 0
            for y in Indexnew:
                if itemgetter('Name', 'Path', 'Hashsum')(x) == itemgetter('Name', 'Path', 'Hashsum')(y):
                    y['Change'] = 'unchanged'
                elif itemgetter('Name', 'Hashsum')(x) == itemgetter('Name', 'Hashsum')(y):
                    y['Change'] = 'moved'
                elif itemgetter('Path', 'Hashsum')(x) == itemgetter('Path', 'Hashsum')(y):
                    y['Change'] = 'renamed'
                elif itemgetter('Name', 'Path')(x) == itemgetter('Name', 'Path')(y):
                    y['Change'] = 'newversion'
                else:
                    counter = counter + 1
                    if counter == len(Indexnew):
                        DeletedFiles.append(x)
        with open(Destination+"/DeletedFiles/"+today+".json", 'w+') as jsonout:
            json.dump(DeletedFiles,jsonout)
        for x in Indexnew:
            counter = 0
            for y in Indexold[keys[-1]]:
                if not itemgetter('Name', 'Path', 'Hashsum')(x) == itemgetter('Name', 'Path', 'Hashsum')(y):
                    if not itemgetter('Name', 'Hashsum')(x) == itemgetter('Name', 'Hashsum')(y):
                        if not itemgetter('Path', 'Hashsum')(x) == itemgetter('Path', 'Hashsum')(y):
                            if not itemgetter('Name', 'Path')(x) == itemgetter('Name', 'Path')(y):
                                counter = counter + 1
                                if counter == len(Indexold[keys[-1]]):
                                    x['Change'] = 'new'
        with open(Destination+"/"+today+".json", 'w+') as jsonout:
            json.dump(Indexnew,jsonout)

def Execute():
    error = 0
    for x in Indexnew:
        if x['Change'] == 'new' or x['Change'] == 'moved' or x['Change'] == 'renamed' or x['Change'] == 'newversion':
            Copyfrom = Source+x['Path']+x['Name']
            Copyto = Destination+"/"+today+x['Path']
            if not os.path.exists(Copyto):
                os.makedirs(Copyto)
            shutil.copy(Copyfrom, Copyto)
            sha512_hash = hashlib.sha512()
            with open(Copyto+x['Name'],"rb") as f:
                for byte_block in iter(lambda: f.read(4096),b""):
                    sha512_hash.update(byte_block)
                hashsum = sha512_hash.hexdigest()
            if not hashsum == x['Hashsum']:
                error = error + 1
                print("Error")
    if error == 0:
        print("Success")
    else:
        print("Error")
        GPIO.output(21, True)
        notify = subprocess.Popen(["notify-run", "send", '"Error during Backup"'], stdout=subprocess.PIPE, stderr=subprocess.PIPE)


"""Run"""
count = 0
for amount in Sources.values():
    count = count + 1
    Source = Sources["folder"+str(count)]
    Destination = Destinations["folder"+str(count)]
    GPIO.output(20, True)
    CreateNewIndex()
    ImportOldIndexes()
    Compare()
    Execute()
    GPIO.output(21, False)

I really hope I did not make any misstakes formating it after pasting it in here.

edit: I forgot to mention that it gets called by crontab

edit: It is supposed to run over critical data and it should be as failproof as possible, thats why i used sha512 instead of md5. Also, it should be capable to handle any kind of file it gets hit with. If you have any ideas on additional safety mechanisms, please tell me. The input will contain about 2 TB of Files, reaching from 1kb to 200GB each. I am the only one using it.

\$\endgroup\$
3
\$\begingroup\$

Style

Python has an "official" Style Guide for Python Code, that most programmers tend to follow, although it was originally only written for the standard library strictly speaking. It's worth a read.

The easiest first step to bring your code more in line with the style guide would be to change function and variable names to the usual lowercase_with_underscores.

Constant values, e.g. today for the sake of this program, are usually named with ALL_UPPERCASE_WITH_UNDERSCORES.

Luckily, there is a variety of tools readily available to help you with checking and (auto)fixing some/most of those issues.

Global variables

Global variables are usually best to be avoided, since they make it hard(er) to track where which parts of your program state are altered. To get rid of them, you will have to rewrite your functions to accept the relevant inputs as parameter and actually return the (modified) values you are going to work with. An example:

def create_new_index(source):
    index_new = []
    # _ is commonly used for "don't care" values
    for path, _, files in os.walk(source):
        ...

    return index_new

I chose to only pass source as argument to the function and also changed it's name since I would also recommend to not write the data in that function. This is not strictly necessary, but helps to keep your functions manageable since they have only a limited responsibility.

Loop like a native

Instead of

count = 0
for amount in Sources.values():
    count = count + 1
    Source = Sources["folder"+str(count)]
    Destination = Destinations["folder"+str(count)]

where amount is never used, you can use enumerate(...) like so

for count, _ in enumerate(Sources.values(), 1):
    source = Sources[f"folder" + str(count)]
    destination = Destinations["folder" + str(count)]
    new_index = create_new_index(source)

or if you don't care about the specific order of the folders, just use

for key, source in Sources.items():
    destination = Destinations[key]
    new_index = create_new_index(source)

As a bonus the last version also allows you to get rid of keys that have to follow a strict FolderX pattern on the keys/folder names in your config files.

itemgetter and matching

itemgetters can be reused.

get_nph = itemgetter('Name', 'Path', 'Hashsum')
get_nh = itemgetter('Name', 'Hashsum')
...
if get_nph(x) == get_nph(y):
    # ... do something
elif get_nh(x) == get_nh(y):
    # ... do something
# and so on

Defining a reusable itemgetter function for each parameter combination you are using is the most straightforward transformation of your code as shown in your question. In your original code you declared a new itemgetter function whenever you needed one. That is unnecessary as the example above shows.

But the code can work without itemgetters altogether, since all you do is compare three attributes of your file for equality and then act accordingly. An alternative implementation of the same approach might look like:

names_match = x['Name'] == y['Name']
paths_match = x['Path'] == y['Path']
hashsums_match = x['Hashsum'] == y['Hashsum']
if names_match and paths_match and hashsums_match:
    y['Change'] = 'unchanged'
elif names_match and hashsums_match:
    y['Change'] = 'moved'
elif paths_match and hashsums_match:
    y['Change'] = 'renamed'
elif names_match and paths_match:
    y['Change'] = 'newversion'
else:
    # ...

I'd tend to say this is more readable. But that might be a matter of taste.

Handling paths

Instead of manually concatenating paths like Destination + "/" + TODAY + x['Path'], you can use os.path.join(...) like os.path.join(destination, TODAY, x['Path']). An other advantage of this function is that it takes care to use the "correct" OS specific separator (i.e. \ on Windows, / on Linux), although this not strictly necessary here since the target is Linux only.

Python 3 also offers the PathLib module, which makes working with paths, and parts thereof, a little bit more convenient. Maybe have a look at it if you plan to rework your script or for future projects.

Running the script

You have marked the portion of your script that is supposed to run on execution with a block comment, """Run""". That might work for a person looking at your code, but the interpreter does not care to much about it. If you ever would try to import a function from your script because you want to reuse it, the back-up routine will be triggered.

Instead if __name__ == "__main__": should be used to (also) tell the interpreter which parts of the file are supposed to be run as script. There is also a nice explanation over at Stack Overflow.

if __name__ == "__main__":
    config = ConfigParser()
    config.read('BackupSettings.ini')
    sources = dict(config.items('Sources'))
    destinations = dict(config.items('Destinations'))
    for key, source in sources.items():
        destination = destinations[key]
        new_index = create_new_index(source)
        ...
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot! This is insanely helpful :) What I don´t really understand tho: what do you mean by reusing itemgetter. What I want to express there is I want to determine in which way the values change. I found that a file should be the same if at least 2 of these 3 values are the same. Depending on those values i can see then what kind of change it is. How can I do this by reusing the itemgetter? \$\endgroup\$ – Nico Lindenlauf Dec 10 '19 at 10:42
  • \$\begingroup\$ Oh, and I got a technikal question: What happens if the Lists get too long to fit into the 4 GB RAM? Is that even possible (realisticly)? \$\endgroup\$ – Nico Lindenlauf Dec 10 '19 at 11:00
  • \$\begingroup\$ I rewrote the itemgetter part. Is it clearer now? Regarding your other question, I'd say that its not that easy to calculate. The only thing that one can easily calculate is how much memory the hex representation of all the hashes would take. But then there are the names, paths, and a non-negligible overhead for datatypes in Python as (partial) "unknowns". An educated guess would be, that you'd need (lower) 10's of millions of files to fill the RAM on your Pi. But don't quote me on that ;-) \$\endgroup\$ – AlexV Dec 10 '19 at 11:59
  • \$\begingroup\$ Ah, thanks, that is WAY better than my approach :) When im back home i will rework my script and maby I´ll paste my results here in case someone has a similar project, or there are other recommendations then :) \$\endgroup\$ – Nico Lindenlauf Dec 10 '19 at 12:50
  • \$\begingroup\$ I just saw on another post about parallelization, and i wonder if it makes sense to use something like the multiprozessing module or the joblib module? \$\endgroup\$ – Nico Lindenlauf Dec 10 '19 at 14:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.