HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Keeping remote folders in sync with local ones

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
keepinglocaloneswithfoldersremotesync

Problem

This started as a hacked-together tool to remove annoyances I was facing with experimenting with code on live remote servers, then getting that code into my development environment after experimenting. I thought others might be able to benefit from the tool, so I cleaned it up and released it as open source on GitHub.

The code in its current state appears below in its ~250 SLOC entirety. I'd appreciate any and all comments--correctness, style, best practices, logging, error handling, etc.

`#!/usr/bin/env python

"""
pytograph - Reflect local filesystem changes on a remote system in real time, automatically.

Requires Python 2.7, and the third-party Python packages config, pysftp, and watchdog.
"""

__author__ = 'Josh Dick '
__email__ = 'josh@joshdick.net'
__copyright__ = '(c) 2011-2012, Josh Dick'
__license__ = 'Simplified BSD'

from config import Config
from watchdog.observers import Observer
from watchdog.events import *
import argparse, getpass, logging, paramiko, posixpath, pysftp, sys, time

logFormat='%(levelname)s: %(message)s'
logging.basicConfig(format=logFormat)
logger = logging.getLogger('pytograph')
logger.setLevel(logging.INFO)

class PytoWatchdogHandler(PatternMatchingEventHandler):

"""
Watchdog event handler.
Triggers appropriate actions on a remote server via a RemoteControl when
specific Watchdog events are fired due to local filesystem changes.
"""

def __init__(self, remote_control = None, **kw):
super(PytoWatchdogHandler, self).__init__(**kw)

if (remote_control == None):
raise Exception('remote_control is a required parameter')
elif not isinstance(remote_control, RemoteControl):
raise Exception('remote_control must be an instance of RemoteControl')
self._remote_control = remote_control

def on_created(self, event):
if isinstance(event, DirCreatedEvent):
# Ignoring this event for now since directories will automatically
# be created on the remote server by transfer_file()
logger.d

Solution

#!/usr/bin/env python

"""
pytograph - Reflect local filesystem changes on a remote system in real time, automatically.

Requires Python 2.7, and the third-party Python packages config, pysftp, and watchdog.
"""

__author__ = 'Josh Dick '
__email__ = 'josh@joshdick.net'
__copyright__ = '(c) 2011-2012, Josh Dick'
__license__ = 'Simplified BSD'

from config import Config
from watchdog.observers import Observer
from watchdog.events import *
import argparse, getpass, logging, paramiko, posixpath, pysftp, sys, time

logFormat='%(levelname)s: %(message)s'
logging.basicConfig(format=logFormat)
logger = logging.getLogger('pytograph')
logger.setLevel(logging.INFO)

class PytoWatchdogHandler(PatternMatchingEventHandler):

  """
  Watchdog event handler.
  Triggers appropriate actions on a remote server via a RemoteControl when
  specific Watchdog events are fired due to local filesystem changes.
  """

  def __init__(self, remote_control = None, **kw):
    super(PytoWatchdogHandler, self).__init__(**kw)

    if (remote_control == None):


Check for None using is None rather then == None. You don't need those parens.

raise Exception('remote_control is a required parameter')


Rather then checking for None, just make remote_control not have a default value.

elif not isinstance(remote_control, RemoteControl):
      raise Exception('remote_control must be an instance of RemoteControl')


Checking the types of arguments is frowned upon in python. You shouldn't do this check, just assume the user did the correct thing. Also, if you must raise an error, use TypeError.

self._remote_control = remote_control

  def on_created(self, event):
    if isinstance(event, DirCreatedEvent):
      # Ignoring this event for now since directories will automatically
      # be created on the remote server by transfer_file()
      logger.debug('Ignoring DirCreatedEvent')


I might include detail about the directory in the log file

else:
      self._remote_control.transfer_file(event.src_path)

  def on_deleted(self, event):
    self._remote_control.delete_resource(event.src_path)


Why resource? You seem to switch randomnly between resource and file.

def on_modified(self, event):
    if isinstance(event, DirModifiedEvent):
      logger.debug('Ignoring DirModifiedEvent')
    else:
      self._remote_control.transfer_file(event.src_path)

  def on_moved(self, event):
    self._remote_control.move_resource(event.src_path, event.dest_path)

class RemoteControl:

  """
  Performs filesystem manipulations on a remote server,
  using data from the local machine's filesystem as necessary.
  """

  def __init__(self, sftp_connection = None, local_base = None, remote_base = None):
    if (sftp_connection == None):
      raise Exception('sftp_connection is a required parameter')
    elif not isinstance(sftp_connection, SFTPConnection):
      raise Exception('sftp_connection must be an instance of SFTPConnection')


Again, don't check types, and if its a required parameter don't provide a default value

self._connection = sftp_connection.connection
    self._ssh_prefix = sftp_connection.ssh_prefix
    self._local_base = local_base
    self._remote_base = remote_base

  # Given a full canonical path on the local filesystem, returns an equivalent full
  # canonical path on the remote filesystem.
  def get_remote_path(self, local_path):
    # Strip the local base path from the local full canonical path to get the relative path
    remote_relative = local_path[len(self._local_base):]
    return self._remote_base + remote_relative


Python has a number of functions like os.path.relpath and os.path.join which would probably be a better choice then string manipulation. They'll handle the funky corner cases.

def transfer_file(self, src_path):
    dest_path = self.get_remote_path(src_path)
    logger.info('Copying\n\t%s\nto\n\t%s:%s' % (src_path, self._ssh_prefix, dest_path))
    try:
      # Make sure the intermediate destination path to this file actually exists on the remote machine
      self._connection.execute('mkdir -p "' + os.path.split(dest_path)[0] + '"')
      self._connection.put(src_path, dest_path)
    except Exception as e:
      logger.error('Caught exception while copying:')
      logger.exception(e)


Here you catch any exception. But that includes exceptions caused by mispelling function names. That'll make it harder to find bugs. I'd recommend narrowing the scope to catch just whatever errors you really expect.

def delete_resource(self, src_path):
    dest_path = self.get_remote_path(src_path)
    logger.info('Deleting %s:%s' % (self._ssh_prefix, dest_path))
    try:
      self._connection.execute('rm -rf "' + dest_path + '"')
    except Exception as e:
      logger.error('Caught exception while deleting:')
      logger.exception(e)


These functions look very similar. This suggests that you should refactor them to combine the common elements.

```
def move_resource(se

Code Snippets

#!/usr/bin/env python

"""
pytograph - Reflect local filesystem changes on a remote system in real time, automatically.

<https://github.com/joshdick/pytograph>

Requires Python 2.7, and the third-party Python packages config, pysftp, and watchdog.
"""

__author__ = 'Josh Dick <joshdick.net>'
__email__ = 'josh@joshdick.net'
__copyright__ = '(c) 2011-2012, Josh Dick'
__license__ = 'Simplified BSD'

from config import Config
from watchdog.observers import Observer
from watchdog.events import *
import argparse, getpass, logging, paramiko, posixpath, pysftp, sys, time

logFormat='%(levelname)s: %(message)s'
logging.basicConfig(format=logFormat)
logger = logging.getLogger('pytograph')
logger.setLevel(logging.INFO)

class PytoWatchdogHandler(PatternMatchingEventHandler):

  """
  Watchdog event handler.
  Triggers appropriate actions on a remote server via a RemoteControl when
  specific Watchdog events are fired due to local filesystem changes.
  """

  def __init__(self, remote_control = None, **kw):
    super(PytoWatchdogHandler, self).__init__(**kw)

    if (remote_control == None):
raise Exception('remote_control is a required parameter')
elif not isinstance(remote_control, RemoteControl):
      raise Exception('remote_control must be an instance of RemoteControl')
self._remote_control = remote_control

  def on_created(self, event):
    if isinstance(event, DirCreatedEvent):
      # Ignoring this event for now since directories will automatically
      # be created on the remote server by transfer_file()
      logger.debug('Ignoring DirCreatedEvent')
else:
      self._remote_control.transfer_file(event.src_path)

  def on_deleted(self, event):
    self._remote_control.delete_resource(event.src_path)

Context

StackExchange Code Review Q#7317, answer score: 8

Revisions (0)

No revisions yet.