launchpad-reviewers team mailing list archive

[Michael Nelson <michael.nelson@xxxxxxxxxxxxx>]

Michael Nelson has proposed merging lp:~michael.nelson/lazr-postgresql/bzr-is-not-a-dep into lp:lazr-postgresql.

Commit message:
Remove dependency on bzr.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~michael.nelson/lazr-postgresql/bzr-is-not-a-dep/+merge/298268

 
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/lazr-postgresql/bzr-is-not-a-dep into lp:lazr-postgresql.

=== modified file 'setup.py'
--- setup.py	2012-05-23 14:01:40 +0000
+++ setup.py	2016-06-24 03:32:44 +0000
@@ -46,7 +46,6 @@
           'Programming Language :: Python',
           ],
       install_requires=[
-          'bzr',
           'psycopg2',
           ],
       extras_require=dict(

=== modified file 'src/lazr/postgresql/tests/__init__.py'
--- src/lazr/postgresql/tests/__init__.py	2016-06-24 02:50:35 +0000
+++ src/lazr/postgresql/tests/__init__.py	2016-06-24 03:32:44 +0000
@@ -24,7 +24,7 @@
 
 
 # A 'just-works' workaround for Ubuntu not exposing initdb to the main PATH.
-os.environ['PATH'] = os.environ['PATH'] + ':/usr/lib/postgresql/8.4/bin'
+os.environ['PATH'] = os.environ['PATH'] + ':/usr/lib/postgresql/9.3/bin'
 
 
 db_resource = DatabaseManager()

=== modified file 'src/lazr/postgresql/tests/test_upgrade.py'
--- src/lazr/postgresql/tests/test_upgrade.py	2016-06-24 02:49:55 +0000
+++ src/lazr/postgresql/tests/test_upgrade.py	2016-06-24 03:32:44 +0000
@@ -19,6 +19,7 @@
 
 from doctest import ELLIPSIS
 import os
+import sys
 from textwrap import dedent
 
 from bzrlib.bzrdir import BzrDir
@@ -41,13 +42,13 @@
     Raises,
     StartsWith,
     )
