Truncating files and logging in Python
Question: I'm writing a script that needs to go delete massive movie/media files from
our company's database. I'm developing in a Mac and Python environment both of
which are new to me. I'm trying to make this as resilient as possible since it
can possibly nuke the database of all projects currently in production and not
the old ones that are retired.

Would like to know, if there any severe logical flaws, if I'm logging right
etc. And any other suggestions to make this as robust and careful as possible
is appreciated.

    
    
    import os.path 
    import shutil 
    import datetime
    import logging
    
    root_path = "blah"
    age_in_days = 2
    truncate_size = 1024
    
    
    class TruncateOldFiles():
        def delete_files(root_path):
            if os.path.exists(root_path):
                for dirpath, dirnames, filenames in os.walk(root_path):
    
                    for file in filenames:
                        current_path = os.path.join(dirpath, file)
                        file_modified_time  = datetime.date(os.path.getmtime(current_path))
    
                        if ((datetime.datetime.now() - file_modified_time) > datetime.timedelta(days = age_in_days)):
                            count += 1
    
    
                    if count == len(files) and not os.path.isfile("donotdelete.txt"):
                        for file in filenames:
                            try:
                                with open (file, 'w+') as file:
                                    file.truncate(1024)
    
                                log()
    
                            except IOError:
                                pass
    
    
    
        def log():
            format = '%(asctime)s - %(name)s - %(levelname)s - %(message)s'
            logging.basicConfig(filename='myapp.log', level=logging.INFO, format = format)
            logging.info('Starting to truncate all files...')
    

Also, I was only able to compile this in terminal but don't quite know how to
debug logical errors from it. I'm used to coding in C++ and Java in IDEs and
here I'm using Xcode which does not seem as conducive to my style of
development.

Thank you.


Answer: I am not sure where the mentioned database comes into play, you seem only to
be working on filenames in a filesystem.

  * You are using `os.path.isfile()` which I would only use for testing if something that exist is a file (and not a directory, a link, etc.). It returns False (I had to look that up) if the name does not exists in the filesystem, so it works. But I would have expected it to throw an IOError. My recommendation is to use `os.path.exists()` instead.

  * be carefull with comparing `date()` and `datetime()`, they are not the same. And to get the `datetime()` from a timestamp use `.fromtimestamp`

  * I hope you realise that the script always looks for the 'donotdelete.txt' in the directory that you start the script from. `os.walk` does not do a `os.chdir`. If that is not what you want (and have the `donotdelete.txt` as a safeguard for certain specific directories by having one in each directory not to truncate, you should test for `os.path.exists(os.path.join(dirpath, 'donotdelete.txt'))`

  * `len(files)`? Do you mean `len(filenames)`, to see if all the files in the directory are old enough by comparing to `count`?

  * You correctly construct a `current_path` from the `dirpath` and the `filename` in the for loop where you test for age. In the truncating for loop you just use `file`, which would try to open in the current directory.

  * You are making an old-style class I would always make new classes [new style](http://docs.python.org/2/reference/datamodel.html#newstyle):

class TruncateOldFiles(object): ....

  * you should have `self` parameters in each of the methods, then you can call `log` as `self.log()`, as is your code would not work unless you do `TruncateOldFiles.log()`

  * I am not sure where the format info in the log gets filled in from. It write (after correcting how `log()` is called, only the line `starting to truncate .....` for each file it would truncate without additional information.

  * count is not initialised, just incremented, you need to do `count = 0`

  * I would pass in the root path, days and truncate size a parameters to the class creation. The latter two probably as defaults.

  * For this kind of destructive non-reversible operations I add a argument to the class creation to be able to have it run without doing anything except for logging. Maybe that is what the test for `donotdelete.txt` is for but that does not log anything, so you have no indication in the log what the program would be doing.

  * For many classes I have a verbose argument that helps with finding errors, this is for interactive running and different from the log

  * You have the 1024 hardcoded instead of using truncate_size, and you are opening and truncating files that are smaller than truncate_size which is unnecessary.

  * you use `file` (a python keyword) as a variable name both in the for loop as well in the with statement, it probably works, but it is not very good style and bound to lead to problems when you extend the code in the for loop.

My class would be more like (but `log()` would still need fixing):

    
    
    class TruncateOldFiles():
        def __init__(self, age_in_days=2, truncate_size=1024,
                     verbose=0, for_real=True):
            self._age = datetime.timedelta(days = age_in_days)
            self._truncate_size = truncate_size
            self._verbose = verbose
            self._for_real = for_real
    
        def delete_files(self, root_path):
            if not os.path.exists(root_path):
                if self._verbose > 1:
                    print 'root_path', self._root_path, 'does not exists'
                return
            for dirpath, dirnames, filenames in os.walk(root_path):
                count = 0
                for filename in filenames:
                    current_path = os.path.join(dirpath, filename)
                    file_modified_time  = datetime.datetime.fromtimestamp(os.path.getmtime(current_path))
                    if self._verbose > 0:
                        print file_modified_time, current_path
                    if ((datetime.datetime.now() - file_modified_time) > self._age):
                        count += 1
                if count == len(filenames) and not os.path.exists(os.path.join(dirpath, "donotdelete.txt")):
                    for filename in filenames:
                        current_path = os.path.join(dirpath, filename)
                        if os.path.getsize(current_path) <= self._truncate_size:
                            if self._verbose > 0:
                                print 'not big enough:', current_path
                            continue
                        try:
                            if self._verbose > 0:
                                print 'truncating:', file
                            if self._for_real:
                                with open (current_path, 'w+') as fp:
                                    fp.truncate(self._truncate_size)
                            self.log()
                        except IOError:
                            pass
    
        def log(self):
            format = '%(asctime)s - %(name)s - %(levelname)s - %(message)s'
            logging.basicConfig(filename='myapp.log', level=logging.INFO, format = format)
            logging.info('Starting to truncate all files...')
    

and the code to test this:

    
    
    tof = TruncateOldFiles(verbose=1, for_real=False)
    tof.delete_files('blah')
    

