5
\$\begingroup\$

I am creating a watchdog in python to monitor a folder and then create a backup with the new version when a file is modified. I extend the FileSystsemEventHandler class to add 7 extra variables. However not every variable is used on every event so I'm not sure where to initialize the variables and where to put the logic for initializing certain variables based on the event. My current program works however I am new to coding and want to make sure I am using proper coding convention.

import os
import getpass
import re
import glob
import sys 
import time
import pathlib
import logging 
import shutil
import tkinter as tk
from tkinter import filedialog

from watchdog.observers import Observer 
from watchdog.events import LoggingEventHandler, FileSystemEventHandler

SLEEP_TIME = 30

class OnMyWatch:

    def __init__(self, watch_src, watch_dest):
        self.observer = Observer()
        self.watch_src = watch_src
        self.watch_dest = watch_dest


    def run(self):
        event_handler = Handler(watch_src, watch_dest)
        self.observer.schedule(event_handler, watch_src, recursive = True)
        self.observer.start()
        try:
            while True:
                time.sleep(SLEEP_TIME)
        except:
            self.observer.stop()
            print("Observer Stopped")

        self.observer.join()



class Handler(FileSystemEventHandler):
    def __init__(self, watch_src, watch_dest): 
        self.watch_src = watch_src
        self.watch_dest = watch_dest

        self.src_dir = ''
        self.src_name = ''
        self.dest_dir = ''
        self.file_extension = ''


    # METHOD DONE
    def on_any_event(self, event):
        if event.event_type == 'moved':
            self.src_dir = os.path.join(self.watch_dest, os.path.relpath(event.src_path, self.watch_src))
            self.dest_dir = os.path.join(self.watch_dest, os.path.relpath(event.dest_path, self.watch_src))
                
        else:
            if event.is_directory:
                self.src_dir = event.src_path
                self.dest_dir = os.path.join(self.watch_dest, os.path.relpath(event.src_path, self.watch_src))
                
            else:
                src_path, self.file_extension = os.path.splitext(event.src_path)
                self.src_name = os.path.basename(src_path)
                self.src_dir = os.path.dirname(src_path)
                self.dest_dir = os.path.join(self.watch_dest, os.path.relpath(src_path, self.watch_src))       
    

    def on_created(self, event):
        # CREATED EVENT


    def on_modified(self, event):
        # MODIFIED EVENT


    def on_moved(self, event):
        # MOVED EVENT
            
    
    def on_deleted(self, event):
        # DELETED EVENT



if __name__ == "__main__":
    root = tk.Tk()
    root.withdraw()

    watch_src = filedialog.askdirectory()
    watch_dest = filedialog.askdirectory()

    watch = OnMyWatch(watch_src, watch_dest)
    watch.run()
\$\endgroup\$
1
  • 2
    \$\begingroup\$ You've omitted too much code to give a review--the variables you initialize are never used. Include the code of "on_created", etc. That said, the short answer is that your event handler is called many times, once for each event, so you should not be storing the variable for each specific event, on the handler. In fact, "on_any_event" should probably be left as the default. \$\endgroup\$ – Zachary Vance Nov 6 '20 at 7:06
3
\$\begingroup\$

Wait loops

I don't think that this:

    try:
        while True:
            time.sleep(SLEEP_TIME)
    except:
        self.observer.stop()
        print("Observer Stopped")

should be necessary. I would expect that if you simply join, it would do what you want - block until the observer is done.

Even if you retained the above structure, you're not using your try correctly. That except should be a finally. In other words, you don't want to stop when something goes wrong; you want to stop whether something goes wrong or not.

Pathlib

on_any_event contains a lot of os.path calls. You'll find that replacing these with pathlib.Path equivalents is better-structured. It's a more object-oriented and "syntactically sugary" way of manipulating paths.

Overall

I'm not convinced that writing these generic watchdog wrappers (Handler, OnMyWatch) is useful. It's subtracting from the utility of watchdog. What if you want code of your own to run once Handler.on_any_event changes some of its attributes? You could subclass Handler and override on_any_event, calling super().on_any_event to set those attributes, but that's a fairly poor data-passing pattern.

You'd be better off writing a purpose-built FileSystemEventHandler that does not have those file-component attributes, does not look at event.event_type, and handles specifically the event you care about.

In other words, this is abstraction that hurts rather than helps you, and introduces functions with side-effects that set attributes that don't make sense as attributes.

\$\endgroup\$

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.