2
\$\begingroup\$

This is what I came up with to delete files not modified within the last 30 minutes from a subdirectory named cache.

'''
Clear cache files older than 30 minutes.
'''

import os
import time

outer = os.getcwd()
cache = os.path.join(outer, 'cache')
count = 0
for f in os.listdir(cache):
        f = os.path.join(cache, f)
        use_by = time.time() - 30 * 60
        if os.path.getatime(f) < use_by:
                os.remove(f)
                count += 1
print("Removed {} files.".format(count))

Would love input as to how to make it more concise, as well as other improvements or details I might have missed.

Had tried to put os.getcwd() within os.path.join but got:

  File "/Users/mikekilmer/Envs/GLITCH/bin/../lib/python2.7/posixpath.py",         line 77, in join
    elif path == '' or path.endswith('/'):
AttributeError: 'builtin_function_or_method' object has no attribute 'endswith'
\$\endgroup\$
2
\$\begingroup\$

Instead of this:

for f in os.listdir(cache):
        # ...
        use_by = time.time() - 30 * 60
        if os.path.getatime(f) < use_by:
                # ...

Consider this:

use_by = time.time() - 30 * 60
for f in os.listdir(cache):
        # ...
        if os.path.getatime(f) < use_by:
                # ...

In this version, use_by is set only once, before the loop begins, which is slightly more efficient. Unless, you really want the timing to be re-evaluated in every iteration.


It can be error-prone and lead to nasty bugs to re-assign the loop variable (as well as method arguments), like here:

for f in os.listdir(cache):
        f = os.path.join(cache, f)

And in any case, I would recommend using more meaningful variable names, for example:

for filename in os.listdir(basedir):
    path = os.path.join(basedir, filename)

The code you posted sounds like a good idea. It could be turned into a reusable, general function with parameters:

  • basedir: base directory to clean
  • limit: time limit to use, in minutes

Something like this:

def wipe_unused(basedir, limit):
    """
    Remove files in *basedir* not accessed within *limit* minutes

    :param basedir: directory to clean
    :param limit: minutes
    """
    atime_limit = time.time() - limit
    count = 0
    for filename in os.listdir(basedir):
        path = os.path.join(basedir, filename)
        if os.path.getatime(path) < atime_limit:
            os.remove(path)
            count += 1
    print("Removed {} files.".format(count))


if __name__ == '__main__':
    wipe_unused(os.path.join(os.getcwd(), 'cache'), 30 * 60)
\$\endgroup\$
  • 1
    \$\begingroup\$ Wow. Thank you, man. Great tips! It's actually quite silly that I was reassigning the var AND calling time.time() on each pass. \$\endgroup\$ – MikeiLL Oct 4 '14 at 21:45

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.