Merge pull request #48 from dbmurphy/master

Adding TODO's and general cleanup comments

Author: dbmurphy

MongoBackup/Backup.py

@@ -21,6 +21,23 @@
 
 class Backup(object):
     def __init__(self, options):
+        # TODO-timv
+        """
+        We should move the layout to look like
+
+            self.options : {
+                "program_name" : None,
+                ...
+                "backup_options": {
+                    "host": localhost,
+                    "port": 27017,
+                },
+                "uploader": {},
+                "notifier": {},
+
+            }
+        Also all options should have defaults  for example program_name should always be something
+        """
         self.program_name = None
         self.version = None
         self.git_commit = None
@@ -88,16 +105,29 @@ def __init__(self, options):
             except Exception:
                 raise Exception, 'Field: %s is required by %s!' % (field, __name__), None
 
+        # Setup signal handler:
+        signal(SIGINT, self.cleanup_and_exit)
+        signal(SIGTERM, self.cleanup_and_exit)
+
+        # TODO Move to function
         # Set default lock file:
         if not self.lock_file:
             self.lock_file = '/tmp/%s.lock' % self.program_name
 
+        # TODO Move to function
+        # Setup backup dir name:
+        time_string = datetime.now().strftime("%Y%m%d_%H%M")
+        self.backup_root_subdirectory = "%s/%s" % (self.backup_name, time_string)
+        self.backup_root_directory = "%s/%s" % (self.backup_location, self.backup_root_subdirectory)
+
+        # TODO Move below to actual functions called by a master run function
         # Setup logging
         if self.verbose:
             self.log_level = logging.DEBUG
         logging.basicConfig(level=self.log_level,
                             format='[%(asctime)s] [%(levelname)s] [%(processName)s] [%(module)s:%(funcName)s:%(lineno)d] %(message)s')
 
+        # TODO Move any reference to the actual dumping into dumper classes
         # Check mongodump binary and set version + dump_gzip flag if 3.2+
         if os.path.isfile(self.backup_binary) and os.access(self.backup_binary, os.X_OK):
             with hide('running', 'warnings'), settings(warn_only=True):
@@ -111,6 +141,7 @@ def __init__(self, options):
             logging.fatal("Cannot find or execute the mongodump binary file %s!" % self.backup_binary)
             sys.exit(1)
 
+        #TODO should this be in init or a sub-function?
         # Get a DB connection
         try:
             self.db         = DB(self.host, self.port, self.user, self.password, self.authdb)
@@ -119,11 +150,7 @@ def __init__(self, options):
         except Exception, e:
             raise e
 
-        # Setup backup dir name:
-        time_string = datetime.now().strftime("%Y%m%d_%H%M")
-        self.backup_root_subdirectory = "%s/%s" % (self.backup_name, time_string)
-        self.backup_root_directory    = "%s/%s" % (self.backup_location, self.backup_root_subdirectory)
-
+        # TODO Move to notifier module called NSCA
         # Setup the notifier:
         try:
             if self.nsca_server and self.nsca_check_name:
@@ -136,21 +163,22 @@ def __init__(self, options):
         except Exception, e:
             raise e
 
-        # Setup signal handler:
-        signal(SIGINT, self.cleanup_and_exit)
-        signal(SIGTERM, self.cleanup_and_exit)
 
+    # TODO Rename class to be more exact as this assumes something went wrong
     # noinspection PyUnusedLocal
     def cleanup_and_exit(self, code, frame):
         if current_process().name == "MainProcess":
             logging.info("Starting cleanup and exit procedure! Killing running threads")
 
+            # TODO Rename the mongodumper module to just "backup" then have submodule in it for the backup type
+            # TODO Move submodules into self that populates as used?
             submodules = ['replset', 'sharding', 'mongodumper', 'oplogtailer', 'archiver', 'uploader_s3']
             for submodule_name in submodules:
                 submodule = getattr(self, submodule_name)
                 if submodule:
                     submodule.close()
 
+            # TODO Pass to notifier  Notifier(level,mesg) and it will pick the medium
             if self.notify_nsca:
                 self.notify_nsca.notify(self.notify_nsca.critical, "%s: backup '%s' failed!" % (
                     self.program_name,
@@ -172,6 +200,23 @@ def exception(self, error_message):
         return self.cleanup_and_exit(None, None)
 
     def run(self):
+        # TODO would be nice to have this code  look like: (functions do the work) and its readable
+        """
+        self.log(version_message,INFO)
+        self.lock()
+        self.start_timer()
+        if not self.is_sharded():
+            self.exec_unsharded()
+        else
+            self.exec_sharded()
+        self.stopTimer()
+        self.archive()
+        self.upload()
+        self.notify()
+        if self.db:
+            self.db.close()
+        self.log(backup_complete_message,INFO)
+        """
         logging.info("Starting %s version %s (git commit hash: %s)" % (self.program_name, self.version, self.git_commit))
 
         # noinspection PyBroadException