launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22546
[Merge] lp:~cjwatson/launchpad/branch-hosting-client into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/branch-hosting-client into lp:launchpad with lp:~cjwatson/launchpad/private-loggerhead as a prerequisite.
Commit message:
Add a client for the loggerhead API.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/branch-hosting-client/+merge/345704
This can't be merged until private-loggerhead is deployed and suitably configured on production, including sorting out haproxy and firewall issues.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/branch-hosting-client into lp:launchpad.
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2018-03-26 18:59:34 +0000
+++ configs/development/launchpad-lazr.conf 2018-05-16 19:07:37 +0000
@@ -47,6 +47,7 @@
access_log: /var/tmp/bazaar.launchpad.dev/codehosting-access.log
blacklisted_hostnames:
use_forking_daemon: True
+internal_bzr_api_endpoint: http://bazaar.launchpad.dev:8090/
internal_git_api_endpoint: http://git.launchpad.dev:19417/
git_browse_root: https://git.launchpad.dev/
git_anon_root: git://git.launchpad.dev/
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2017-06-16 10:02:47 +0000
+++ lib/lp/code/configure.zcml 2018-05-16 19:07:37 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -538,6 +538,13 @@
<allow interface="lp.code.interfaces.branchrevision.IBranchRevision"/>
</class>
+ <!-- Branch hosting -->
+ <securedutility
+ class="lp.code.model.branchhosting.BranchHostingClient"
+ provides="lp.code.interfaces.branchhosting.IBranchHostingClient">
+ <allow interface="lp.code.interfaces.branchhosting.IBranchHostingClient" />
+ </securedutility>
+
<!-- CodeReviewComment -->
<class class="lp.code.model.codereviewcomment.CodeReviewComment">
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2017-01-16 22:27:56 +0000
+++ lib/lp/code/errors.py 2018-05-16 19:07:37 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Errors used in the lp/code modules."""
@@ -13,7 +13,9 @@
'BranchCreatorNotMemberOfOwnerTeam',
'BranchCreatorNotOwner',
'BranchExists',
+ 'BranchFileNotFound',
'BranchHasPendingWrites',
+ 'BranchHostingFault',
'BranchTargetError',
'BranchTypeError',
'BuildAlreadyPending',
@@ -348,6 +350,34 @@
"""Raised when the user specifies an unrecognized branch type."""
+class BranchHostingFault(Exception):
+ """Raised when there is a fault fetching information about a branch."""
+
+
+class BranchFileNotFound(BranchHostingFault):
+ """Raised when a file does not exist in a branch."""
+
+ def __init__(self, unique_name, filename=None, file_id=None, rev=None):
+ super(BranchFileNotFound, self).__init__()
+ if (filename is None) == (file_id is None):
+ raise AssertionError(
+ "Exactly one of filename and file_id must be given.")
+ self.unique_name = unique_name
+ self.filename = filename
+ self.file_id = file_id
+ self.rev = rev
+
+ def __str__(self):
+ message = "Branch %s has no file " % self.unique_name
+ if self.filename is not None:
+ message += self.filename
+ else:
+ message += "with ID %s" % self.file_id
+ if self.rev is not None:
+ message += " at revision %s" % self.rev
+ return message
+
+
class GitRepositoryCreationException(Exception):
"""Base class for Git repository creation exceptions."""
=== added file 'lib/lp/code/interfaces/branchhosting.py'
--- lib/lp/code/interfaces/branchhosting.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/interfaces/branchhosting.py 2018-05-16 19:07:37 +0000
@@ -0,0 +1,56 @@
+# Copyright 2018 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interface for communication with the Loggerhead API."""
+
+__metaclass__ = type
+__all__ = [
+ 'IBranchHostingClient',
+ ]
+
+from zope.interface import Interface
+
+
+class IBranchHostingClient(Interface):
+ """Interface for the internal API provided by Loggerhead."""
+
+ def getDiff(unique_name, new, old=None, context_lines=None, logger=None):
+ """Get the diff between two revisions.
+
+ :param unique_name: Unique name of the branch.
+ :param new: The new revno or revision ID.
+ :param old: The old revno or revision ID. Defaults to the parent
+ revision of `new`.
+ :param context_lines: Include this number of lines of context around
+ each hunk.
+ :param logger: An optional logger.
+ :raises BranchHostingFault: if the API returned an error.
+ :return: The diff between `old` and `new` as a byte string.
+ """
+
+ def getInventory(unique_name, dirname, rev=None, logger=None):
+ """Get information on files in a directory.
+
+ :param unique_name: Unique name of the branch.
+ :param dirname: The name of the directory, relative to the root of
+ the branch.
+ :param rev: An optional revno or revision ID. Defaults to 'head:'.
+ :param logger: An optional logger.
+ :raises BranchFileNotFound: if the directory does not exist.
+ :raises BranchHostingFault: if the API returned some other error.
+ :return: The directory inventory as a dict: see
+ `loggerhead.controllers.inventory_ui` for details.
+ """
+
+ def getBlob(unique_name, file_id, rev=None, logger=None):
+ """Get a blob by file name from a branch.
+
+ :param unique_name: Unique name of the branch.
+ :param file_id: The file ID of the file. (`getInventory` may be
+ useful to retrieve this.)
+ :param rev: An optional revno or revision ID. Defaults to 'head:'.
+ :param logger: An optional logger.
+ :raises BranchFileNotFound: if the directory does not exist.
+ :raises BranchHostingFault: if the API returned some other error.
+ :return: The blob content as a byte string.
+ """
=== added file 'lib/lp/code/model/branchhosting.py'
--- lib/lp/code/model/branchhosting.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/branchhosting.py 2018-05-16 19:07:37 +0000
@@ -0,0 +1,153 @@
+# Copyright 2018 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Communication with the Loggerhead API for Bazaar code hosting."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+ 'BranchHostingClient',
+ ]
+
+import json
+import sys
+
+from lazr.restful.utils import get_current_browser_request
+import requests
+from six import reraise
+from six.moves.urllib_parse import (
+ quote,
+ urlencode,
+ urljoin,
+ )
+from zope.interface import implementer
+
+from lp.code.errors import (
+ BranchFileNotFound,
+ BranchHostingFault,
+ )
+from lp.code.interfaces.branchhosting import IBranchHostingClient
+from lp.services.config import config
+from lp.services.timeline.requesttimeline import get_request_timeline
+from lp.services.timeout import (
+ get_default_timeout_function,
+ TimeoutError,
+ urlfetch,
+ )
+
+
+class RequestExceptionWrapper(requests.RequestException):
+ """A non-requests exception that occurred during a request."""
+
+
+@implementer(IBranchHostingClient)
+class BranchHostingClient:
+ """A client for the Bazaar Loggerhead API."""
+
+ def __init__(self):
+ self.endpoint = config.codehosting.internal_bzr_api_endpoint
+
+ def _request(self, method, unique_name, quoted_tail, as_json=False,
+ **kwargs):
+ """Make a request to the Loggerhead API."""
+ # Fetch the current timeout before starting the timeline action,
+ # since making a database query inside this action will result in an
+ # OverlappingActionError.
+ get_default_timeout_function()()
+ timeline = get_request_timeline(get_current_browser_request())
+ if as_json:
+ components = [unique_name, "+json", quoted_tail]
+ else:
+ components = [unique_name, quoted_tail]
+ path = "/" + "/".join(components)
+ action = timeline.start(
+ "branch-hosting-%s" % method, "%s %s" % (path, json.dumps(kwargs)))
+ try:
+ response = urlfetch(
+ urljoin(self.endpoint, path), trust_env=False, method=method,
+ **kwargs)
+ except TimeoutError:
+ # Re-raise this directly so that it can be handled specially by
+ # callers.
+ raise
+ except requests.RequestException:
+ raise
+ except Exception:
+ _, val, tb = sys.exc_info()
+ reraise(
+ RequestExceptionWrapper, RequestExceptionWrapper(*val.args),
+ tb)
+ finally:
+ action.finish()
+ if as_json:
+ if response.content:
+ return response.json()
+ else:
+ return None
+ else:
+ return response.content
+
+ def _get(self, unique_name, tail, **kwargs):
+ return self._request("get", unique_name, tail, **kwargs)
+
+ def getDiff(self, unique_name, new, old=None, context_lines=None,
+ logger=None):
+ """See `IBranchHostingClient`."""
+ try:
+ if logger is not None:
+ if old is None:
+ logger.info(
+ "Requesting diff for %s from parent of %s to %s" %
+ (unique_name, new, new))
+ else:
+ logger.info(
+ "Requesting diff for %s from %s to %s" %
+ (unique_name, old, new))
+ quoted_tail = "diff/%s" % quote(new, safe="")
+ if old is not None:
+ quoted_tail += "/%s" % quote(old, safe="")
+ return self._get(
+ unique_name, quoted_tail, as_json=False,
+ params={"context_lines": context_lines})
+ except requests.RequestException as e:
+ raise BranchHostingFault(
+ "Failed to get diff from Bazaar branch: %s" % e)
+
+ def getInventory(self, unique_name, dirname, rev=None, logger=None):
+ """See `IBranchHostingClient`."""
+ try:
+ if logger is not None:
+ logger.info(
+ "Requesting inventory at %s from branch %s" %
+ (dirname, unique_name))
+ quoted_tail = "files/%s/%s" % (
+ quote(rev or "head:", safe=""), quote(dirname.lstrip("/")))
+ return self._get(unique_name, quoted_tail, as_json=True)
+ except requests.RequestException as e:
+ if e.response.status_code == requests.codes.NOT_FOUND:
+ raise BranchFileNotFound(
+ unique_name, filename=dirname, rev=rev)
+ else:
+ raise BranchHostingFault(
+ "Failed to get inventory from Bazaar branch: %s" % e)
+
+ def getBlob(self, unique_name, file_id, rev=None, logger=None):
+ """See `IBranchHostingClient`."""
+ try:
+ if logger is not None:
+ logger.info(
+ "Fetching file ID %s from branch %s" %
+ (file_id, unique_name))
+ return self._get(
+ unique_name,
+ "download/%s/%s" % (
+ quote(rev or "head:", safe=""), quote(file_id, safe="")),
+ as_json=False)
+ except requests.RequestException as e:
+ if e.response.status_code == requests.codes.NOT_FOUND:
+ raise BranchFileNotFound(
+ unique_name, file_id=file_id, rev=rev)
+ else:
+ raise BranchHostingFault(
+ "Failed to get file from Bazaar branch: %s" % e)
=== added file 'lib/lp/code/model/tests/test_branchhosting.py'
--- lib/lp/code/model/tests/test_branchhosting.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/tests/test_branchhosting.py 2018-05-16 19:07:37 +0000
@@ -0,0 +1,240 @@
+# Copyright 2018 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Unit tests for `BranchHostingClient`.
+
+We don't currently do integration testing against a real hosting service,
+but we at least check that we're sending the right requests.
+"""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from contextlib import contextmanager
+
+from httmock import (
+ all_requests,
+ HTTMock,
+ )
+from lazr.restful.utils import get_current_browser_request
+from testtools.matchers import MatchesStructure
+from zope.component import getUtility
+from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
+
+from lp.code.errors import (
+ BranchFileNotFound,
+ BranchHostingFault,
+ )
+from lp.code.interfaces.branchhosting import IBranchHostingClient
+from lp.services.job.interfaces.job import (
+ IRunnableJob,
+ JobStatus,
+ )
+from lp.services.job.model.job import Job
+from lp.services.job.runner import (
+ BaseRunnableJob,
+ JobRunner,
+ )
+from lp.services.timeline.requesttimeline import get_request_timeline
+from lp.services.timeout import (
+ get_default_timeout_function,
+ set_default_timeout_function,
+ )
+from lp.services.webapp.url import urlappend
+from lp.testing import TestCase
+from lp.testing.layers import ZopelessDatabaseLayer
+
+
+class TestBranchHostingClient(TestCase):
+
+ layer = ZopelessDatabaseLayer
+
+ def setUp(self):
+ super(TestBranchHostingClient, self).setUp()
+ self.client = getUtility(IBranchHostingClient)
+ self.endpoint = removeSecurityProxy(self.client).endpoint
+ self.request = None
+
+ @contextmanager
+ def mockRequests(self, status_code=200, content=b"", reason=None,
+ set_default_timeout=True):
+ @all_requests
+ def handler(url, request):
+ self.assertIsNone(self.request)
+ self.request = request
+ return {
+ "status_code": status_code,
+ "content": content,
+ "reason": reason,
+ }
+
+ with HTTMock(handler):
+ original_timeout_function = get_default_timeout_function()
+ if set_default_timeout:
+ set_default_timeout_function(lambda: 60.0)
+ try:
+ yield
+ finally:
+ set_default_timeout_function(original_timeout_function)
+
+ def assertRequest(self, url_suffix, **kwargs):
+ self.assertThat(self.request, MatchesStructure.byEquality(
+ url=urlappend(self.endpoint, url_suffix), method="GET", **kwargs))
+ timeline = get_request_timeline(get_current_browser_request())
+ action = timeline.actions[-1]
+ self.assertEqual("branch-hosting-get", action.category)
+ self.assertEqual(
+ "/" + url_suffix.split("?", 1)[0], action.detail.split(" ", 1)[0])
+
+ def test_getDiff(self):
+ with self.mockRequests(content=b"---\n+++\n"):
+ diff = self.client.getDiff("~owner/project/branch", "2", "1")
+ self.assertEqual(b"---\n+++\n", diff)
+ self.assertRequest("~owner/project/branch/diff/2/1")
+
+ def test_getDiff_no_old_revision(self):
+ with self.mockRequests(content=b"---\n+++\n"):
+ diff = self.client.getDiff("~owner/project/branch", "2")
+ self.assertEqual(b"---\n+++\n", diff)
+ self.assertRequest("~owner/project/branch/diff/2")
+
+ def test_getDiff_context_lines(self):
+ with self.mockRequests(content=b"---\n+++\n"):
+ diff = self.client.getDiff(
+ "~owner/project/branch", "2", "1", context_lines=4)
+ self.assertEqual(b"---\n+++\n", diff)
+ self.assertRequest("~owner/project/branch/diff/2/1?context_lines=4")
+
+ def test_getDiff_failure(self):
+ with self.mockRequests(status_code=400, reason=b"Bad request"):
+ self.assertRaisesWithContent(
+ BranchHostingFault,
+ "Failed to get diff from Bazaar branch: "
+ "400 Client Error: Bad request",
+ self.client.getDiff, "~owner/project/branch", "2", "1")
+
+ def test_getInventory(self):
+ with self.mockRequests(content=b'{"filelist": []}'):
+ response = self.client.getInventory(
+ "~owner/project/branch", "dir/path/file/name")
+ self.assertEqual({"filelist": []}, response)
+ self.assertRequest(
+ "~owner/project/branch/+json/files/head%3A/dir/path/file/name")
+
+ def test_getInventory_revision(self):
+ with self.mockRequests(content=b'{"filelist": []}'):
+ response = self.client.getInventory(
+ "~owner/project/branch", "dir/path/file/name", rev="a")
+ self.assertEqual({"filelist": []}, response)
+ self.assertRequest(
+ "~owner/project/branch/+json/files/a/dir/path/file/name")
+
+ def test_getInventory_not_found(self):
+ with self.mockRequests(status_code=404, reason=b"Not found"):
+ self.assertRaisesWithContent(
+ BranchFileNotFound,
+ "Branch ~owner/project/branch has no file dir/path/file/name",
+ self.client.getInventory,
+ "~owner/project/branch", "dir/path/file/name")
+
+ def test_getInventory_revision_not_found(self):
+ with self.mockRequests(status_code=404, reason=b"Not found"):
+ self.assertRaisesWithContent(
+ BranchFileNotFound,
+ "Branch ~owner/project/branch has no file dir/path/file/name "
+ "at revision a",
+ self.client.getInventory,
+ "~owner/project/branch", "dir/path/file/name", rev="a")
+
+ def test_getInventory_failure(self):
+ with self.mockRequests(status_code=400, reason=b"Bad request"):
+ self.assertRaisesWithContent(
+ BranchHostingFault,
+ "Failed to get inventory from Bazaar branch: "
+ "400 Client Error: Bad request",
+ self.client.getInventory,
+ "~owner/project/branch", "dir/path/file/name")
+
+ def test_getInventory_url_quoting(self):
+ with self.mockRequests(content=b'{"filelist": []}'):
+ self.client.getInventory(
+ "~owner/project/branch", "+file/ name?", rev="+rev/ id?")
+ self.assertRequest(
+ "~owner/project/branch/+json/"
+ "files/%2Brev%2F%20id%3F/%2Bfile/%20name%3F")
+
+ def test_getBlob(self):
+ blob = b"".join(chr(i) for i in range(256))
+ with self.mockRequests(content=blob):
+ response = self.client.getBlob("~owner/project/branch", "file-id")
+ self.assertEqual(blob, response)
+ self.assertRequest("~owner/project/branch/download/head%3A/file-id")
+
+ def test_getBlob_revision(self):
+ blob = b"".join(chr(i) for i in range(256))
+ with self.mockRequests(content=blob):
+ response = self.client.getBlob(
+ "~owner/project/branch", "file-id", rev="a")
+ self.assertEqual(blob, response)
+ self.assertRequest("~owner/project/branch/download/a/file-id")
+
+ def test_getBlob_not_found(self):
+ with self.mockRequests(status_code=404, reason=b"Not found"):
+ self.assertRaisesWithContent(
+ BranchFileNotFound,
+ "Branch ~owner/project/branch has no file with ID file-id",
+ self.client.getBlob, "~owner/project/branch", "file-id")
+
+ def test_getBlob_revision_not_found(self):
+ with self.mockRequests(status_code=404, reason=b"Not found"):
+ self.assertRaisesWithContent(
+ BranchFileNotFound,
+ "Branch ~owner/project/branch has no file with ID file-id "
+ "at revision a",
+ self.client.getBlob,
+ "~owner/project/branch", "file-id", rev="a")
+
+ def test_getBlob_failure(self):
+ with self.mockRequests(status_code=400, reason=b"Bad request"):
+ self.assertRaisesWithContent(
+ BranchHostingFault,
+ "Failed to get file from Bazaar branch: "
+ "400 Client Error: Bad request",
+ self.client.getBlob, "~owner/project/branch", "file-id")
+
+ def test_getBlob_url_quoting(self):
+ blob = b"".join(chr(i) for i in range(256))
+ with self.mockRequests(content=blob):
+ self.client.getBlob(
+ "~owner/project/branch", "+file/ id?", rev="+rev/ id?")
+ self.assertRequest(
+ "~owner/project/branch/"
+ "download/%2Brev%2F%20id%3F/%2Bfile%2F%20id%3F")
+
+ def test_works_in_job(self):
+ # `BranchHostingClient` is usable from a running job.
+ blob = b"".join(chr(i) for i in range(256))
+
+ @implementer(IRunnableJob)
+ class GetBlobJob(BaseRunnableJob):
+ def __init__(self, testcase):
+ super(GetBlobJob, self).__init__()
+ self.job = Job()
+ self.testcase = testcase
+
+ def run(self):
+ with self.testcase.mockRequests(
+ content=blob, set_default_timeout=False):
+ self.blob = self.testcase.client.getBlob(
+ "~owner/project/branch", "file-id")
+ # We must make this assertion inside the job, since the job
+ # runner creates a separate timeline.
+ self.testcase.assertRequest(
+ "~owner/project/branch/download/head%3A/file-id")
+
+ job = GetBlobJob(self)
+ JobRunner([job]).runAll()
+ self.assertEqual(JobStatus.COMPLETED, job.job.status)
+ self.assertEqual(blob, job.blob)
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2018-05-16 19:07:36 +0000
+++ lib/lp/services/config/schema-lazr.conf 2018-05-16 19:07:37 +0000
@@ -352,6 +352,9 @@
# of shutting down and so should not receive any more connections.
web_status_port = tcp:8022
+# The URL of the internal Bazaar hosting API endpoint.
+internal_bzr_api_endpoint: none
+
# The URL of the internal Git hosting API endpoint.
internal_git_api_endpoint: none
Follow ups
