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

Proposed by Colin Watson on 2018-08-16
Status: Merged
Merged at revision: 18752
Proposed branch: lp:~cjwatson/launchpad/code-import-push-bzr-ssh
Merge into: lp:launchpad
Diff against target: 61 lines (+29/-3)
2 files modified
lib/lp/codehosting/codeimport/tests/test_worker.py (+17/-0)
lib/lp/codehosting/codeimport/worker.py (+12/-3)
To merge this branch: bzr merge lp:~cjwatson/launchpad/code-import-push-bzr-ssh
Reviewer Review Type Date Requested Status
William Grant code 2018-08-16 Approve on 2018-08-16
Review via email: mp+353243@code.launchpad.net

Commit message

Push code imports over bzr+ssh rather than sftp.

Description of the change

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.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
2--- lib/lp/codehosting/codeimport/tests/test_worker.py 2018-05-06 08:52:34 +0000
3+++ lib/lp/codehosting/codeimport/tests/test_worker.py 2018-08-16 12:53:42 +0000
4@@ -169,6 +169,23 @@
5 store.transport.base.rstrip('/'),
6 config.codeimport.bazaar_branch_store.rstrip('/'))
7
8+ def test__getMirrorURL(self):
9+ # _getMirrorURL returns a URL for the branch with the given id.
10+ store = BazaarBranchStore(get_transport_from_url(
11+ 'sftp://storage.example/branches'))
12+ self.assertEqual(
13+ 'sftp://storage.example/branches/000186a0',
14+ store._getMirrorURL(100000))
15+
16+ def test__getMirrorURL_push(self):
17+ # _getMirrorURL prefers bzr+ssh over sftp when constructing push
18+ # URLs.
19+ store = BazaarBranchStore(get_transport_from_url(
20+ 'sftp://storage.example/branches'))
21+ self.assertEqual(
22+ 'bzr+ssh://storage.example/branches/000186a0',
23+ store._getMirrorURL(100000, push=True))
24+
25 def test_getNewBranch(self):
26 # If there's no Bazaar branch of this id, then pull creates a new
27 # Bazaar branch.
28
29=== modified file 'lib/lp/codehosting/codeimport/worker.py'
30--- lib/lp/codehosting/codeimport/worker.py 2018-03-15 20:44:04 +0000
31+++ lib/lp/codehosting/codeimport/worker.py 2018-08-16 12:53:42 +0000
32@@ -166,9 +166,18 @@
33 """Construct a Bazaar branch store based at `transport`."""
34 self.transport = transport
35
36- def _getMirrorURL(self, db_branch_id):
37+ def _getMirrorURL(self, db_branch_id, push=False):
38 """Return the URL that `db_branch` is stored at."""
39- return urljoin(self.transport.base, '%08x' % db_branch_id)
40+ base_url = self.transport.base
41+ if push:
42+ # Pulling large branches over sftp is less CPU-intensive, but
43+ # pushing over bzr+ssh seems to be more reliable.
44+ split = urlsplit(base_url)
45+ if split.scheme == 'sftp':
46+ base_url = urlunsplit([
47+ 'bzr+ssh', split.netloc, split.path, split.query,
48+ split.fragment])
49+ return urljoin(base_url, '%08x' % db_branch_id)
50
51 def pull(self, db_branch_id, target_path, required_format,
52 needs_tree=False, stacked_on_url=None):
53@@ -218,7 +227,7 @@
54 (i.e. actually transferred revisions).
55 """
56 self.transport.create_prefix()
57- target_url = self._getMirrorURL(db_branch_id)
58+ target_url = self._getMirrorURL(db_branch_id, push=True)
59 try:
60 remote_branch = Branch.open(target_url)
61 except NotBranchError: