update tests and docs for migrate.versioning.script.*

4 files changed, 91 insertions, 31 deletions

diff --git a/migrate/versioning/script/__init__.py b/migrate/versioning/script/__init__.py
index ac8d1f4..c788eda 100644
--- a/migrate/versioning/script/__init__.py
+++ b/migrate/versioning/script/__init__.py
@@ -1,3 +1,6 @@
-from py import PythonScript
-from sql import SqlScript
-from base import BaseScript
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+from migrate.versioning.script.base import BaseScript
+from migrate.versioning.script.py import PythonScript
+from migrate.versioning.script.sql import SqlScript
diff --git a/migrate/versioning/script/base.py b/migrate/versioning/script/base.py
index 55aadd3..2fdc5df 100644
--- a/migrate/versioning/script/base.py
+++ b/migrate/versioning/script/base.py
@@ -1,12 +1,12 @@
 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 
-from migrate.versioning.base import log,operations
-from migrate.versioning import pathed,exceptions
+from migrate.versioning.base import log, operations
+from migrate.versioning import pathed, exceptions
 
 
 class BaseScript(pathed.Pathed):
-    """Base class for other types of scripts
+    """Base class for other types of scripts.
     All scripts have the following properties:
 
     source (script.source())
@@ -17,18 +17,20 @@ class BaseScript(pathed.Pathed):
       The operations defined by the script: upgrade(), downgrade() or both.
       Returns a tuple of operations.
       Can also check for an operation with ex. script.operation(Script.ops.up)
-    """
+    """ # TODO: sphinxfy this and implement it correctly
 
-    def __init__(self,path):
+    def __init__(self, path):
         log.info('Loading script %s...' % path)
         self.verify(path)
         super(BaseScript, self).__init__(path)
         log.info('Script %s loaded successfully' % path)
     
     @classmethod
-    def verify(cls,path):
-        """Ensure this is a valid script, or raise InvalidScriptError
+    def verify(cls, path):
+        """Ensure this is a valid script
         This version simply ensures the script file's existence
