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
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
