0
\$\begingroup\$

I am trying to remove files in my directory if they are over 30 days. However I don't know what other checks I need to do. The examples I found online wasn't very thorough besides just subtracting the datetime of when the file was created against X amount of days.

import datetime
import os

def remove_old_files():
    """
    Remove any file over 30 days
    """

    file_path_directory = "inter/"
    today = datetime.datetime.now()
    retention_days = 30
    try:
        # Check each item in our path and verify if it's a file.
        for each_file in os.listdir(file_path_directory):
            each_file_path = os.path.join(file_path_directory, each_file)
            if os.path.isfile(each_file_path):

                # Get created datetime for each file and subtract the days from today.
                file_creation_date = datetime.datetime.fromtimestamp(os.path.getctime(each_file_path))
                number_of_days = today - file_creation_date
                if number_of_days.days > retention_days:
                    os.remove(each_file_path)

    except Exception as e:
        print("Exception in remove_old_files: ", e)
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Why Python 2? It's officially deprecated. \$\endgroup\$
    – Reinderien
    Jan 31 at 3:29

1 Answer 1

1
\$\begingroup\$

Review point number one is, basically, don't use Python 2 unless you have a gun to your head.

Make your path parametric.

Your exception handling isn't particularly useful, because it swallows all exceptions, printing only a very brief description of them, and preventing them from being passed to the upper level. If you want this to be fail-safe, make an except in the outer calling context and not in your function itself.

I think it would be less surprising to call your variable now instead of today, since you aren't rounding to the date. Similarly, represent your retention variable as a timedelta instead of just an integer.

Suggested

from datetime import datetime, timedelta
import os


def remove_old_files(file_path_directory="inter/"):
    """
    Remove any file older than 30 days
    """

    now = datetime.now()
    retention = timedelta(days=30)

    # Check each item in our path and verify if it's a file.
    for each_file in os.listdir(file_path_directory):
        each_file_path = os.path.join(file_path_directory, each_file)

        if os.path.isfile(each_file_path):
            # Get created datetime for each file and subtract the days from today.
            file_creation_date = datetime.fromtimestamp(os.path.getctime(each_file_path))
            age = now - file_creation_date
            if age > retention:
                os.remove(each_file_path)


if __name__ == '__main__':
    remove_old_files('./archived')

Note that this would have more niceties if and when you were able to adopt Python 3, including typehints and pathlib.

\$\endgroup\$
5
  • \$\begingroup\$ Yeah unfortunately I am stuck on python 2. Thank you for the feedback \$\endgroup\$
    – JThao
    Jan 31 at 14:30
  • \$\begingroup\$ Was days not needed for age? age.days > retention? \$\endgroup\$
    – JThao
    Jan 31 at 15:13
  • 1
    \$\begingroup\$ No, because it's nicer to compare against a timespan that has itself been initialised with a day count \$\endgroup\$
    – Reinderien
    Jan 31 at 15:16
  • \$\begingroup\$ I do have another question regarding path. If my path is path = "$STAFF/inter" is the correct way to do this is with os.environ e.g. home_path = os.environ['STAFF'] + "/inter/" ? \$\endgroup\$
    – JThao
    Jan 31 at 18:24
  • 1
    \$\begingroup\$ Sure, though don't use + - use join \$\endgroup\$
    – Reinderien
    Jan 31 at 18:28

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.