-
 from lazr.postgresql.tests import (
     db_resource,
     ResourcedTestCase,
     )
 from lazr.postgresql.upgrade import (
     apply_patches_normal,
+    get_branch_info,
     CannotApply,
     missing_patches,
     PATCH_STANDARD,
@@ -136,7 +137,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_no_replication_dry_run_all_types(self):
         env = self.useFixture(FakedEnvironment(False))
         upgrade(env.connect, env.patchdir, True)
@@ -149,7 +150,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_no_replication_dry_run_specific_types(self):
         env = self.useFixture(FakedEnvironment(False))
         upgrade(env.connect, env.patchdir, True,
@@ -163,7 +164,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_no_replication_passes_one_only_down(self):
         env = self.useFixture(FakedEnvironment(False))
         upgrade(env.connect, env.patchdir, one_only=True)
@@ -176,7 +177,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_no_replication_actual_all_types(self):
         env = self.useFixture(FakedEnvironment(False))
         upgrade(env.connect, env.patchdir)
@@ -189,7 +190,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_no_replication_actual_specific_types(self):
         env = self.useFixture(FakedEnvironment(False))
         upgrade(env.connect, env.patchdir, patch_types=[PATCH_STANDARD])
@@ -203,7 +204,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_replication_dry_run(self):
         env = self.useFixture(FakedEnvironment(True))
         upgrade(env.connect, env.patchdir, True)
@@ -216,7 +217,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_replication_passes_one_only_down(self):
         env = self.useFixture(FakedEnvironment(True))
         upgrade(env.connect, env.patchdir, one_only=True)
@@ -228,7 +229,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_replication_actual_all_types(self):
         env = self.useFixture(FakedEnvironment(True))
         upgrade(env.connect, env.patchdir)
@@ -240,7 +241,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_replication_actual_specific_types(self):
         env = self.useFixture(FakedEnvironment(True))
         upgrade(env.connect, env.patchdir, patch_types=[PATCH_STANDARD])
@@ -253,7 +254,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_replication_actual_custom_cluster(self):
         env = self.useFixture(FakedEnvironment(True))
         upgrade(env.connect, env.patchdir, slony_cluster_name='foo')
@@ -284,7 +285,7 @@
             'close',
             ]
         self.assertEqual(expected_calls, env.log)
-        
+
     def test_apply_all_with_one_only(self):
         env = self.useFixture(FakedEnvironment(False, num_patches_results=2))
         upgrade(env.connect, env.patchdir, apply_all=True, one_only=True)
@@ -530,7 +531,7 @@
     resources = [('db', db_resource)]
 
     def test_grabs_branch_info_when_in_branch(self):
-        workdir = self.useFixture(TempDir()).path 
+        workdir = self.useFixture(TempDir()).path
         wt = BzrDir.create_standalone_workingtree(workdir)
         wt.branch.nick='foo'
         wt.commit("")
@@ -543,6 +544,54 @@
         self.assertThat(con, HasDBPatch(SCHEMA_MAJOR, 0, 0, 'foo', 2, 'abcdef'))
 
 
+class TestGetBranchInfo(TestCaseInTempDir):
+
+    def test_get_branch_info_when_in_branch(self):
+        workdir = self.test_dir
+        patches_dir = os.path.join(workdir, 'patches')
+        os.mkdir(patches_dir)
+        wt = BzrDir.create_standalone_workingtree(workdir)
+        wt.branch.nick = 'foo'
+        wt.commit("")
+        wt.commit("", rev_id='abcdef')
+
+        branch_info = get_branch_info(patches_dir)
+
+        self.assertEqual(branch_info, ('foo', 2, 'abcdef'))
+
+    def test_get_branch_info_when_not_in_branch(self):
+        workdir = self.useFixture(TempDir()).path
+        patches_dir = os.path.join(workdir, 'patches')
+        os.mkdir(patches_dir)
+        # The call to Branch.open_containing will traverse up
+        # the tree until it finds a .bzr or gets to /.
+        self.disable_directory_isolation()
+
+        branch_info = get_branch_info(patches_dir)
+
+        self.assertEqual(branch_info, (None, None, None))
+
+    def remove_module(self, module_name):
+        original_modules = sys.modules.copy()
+
+        del sys.modules[module_name]
+
+        def restore_modules():
+            sys.modules.update(original_modules)
+
+        self.addCleanup(restore_modules)
+
+    def test_get_branch_info_when_bzr_not_available(self):
+        workdir = self.test_dir
+        patches_dir = os.path.join(workdir, 'patches')
+        os.mkdir(patches_dir)
+        self.remove_module('bzrlib')
+
+        branch_info = get_branch_info(patches_dir)
+
+        self.assertEqual(branch_info, (None, None, None))
+
+
 class TestReportPatchTypes(ResourcedTestCase):
 
     resources = [('db', db_resource)]

=== modified file 'src/lazr/postgresql/upgrade.py'
--- src/lazr/postgresql/upgrade.py	2012-04-30 15:31:03 +0000
+++ src/lazr/postgresql/upgrade.py	2016-06-24 03:32:44 +0000
@@ -20,9 +20,15 @@
 import os.path
 import re
 from StringIO import StringIO
+import sys
 
-import bzrlib.branch
-import bzrlib.errors
+# We don't depend on bzrlib, but can record branch names
+# with schema upgrades if it is available.
+try:
+    import bzrlib.branch
+    import bzrlib.errors
+except ImportError:
+    pass
 
 from lazr.postgresql import slony
 
@@ -69,7 +75,7 @@
 def upgrade(connect, schema_dir, dry_run=False, slony_cluster_name=None,
         patch_types=None, apply_all=False, one_only=False):
     """Main programmatic entrypoint to the migration facility.
-    
+
     :param connect: A callable which returns a postgresql DB connection.
     :param schema_dir: A directory containing DB patches.
     :param dry_run: If True perform the changes in a transaction and roll it
@@ -202,6 +208,21 @@
     return sql
 
 
+def get_branch_info(patches_dir):
+    """Conditionally return branch info if bzrlib is available."""
+    branch_info = (None, None, None)
+    if 'bzrlib' in sys.modules:
+        try:
+            with bzrlib.initialize(True, StringIO(), StringIO(), StringIO()):
+                branch = bzrlib.branch.Branch.open_containing(patches_dir)[0]
+        except bzrlib.errors.NotBranchError:
+            pass
+        else:
+            branch_info = (branch.nick,) + branch.last_revision_info()
+
+    return branch_info
+
+
 def apply_patches_normal(con, patches_dir, patch_types=None, one_only=False):
     """Apply patches to a DB in a transaction on one node."""
     log = logging.getLogger("lazr.postgresql.upgrade")
@@ -209,13 +230,8 @@
         con, patches_dir, patch_types=patch_types, one_only=one_only)
     if not patches:
         return patches
-    with bzrlib.initialize(True, StringIO(), StringIO(), StringIO()):
-        try:
-            branch = bzrlib.branch.Branch.open_containing(patches_dir)[0]
-        except bzrlib.errors.NotBranchError:
-            branch_info = (None, None, None)
-        else:
-            branch_info = (branch.nick,) + branch.last_revision_info()
+
+    branch_info = get_branch_info(patches_dir)
     queries = []
     for patch in patches:
         # Cannot do schema management on schema management patches until they