+
+        :raises: :exc:`InvalidScriptError <migrate.versioning.exceptions.InvalidScriptError>`
         """
         try:
             cls.require_found(path)
@@ -36,10 +38,16 @@ class BaseScript(pathed.Pathed):
             raise exceptions.InvalidScriptError(path)
 
     def source(self):
+        """:returns: source code of the script.
+        :rtype: string
+        """
         fd = open(self.path)
         ret = fd.read()
         fd.close()
         return ret
 
     def run(self, engine):
+        """Core of each BaseScript subclass.
+        This method executes the script.
+        """
         raise NotImplementedError()
diff --git a/migrate/versioning/script/py.py b/migrate/versioning/script/py.py
index cd7c634..e942f26 100644
--- a/migrate/versioning/script/py.py
+++ b/migrate/versioning/script/py.py
@@ -12,10 +12,13 @@ from migrate.versioning.script import base
 from migrate.versioning.util import import_path, load_model, construct_engine
 
 class PythonScript(base.BaseScript):
+    """Base for Python scripts"""
 
     @classmethod
     def create(cls, path, **opts):
-        """Create an empty migration script"""
+        """Create an empty migration script at specified path
+        
+        :returns: :class:`PythonScript instance <migrate.versioning.script.py.PythonScript>`"""
         cls.require_notfound(path)
 
         # TODO: Use the default script template (defined in the template
@@ -25,30 +28,51 @@ class PythonScript(base.BaseScript):
         src = template.get_script(template_file)
         shutil.copy(src, path)
 
+        return cls(path)
+
     @classmethod
     def make_update_script_for_model(cls, engine, oldmodel,
                                      model, repository, **opts):
-        """Create a migration script"""
+        """Create a migration script based on difference between two SA models.
+        
+        :param repository: path to migrate repository
+        :param oldmodel: dotted.module.name:SAClass or SAClass object
+        :param model: dotted.module.name:SAClass or SAClass object
+        :param engine: SQLAlchemy engine
+        :type repository: string or :class:`Repository instance <migrate.versioning.repository.Repository>`
+        :type oldmodel: string or Class
+        :type model: string or Class
+        :type engine: Engine instance
+        :returns: Upgrade / Downgrade script
+        :rtype: string
+        """
         
-        # Compute differences.
         if isinstance(repository, basestring):
             # oh dear, an import cycle!
             from migrate.versioning.repository import Repository
             repository = Repository(repository)
+
         oldmodel = load_model(oldmodel)
         model = load_model(model)
+
+        # Compute differences.
         diff = schemadiff.getDiffOfModelAgainstModel(
             oldmodel,
             model,
             engine,
             excludeTables=[repository.version_table])
+        # TODO: diff can be False (there is no difference?)
         decls, upgradeCommands, downgradeCommands = \
             genmodel.ModelGenerator(diff).toUpgradeDowngradePython()
 
         # Store differences into file.
-        template_file = None
-        src = template.get_script(template_file)
-        contents = open(src).read()
+        # TODO: add custom templates
+        src = template.get_script(None)
+        f = open(src)
+        contents = f.read()
+        f.close()
+
+        # generate source
         search = 'def upgrade():'
         contents = contents.replace(search, '\n\n'.join((decls, search)), 1)
         if upgradeCommands:
@@ -58,11 +82,18 @@ class PythonScript(base.BaseScript):
         return contents
 
     @classmethod
-    def verify_module(cls,path):
-        """Ensure this is a valid script, or raise InvalidScriptError"""
+    def verify_module(cls, path):
+        """Ensure path is a valid script
+        
+        :param path: Script location
+        :type path: string
+
+        :raises: :exc:`InvalidScriptError <migrate.versioning.exceptions.InvalidScriptError>`
+        :returns: Python module
+        """
         # Try to import and get the upgrade() func
         try:
-            module=import_path(path)
+            module = import_path(path)
         except:
             # If the script itself has errors, that's not our problem
             raise
@@ -73,8 +104,11 @@ class PythonScript(base.BaseScript):
         return module
 
     def preview_sql(self, url, step, **args):
-        """Mock engine to store all executable calls in a string \
-        and execute the step"""
+        """Mocks SQLAlchemy Engine to store all executed calls in a string 
+        and runs :meth:`PythonScript.run <migrate.versioning.script.py.PythonScript.run>`
+        
+        :returns: SQL file
+        """
         buf = StringIO()
         args['engine_arg_strategy'] = 'mock'
         args['engine_arg_executor'] = lambda s, p='': buf.write(s + p)
@@ -85,8 +119,14 @@ class PythonScript(base.BaseScript):
         return buf.getvalue()
             
     def run(self, engine, step):
-        """Core method of Script file. \
-            Exectues update() or downgrade() function"""
+        """Core method of Script file. 
+        Exectues :func:`update` or :func:`downgrade` functions
+
+        :param engine: SQLAlchemy Engine
+        :param step: Operation to run
+        :type engine: string
+        :type step: int
+        """
         if step > 0:
             op = 'upgrade'
         elif step < 0:
@@ -104,13 +144,16 @@ class PythonScript(base.BaseScript):
 
     @property
     def module(self):
-        if not hasattr(self,'_module'):
+        """Calls :meth:`migrate.versioning.script.py.verify_module`
+        and returns it.
+        """
+        if not hasattr(self, '_module'):
             self._module = self.verify_module(self.path)
         return self._module
 
     def _func(self, funcname):
-        fn = getattr(self.module, funcname, None)
-        if not fn:
+        try:
+            return getattr(self.module, funcname)
+        except AttributeError:
             msg = "The function %s is not defined in this script"
-            raise exceptions.ScriptError(msg%funcname)
-        return fn
+            raise exceptions.ScriptError(msg % funcname)
diff --git a/migrate/versioning/script/sql.py b/migrate/versioning/script/sql.py
index 88c3bf8..851fdf2 100644
--- a/migrate/versioning/script/sql.py
+++ b/migrate/versioning/script/sql.py
@@ -1,8 +1,15 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
 from migrate.versioning.script import base
 
+
 class SqlScript(base.BaseScript):
     """A file containing plain SQL statements."""
-    def run(self, engine, step):
+
+    # TODO: why is step parameter even here?
+    def run(self, engine, step=None):
+        """Runs SQL script through raw dbapi execute call"""
         text = self.source()
         # Don't rely on SA's autocommit here
         # (SA uses .startswith to check if a commit is needed. What if script
@@ -11,14 +18,13 @@ class SqlScript(base.BaseScript):
         try:
             trans = conn.begin()
             try:
-                # ###HACK: SQLite doesn't allow multiple statements through
+                # HACK: SQLite doesn't allow multiple statements through
                 # its execute() method, but it provides executescript() instead
                 dbapi = conn.engine.raw_connection()
                 if getattr(dbapi, 'executescript', None):
                     dbapi.executescript(text)
                 else:
                     conn.execute(text)
-                # Success
                 trans.commit()
             except:
                 trans.rollback()