Merge lp:~cjwatson/launchpad/code-import-push-bzr-ssh into lp:launchpad

[Colin Watson]

Push code imports over bzr+ssh rather than sftp.

Some imports seem to hang when pushing to sftp; I can reproduce this when experimenting locally, but bzr+ssh fixes things (as long as SSH connection sharing is disabled).

I've done this here rather than in the production configs in order that we can carry on pulling over sftp, which is apparently less CPU-intensive.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2018-05-06 08:52:34 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2018-08-16 12:53:42 +0000
@@ -169,6 +169,23 @@
             store.transport.base.rstrip('/'),
             config.codeimport.bazaar_branch_store.rstrip('/'))
 
+    def test__getMirrorURL(self):
+        # _getMirrorURL returns a URL for the branch with the given id.
+        store = BazaarBranchStore(get_transport_from_url(
+            'sftp://storage.example/branches'))
+        self.assertEqual(
+            'sftp://storage.example/branches/000186a0',
+            store._getMirrorURL(100000))
+
+    def test__getMirrorURL_push(self):
+        # _getMirrorURL prefers bzr+ssh over sftp when constructing push
+        # URLs.
+        store = BazaarBranchStore(get_transport_from_url(
+            'sftp://storage.example/branches'))
+        self.assertEqual(
+            'bzr+ssh://storage.example/branches/000186a0',
+            store._getMirrorURL(100000, push=True))
+
     def test_getNewBranch(self):
         # If there's no Bazaar branch of this id, then pull creates a new
         # Bazaar branch.

=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2018-03-15 20:44:04 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2018-08-16 12:53:42 +0000
@@ -166,9 +166,18 @@
         """Construct a Bazaar branch store based at `transport`."""
         self.transport = transport
 
-    def _getMirrorURL(self, db_branch_id):
+    def _getMirrorURL(self, db_branch_id, push=False):
         """Return the URL that `db_branch` is stored at."""
-        return urljoin(self.transport.base, '%08x' % db_branch_id)
+        base_url = self.transport.base
+        if push:
+            # Pulling large branches over sftp is less CPU-intensive, but
+            # pushing over bzr+ssh seems to be more reliable.
+            split = urlsplit(base_url)
+            if split.scheme == 'sftp':
+                base_url = urlunsplit([
+                    'bzr+ssh', split.netloc, split.path, split.query,
+                    split.fragment])
+        return urljoin(base_url, '%08x' % db_branch_id)
 
     def pull(self, db_branch_id, target_path, required_format,
              needs_tree=False, stacked_on_url=None):
@@ -218,7 +227,7 @@
             (i.e. actually transferred revisions).
         """
         self.transport.create_prefix()
-        target_url = self._getMirrorURL(db_branch_id)
+        target_url = self._getMirrorURL(db_branch_id, push=True)
         try:
             remote_branch = Branch.open(target_url)
         except NotBranchError: