← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/github-link into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/github-link into lp:launchpad.

Commit message:
Add basic GitHub bug linking.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #848666 in Launchpad itself: "Support GitHub issues as external bugtracker"
  https://bugs.launchpad.net/launchpad/+bug/848666

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/github-link/+merge/299074

Add basic GitHub bug linking.

Each repository with linked issues needs its own BugTracker row, as GitHub namespaces issue numbers per repository rather than globally.  However, our rate limit for GitHub API requests is global, and GitHub's documentation indicates that they may take punitive measures against clients that don't do a good job of backing off when they exceed their rate limit.  This necessitates a GitHubRateLimit utility to keep track of how much of our limit we have left.

Our model of remote bug state is very simple for now.  We map "open" to NEW, "closed" to FIXRELEASED, and ignore importance altogether.  However, we do at least record the remote bug's labels even though we don't currently map them to anything.

On deployment, we'll want to get sysadmins to create a bot account for us on GitHub and generate an OAuth token.  While unauthenticated access will minimally work, GitHub sets a much lower rate limit for that than for authenticated access.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/github-link into lp:launchpad.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2016-02-04 12:45:38 +0000
+++ lib/lp/bugs/configure.zcml	2016-07-04 17:24:09 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -518,6 +518,17 @@
                 interface="lp.bugs.interfaces.bugtracker.IRemoteBug"/>
         </class>
 
+        <!-- GitHubRateLimit -->
+
+        <class
+            class="lp.bugs.externalbugtracker.github.GitHubRateLimit">
+            <allow
+                interface="lp.bugs.externalbugtracker.github.IGitHubRateLimit"/>
+        </class>
+        <utility
+            factory="lp.bugs.externalbugtracker.github.GitHubRateLimit"
+            provides="lp.bugs.externalbugtracker.github.IGitHubRateLimit"/>
+
     <!-- IBugBranch -->
 
     <class

=== modified file 'lib/lp/bugs/doc/externalbugtracker-bugzilla-lp-plugin.txt'
--- lib/lp/bugs/doc/externalbugtracker-bugzilla-lp-plugin.txt	2012-12-26 01:32:19 +0000
+++ lib/lp/bugs/doc/externalbugtracker-bugzilla-lp-plugin.txt	2016-07-04 17:24:09 +0000
@@ -7,8 +7,7 @@
 For testing purposes, a custom XML-RPC transport can be passed to it,
 so that we can avoid network traffic in tests.
 
-    >>> from lp.bugs.externalbugtracker import (
-    ...     BugzillaLPPlugin)
+    >>> from lp.bugs.externalbugtracker.bugzilla import BugzillaLPPlugin
     >>> from lp.bugs.tests.externalbugtracker import (
     ...     TestBugzillaXMLRPCTransport)
     >>> test_transport = TestBugzillaXMLRPCTransport('http://example.com/')

=== modified file 'lib/lp/bugs/doc/externalbugtracker-bugzilla.txt'
--- lib/lp/bugs/doc/externalbugtracker-bugzilla.txt	2012-12-26 01:32:19 +0000
+++ lib/lp/bugs/doc/externalbugtracker-bugzilla.txt	2016-07-04 17:24:09 +0000
@@ -113,7 +113,7 @@
 
     >>> transaction.commit()
 
-    >>> from lp.bugs.externalbugtracker import (
+    >>> from lp.bugs.externalbugtracker.bugzilla import (
     ...     BugzillaAPI, BugzillaLPPlugin)
     >>> bugzilla_to_use = bugzilla.getExternalBugTrackerToUse()
 

=== modified file 'lib/lp/bugs/doc/externalbugtracker-trac-lp-plugin.txt'
--- lib/lp/bugs/doc/externalbugtracker-trac-lp-plugin.txt	2012-11-02 03:23:34 +0000
+++ lib/lp/bugs/doc/externalbugtracker-trac-lp-plugin.txt	2016-07-04 17:24:09 +0000
@@ -7,10 +7,8 @@
 For testing purposes, a custom XML-RPC transport can be passed to it,
 so that we can avoid network traffic in tests.
 
-    >>> from lp.bugs.externalbugtracker import (
-    ...     TracLPPlugin)
-    >>> from lp.bugs.tests.externalbugtracker import (
-    ...     TestTracXMLRPCTransport)
+    >>> from lp.bugs.externalbugtracker.trac import TracLPPlugin
+    >>> from lp.bugs.tests.externalbugtracker import TestTracXMLRPCTransport
     >>> test_transport = TestTracXMLRPCTransport('http://example.com/')
     >>> trac = TracLPPlugin(
     ...     'http://example.com/', xmlrpc_transport=test_transport)

=== modified file 'lib/lp/bugs/doc/externalbugtracker-trac.txt'
--- lib/lp/bugs/doc/externalbugtracker-trac.txt	2012-12-26 01:32:19 +0000
+++ lib/lp/bugs/doc/externalbugtracker-trac.txt	2016-07-04 17:24:09 +0000
@@ -41,8 +41,7 @@
     >>> chosen_trac = trac.getExternalBugTrackerToUse()
     http://example.com/launchpad-auth/check
 
-    >>> from lp.bugs.externalbugtracker import (
-    ...     TracLPPlugin)
+    >>> from lp.bugs.externalbugtracker.trac import TracLPPlugin
     >>> isinstance(chosen_trac, TracLPPlugin)
     True
     >>> chosen_trac.baseurl

=== modified file 'lib/lp/bugs/doc/externalbugtracker.txt'
--- lib/lp/bugs/doc/externalbugtracker.txt	2016-02-05 16:51:12 +0000
+++ lib/lp/bugs/doc/externalbugtracker.txt	2016-07-04 17:24:09 +0000
@@ -49,8 +49,7 @@
 instance.  Usually there is only one version, so the default for the
 original instance is to return itself.
 
-    >>> from lp.bugs.externalbugtracker import (
-    ...     ExternalBugTracker)
+    >>> from lp.bugs.externalbugtracker import ExternalBugTracker
     >>> external_bugtracker = ExternalBugTracker('http://example.com/')
     >>> chosen_bugtracker = external_bugtracker.getExternalBugTrackerToUse()
     >>> chosen_bugtracker is external_bugtracker
@@ -63,9 +62,11 @@
 (ExternalBugTracker, bug_watches) tuples.
 
     >>> from lp.bugs.externalbugtracker import (
-    ...     Bugzilla, BugzillaAPI, BUG_TRACKER_CLASSES)
-    >>> from lp.bugs.interfaces.bugtracker import (
-    ...     BugTrackerType)
+    ...     Bugzilla,
+    ...     BUG_TRACKER_CLASSES,
+    ...     )
+    >>> from lp.bugs.externalbugtracker.bugzilla import BugzillaAPI
+    >>> from lp.bugs.interfaces.bugtracker import BugTrackerType
     >>> from lp.testing.factory import LaunchpadObjectFactory
 
     >>> factory = LaunchpadObjectFactory()

=== modified file 'lib/lp/bugs/externalbugtracker/__init__.py'
--- lib/lp/bugs/externalbugtracker/__init__.py	2013-01-07 02:40:55 +0000
+++ lib/lp/bugs/externalbugtracker/__init__.py	2016-07-04 17:24:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """__init__ module for the externalbugtracker package."""
@@ -14,6 +14,7 @@
     'DebBugs',
     'DebBugsDatabaseNotFound',
     'ExternalBugTracker',
+    'GitHub',
     'InvalidBugId',
     'LookupTree',
     'Mantis',
@@ -31,20 +32,43 @@
     'get_external_bugtracker',
     ]
 
-from lp.bugs.externalbugtracker.base import *
-from lp.bugs.externalbugtracker.bugzilla import *
-from lp.bugs.externalbugtracker.debbugs import *
-from lp.bugs.externalbugtracker.mantis import *
-from lp.bugs.externalbugtracker.roundup import *
-from lp.bugs.externalbugtracker.rt import *
-from lp.bugs.externalbugtracker.sourceforge import *
-from lp.bugs.externalbugtracker.trac import *
+from lp.bugs.externalbugtracker.base import (
+    BATCH_SIZE_UNLIMITED,
+    BugNotFound,
+    BugTrackerConnectError,
+    BugWatchUpdateError,
+    BugWatchUpdateWarning,
+    ExternalBugTracker,
+    InvalidBugId,
+    LookupTree,
+    PrivateRemoteBug,
+    UnknownBugTrackerTypeError,
+    UnknownRemoteStatusError,
+    UnparsableBugData,
+    UnparsableBugTrackerVersion,
+    UnsupportedBugTrackerVersion,
+    )
+from lp.bugs.externalbugtracker.bugzilla import Bugzilla
+from lp.bugs.externalbugtracker.debbugs import (
+    DebBugs,
+    DebBugsDatabaseNotFound,
+    )
+from lp.bugs.externalbugtracker.github import GitHub
+from lp.bugs.externalbugtracker.mantis import (
+    Mantis,
+    MantisLoginHandler,
+    )
+from lp.bugs.externalbugtracker.roundup import Roundup
+from lp.bugs.externalbugtracker.rt import RequestTracker
+from lp.bugs.externalbugtracker.sourceforge import SourceForge
+from lp.bugs.externalbugtracker.trac import Trac
 from lp.bugs.interfaces.bugtracker import BugTrackerType
 
 
 BUG_TRACKER_CLASSES = {
     BugTrackerType.BUGZILLA: Bugzilla,
     BugTrackerType.DEBBUGS: DebBugs,
+    BugTrackerType.GITHUB: GitHub,
     BugTrackerType.MANTIS: Mantis,
     BugTrackerType.TRAC: Trac,
     BugTrackerType.ROUNDUP: Roundup,

=== modified file 'lib/lp/bugs/externalbugtracker/base.py'
--- lib/lp/bugs/externalbugtracker/base.py	2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/externalbugtracker/base.py	2016-07-04 17:24:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """External bugtrackers."""
@@ -14,6 +14,7 @@
     'ExternalBugTracker',
     'InvalidBugId',
     'LookupTree',
+    'LP_USER_AGENT',
     'PrivateRemoteBug',
     'UnknownBugTrackerTypeError',
     'UnknownRemoteImportanceError',
@@ -236,7 +237,7 @@
     def _getHeaders(self):
         # For some reason, bugs.kde.org doesn't allow the regular urllib
         # user-agent string (Python-urllib/2.x) to access their bugzilla.
-        return {'User-agent': LP_USER_AGENT, 'Host': self.basehost}
+        return {'User-Agent': LP_USER_AGENT, 'Host': self.basehost}
 
     def _fetchPage(self, page, data=None):
         """Fetch a page from the remote server.

=== added file 'lib/lp/bugs/externalbugtracker/github.py'
--- lib/lp/bugs/externalbugtracker/github.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/externalbugtracker/github.py	2016-07-04 17:24:09 +0000
@@ -0,0 +1,264 @@
+# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""GitHub ExternalBugTracker utility."""
+
+__metaclass__ = type
+__all__ = [
+    'BadGitHubURL',
+    'GitHub',
+    'GitHubRateLimit',
+    'IGitHubRateLimit',
+    ]
+
+import httplib
+import time
+from urllib import urlencode
+from urlparse import (
+    urljoin,
+    urlunsplit,
+    )
+
+import pytz
+import requests
+from zope.component import getUtility
+from zope.interface import Interface
+
+from lp.bugs.externalbugtracker import (
+    BugTrackerConnectError,
+    BugWatchUpdateError,
+    ExternalBugTracker,
+    UnknownRemoteStatusError,
+    UnparsableBugTrackerVersion,
+    )
+from lp.bugs.externalbugtracker.base import LP_USER_AGENT
+from lp.bugs.interfaces.bugtask import (
+    BugTaskImportance,
+    BugTaskStatus,
+    )
+from lp.bugs.interfaces.externalbugtracker import UNKNOWN_REMOTE_IMPORTANCE
+from lp.services.config import config
+from lp.services.database.isolation import ensure_no_transaction
+from lp.services.webapp.url import urlsplit
+
+
+class GitHubExceededRateLimit(BugWatchUpdateError):
+
+    def __init__(self, host, reset):
+        self.host = host
+        self.reset = reset
+
+    def __str__(self):
+        return "Rate limit for %s exceeded (resets at %s)" % (
+            self.host, time.ctime(self.reset))
+
+
+class IGitHubRateLimit(Interface):
+    """Interface for rate-limit tracking for the GitHub Issues API."""
+
+    def makeRequest(method, url, token=None, **kwargs):
+        """Make a request, but only if the remote host's rate limit permits it.
+
+        :param method: The HTTP request method.
+        :param url: The URL to request.
+        :param token: If not None, an OAuth token to use as authentication
+            to the remote host when asking it for the current rate limit.
+        :return: A `requests.Response` object.
+        :raises GitHubExceededRateLimit: if the rate limit was exceeded.
+        """
+
+    def clearCache():
+        """Forget any cached rate limits."""
+
+
+class GitHubRateLimit:
+    """Rate-limit tracking for the GitHub Issues API."""
+
+    def __init__(self):
+        self.clearCache()
+
+    def _update(self, host, token=None):
+        headers = {
+            "User-Agent": LP_USER_AGENT,
+            "Host": host,
+            "Accept": "application/vnd.github.v3+json",
+            }
+        if token is not None:
+            headers["Authorization"] = "token %s" % token
+        url = "https://%s/rate_limit"; % host
+        try:
+            response = requests.get(url, headers=headers)
+            response.raise_for_status()
+            self._limits[(host, token)] = response.json()["resources"]["core"]
+        except requests.RequestException as e:
+            raise BugTrackerConnectError(url, e)
+
+    @ensure_no_transaction
+    def makeRequest(self, method, url, token=None, **kwargs):
+        """See `IGitHubRateLimit`."""
+        host = urlsplit(url).netloc
+        if (host, token) not in self._limits:
+            self._update(host, token=token)
+        limit = self._limits[(host, token)]
+        if not limit["remaining"]:
+            raise GitHubExceededRateLimit(host, limit["reset"])
+        response = requests.request(method, url, **kwargs)
+        limit["remaining"] -= 1
+        return response
+
+    def clearCache(self):
+        """See `IGitHubRateLimit`."""
+        self._limits = {}
+
+
+class BadGitHubURL(UnparsableBugTrackerVersion):
+    """The GitHub Issues URL is malformed."""
+
+
+class GitHub(ExternalBugTracker):
+    """An `ExternalBugTracker` for dealing with GitHub issues."""
+
+    # Avoid eating through our rate limit unnecessarily.
+    batch_query_threshold = 1
+
+    def __init__(self, baseurl):
+        _, host, path, query, fragment = urlsplit(baseurl)
+        host = "api." + host
+        path = path.rstrip("/")
+        if not path.endswith("/issues"):
+            raise BadGitHubURL(baseurl)
+        path = "/repos" + path[:-len("/issues")]
+        baseurl = urlunsplit(("https", host, path, query, fragment))
+        super(GitHub, self).__init__(baseurl)
+        self.cached_bugs = {}
+
+    @property
+    def credentials(self):
+        credentials_config = config["checkwatches.credentials"]
+        # lazr.config.Section doesn't support get().
+        try:
+            token = credentials_config["%s.token" % self.basehost]
+        except KeyError:
+            token = None
+        return {"token": token}
+
+    def getModifiedRemoteBugs(self, bug_ids, last_accessed):
+        """See `IExternalBugTracker`."""
+        modified_bugs = self.getRemoteBugBatch(
+            bug_ids, last_accessed=last_accessed)
+        self.cached_bugs.update(modified_bugs)
+        return list(modified_bugs)
+
+    def getRemoteBug(self, bug_id):
+        """See `ExternalBugTracker`."""
+        bug_id = int(bug_id)
+        if bug_id not in self.cached_bugs:
+            self.cached_bugs[bug_id] = (
+                self._getPage("issues/%s" % bug_id).json())
+        return bug_id, self.cached_bugs[bug_id]
+
+    def getRemoteBugBatch(self, bug_ids, last_accessed=None):
+        """See `ExternalBugTracker`."""
+        # The GitHub API does not support exporting only a subset of bug IDs
+        # as a batch.  As a result, our caching is only effective if we have
+        # cached *all* the requested bug IDs; this is the case when we're
+        # being called on the result of getModifiedRemoteBugs, so it's still
+        # a useful optimisation.
+        bug_ids = [int(bug_id) for bug_id in bug_ids]
+        bugs = {
+            bug_id: self.cached_bugs[bug_id]
+            for bug_id in bug_ids if bug_id in self.cached_bugs}
+        if set(bugs) == set(bug_ids):
+            return bugs
+        params = [("state", "all")]
+        if last_accessed is not None:
+            since = last_accessed.astimezone(pytz.UTC).strftime(
+                "%Y-%m-%dT%H:%M:%SZ")
+            params.append(("since", since))
+        page = "issues?%s" % urlencode(params)
+        for remote_bug in self._getCollection(page):
+            # We're only interested in the bug if it's one of the ones in
+            # bug_ids.
+            if remote_bug["id"] not in bug_ids:
+                continue
+            bugs[remote_bug["id"]] = remote_bug
+            self.cached_bugs[remote_bug["id"]] = remote_bug
+        return bugs
+
+    def getRemoteImportance(self, bug_id):
+        """See `ExternalBugTracker`."""
+        return UNKNOWN_REMOTE_IMPORTANCE
+
+    def getRemoteStatus(self, bug_id):
+        """See `ExternalBugTracker`."""
+        remote_bug = self.bugs[int(bug_id)]
+        state = remote_bug["state"]
+        labels = [label["name"] for label in remote_bug["labels"]]
+        return " ".join([state] + labels)
+
+    def convertRemoteImportance(self, remote_importance):
+        """See `IExternalBugTracker`."""
+        return BugTaskImportance.UNKNOWN
+
+    def convertRemoteStatus(self, remote_status):
+        """See `IExternalBugTracker`.
+
+        A GitHub status consists of the state followed by optional labels.
+        """
+        state = remote_status.split(" ", 1)[0]
+        if state == "open":
+            return BugTaskStatus.NEW
+        elif state == "closed":
+            return BugTaskStatus.FIXRELEASED
+        else:
+            raise UnknownRemoteStatusError(remote_status)
+
+    def _getHeaders(self, last_accessed=None):
+        """See `ExternalBugTracker`."""
+        headers = super(GitHub, self)._getHeaders()
+        token = self.credentials["token"]
+        if token is not None:
+            headers["Authorization"] = "token %s" % token
+        headers["Accept"] = "application/vnd.github.v3+json"
+        if last_accessed is not None:
+            headers["If-Modified-Since"] = (
+                last_accessed.astimezone(pytz.UTC).strftime(
+                    "%a, %d %b %Y %H:%M:%S GMT"))
+        return headers
+
+    def _getPage(self, page, last_accessed=None):
+        """See `ExternalBugTracker`."""
+        # We prefer to use requests here because it knows how to parse Link
+        # headers.  Note that this returns a `requests.Response`, not the
+        # page data.
+        try:
+            response = getUtility(IGitHubRateLimit).makeRequest(
+                "GET", urljoin(self.baseurl + "/", page),
+                headers=self._getHeaders(last_accessed=last_accessed))
+            response.raise_for_status()
+            return response
+        except requests.RequestException as e:
+            raise BugTrackerConnectError(self.baseurl, e)
+
+    def _getCollection(self, base_page, last_accessed=None):
+        """Yield each item from a batched remote collection.
+
+        If the collection has not been modified since `last_accessed`, yield
+        no items.
+        """
+        page = base_page
+        while page is not None:
+            try:
+                response = self._getPage(page, last_accessed=last_accessed)
+            except BugTrackerConnectError as e:
+                if (e.response is not None and
+                        e.response.status_code == httplib.NOT_MODIFIED):
+                    return
+                else:
+                    raise
+            for item in response.json():
+                yield item
+            if "next" in response.links:
+                page = response.links["next"]["url"]
+            else:
+                page = None

=== added file 'lib/lp/bugs/externalbugtracker/tests/test_github.py'
--- lib/lp/bugs/externalbugtracker/tests/test_github.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/externalbugtracker/tests/test_github.py	2016-07-04 17:24:09 +0000
@@ -0,0 +1,375 @@
+# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the GitHub Issues BugTracker."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from datetime import datetime
+import json
+from urlparse import (
+    parse_qs,
+    urlunsplit,
+    )
+
+from httmock import (
+    HTTMock,
+    urlmatch,
+    )
+import pytz
+import transaction
+from zope.component import getUtility
+
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.bugs.externalbugtracker import (
+    BugTrackerConnectError,
+    get_external_bugtracker,
+    )
+from lp.bugs.externalbugtracker.github import (
+    BadGitHubURL,
+    GitHub,
+    GitHubExceededRateLimit,
+    IGitHubRateLimit,
+    )
+from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.bugs.interfaces.bugtracker import BugTrackerType
+from lp.bugs.interfaces.externalbugtracker import IExternalBugTracker
+from lp.bugs.scripts.checkwatches import CheckwatchesMaster
+from lp.services.log.logger import BufferLogger
+from lp.testing import (
+    TestCase,
+    TestCaseWithFactory,
+    verifyObject,
+    )
+from lp.testing.layers import (
+    ZopelessDatabaseLayer,
+    ZopelessLayer,
+    )
+
+
+class TestGitHubRateLimit(TestCase):
+
+    layer = ZopelessLayer
+
+    def setUp(self):
+        super(TestGitHubRateLimit, self).setUp()
+        self.rate_limit = getUtility(IGitHubRateLimit)
+        self.addCleanup(self.rate_limit.clearCache)
+
+    @urlmatch(path=r"^/rate_limit$")
+    def _rate_limit_handler(self, url, request):
+        self.rate_limit_request = request
+        self.rate_limit_headers = request.headers
+        return {
+            "status_code": 200,
+            "content": {"resources": {"core": self.initial_rate_limit}},
+            }
+
+    @urlmatch(path=r"^/$")
+    def _target_handler(self, url, request):
+        self.target_request = request
+        return {"status_code": 200, "content": b"test"}
+
+    def test_makeRequest_no_token(self):
+        self.initial_rate_limit = {
+            "limit": 60, "remaining": 50, "reset": 1000000000}
+        with HTTMock(self._rate_limit_handler, self._target_handler):
+            response = self.rate_limit.makeRequest(
+                "GET", "http://example.org/";)
+        self.assertNotIn("Authorization", self.rate_limit_headers)
+        self.assertEqual(b"test", response.content)
+        limit = self.rate_limit._limits[("example.org", None)]
+        self.assertEqual(49, limit["remaining"])
+        self.assertEqual(1000000000, limit["reset"])
+
+        limit["remaining"] = 0
+        self.rate_limit_request = None
+        with HTTMock(self._rate_limit_handler, self._target_handler):
+            self.assertRaisesWithContent(
+                GitHubExceededRateLimit,
+                "Rate limit for example.org exceeded "
+                "(resets at Sun Sep  9 07:16:40 2001)",
+                self.rate_limit.makeRequest,
+                "GET", "http://example.org/";)
+        self.assertIsNone(self.rate_limit_request)
+        self.assertEqual(0, limit["remaining"])
+
+    def test_makeRequest_check_token(self):
+        self.initial_rate_limit = {
+            "limit": 5000, "remaining": 4000, "reset": 1000000000}
+        with HTTMock(self._rate_limit_handler, self._target_handler):
+            response = self.rate_limit.makeRequest(
+                "GET", "http://example.org/";, token="abc")
+        self.assertEqual("token abc", self.rate_limit_headers["Authorization"])
+        self.assertEqual(b"test", response.content)
+        limit = self.rate_limit._limits[("example.org", "abc")]
+        self.assertEqual(3999, limit["remaining"])
+        self.assertEqual(1000000000, limit["reset"])
+
+        limit["remaining"] = 0
+        self.rate_limit_request = None
+        with HTTMock(self._rate_limit_handler, self._target_handler):
+            self.assertRaisesWithContent(
+                GitHubExceededRateLimit,
+                "Rate limit for example.org exceeded "
+                "(resets at Sun Sep  9 07:16:40 2001)",
+                self.rate_limit.makeRequest,
+                "GET", "http://example.org/";, token="abc")
+        self.assertIsNone(self.rate_limit_request)
+        self.assertEqual(0, limit["remaining"])
+
+    def test_makeRequest_check_503(self):
+        @urlmatch(path=r"^/rate_limit$")
+        def rate_limit_handler(url, request):
+            return {"status_code": 503}
+
+        with HTTMock(rate_limit_handler):
+            self.assertRaises(
+                BugTrackerConnectError, self.rate_limit.makeRequest,
+                "GET", "http://example.org/";)
+
+
+class TestGitHub(TestCase):
+
+    layer = ZopelessLayer
+
+    def setUp(self):
+        super(TestGitHub, self).setUp()
+        self.addCleanup(getUtility(IGitHubRateLimit).clearCache)
+        self.sample_bugs = [
+            {"id": 1, "state": "open", "labels": []},
+            {"id": 2, "state": "open", "labels": [{"name": "feature"}]},
+            {"id": 3, "state": "open",
+             "labels": [{"name": "feature"}, {"name": "ui"}]},
+            {"id": 4, "state": "closed", "labels": []},
+            {"id": 5, "state": "closed", "labels": [{"name": "feature"}]},
+            ]
+
+    def test_implements_interface(self):
+        self.assertTrue(verifyObject(
+            IExternalBugTracker,
+            GitHub("https://github.com/user/repository/issues";)))
+
+    def test_requires_issues_url(self):
+        self.assertRaises(
+            BadGitHubURL, GitHub, "https://github.com/user/repository";)
+
+    @urlmatch(path=r"^/rate_limit$")
+    def _rate_limit_handler(self, url, request):
+        self.rate_limit_request = request
+        rate_limit = {"limit": 5000, "remaining": 4000, "reset": 1000000000}
+        return {
+            "status_code": 200,
+            "content": {"resources": {"core": rate_limit}},
+            }
+
+    def test_getRemoteBug(self):
+        @urlmatch(path=r".*/issues/1$")
+        def handler(url, request):
+            self.request = request
+            return {"status_code": 200, "content": self.sample_bugs[0]}
+
+        tracker = GitHub("https://github.com/user/repository/issues";)
+        with HTTMock(self._rate_limit_handler, handler):
+            self.assertEqual(
+                (1, self.sample_bugs[0]), tracker.getRemoteBug("1"))
+        self.assertEqual(
+            "https://api.github.com/repos/user/repository/issues/1";,
+            self.request.url)
+
+    @urlmatch(path=r".*/issues$")
+    def _issues_handler(self, url, request):
+        self.issues_request = request
+        return {"status_code": 200, "content": json.dumps(self.sample_bugs)}
+
+    def test_getRemoteBugBatch(self):
+        tracker = GitHub("https://github.com/user/repository/issues";)
+        with HTTMock(self._rate_limit_handler, self._issues_handler):
+            self.assertEqual(
+                {bug["id"]: bug for bug in self.sample_bugs[:2]},
+                tracker.getRemoteBugBatch(["1", "2"]))
+        self.assertEqual(
+            "https://api.github.com/repos/user/repository/issues?state=all";,
+            self.issues_request.url)
+
+    def test_getRemoteBugBatch_last_accessed(self):
+        tracker = GitHub("https://github.com/user/repository/issues";)
+        since = datetime(2015, 1, 1, 12, 0, 0, tzinfo=pytz.UTC)
+        with HTTMock(self._rate_limit_handler, self._issues_handler):
+            self.assertEqual(
+                {bug["id"]: bug for bug in self.sample_bugs[:2]},
+                tracker.getRemoteBugBatch(["1", "2"], last_accessed=since))
+        self.assertEqual(
+            "https://api.github.com/repos/user/repository/issues?";
+            "state=all&since=2015-01-01T12%3A00%3A00Z",
+            self.issues_request.url)
+
+    def test_getRemoteBugBatch_caching(self):
+        tracker = GitHub("https://github.com/user/repository/issues";)
+        with HTTMock(self._rate_limit_handler, self._issues_handler):
+            tracker.initializeRemoteBugDB(
+                [str(bug["id"]) for bug in self.sample_bugs])
+            self.issues_request = None
+            self.assertEqual(
+                {bug["id"]: bug for bug in self.sample_bugs[:2]},
+                tracker.getRemoteBugBatch(["1", "2"]))
+            self.assertIsNone(self.issues_request)
+
+    def test_getRemoteBugBatch_pagination(self):
+        @urlmatch(path=r".*/issues")
+        def handler(url, request):
+            self.issues_requests.append(request)
+            base_url = urlunsplit(list(url[:3]) + ["", ""])
+            page = int(parse_qs(url.query).get("page", ["1"])[0])
+            links = []
+            if page != 3:
+                links.append('<%s?page=%d>; rel="next"' % (base_url, page + 1))
+                links.append('<%s?page=3>; rel="last"' % base_url)
+            if page != 1:
+                links.append('<%s?page=1>; rel="first"' % base_url)
+                links.append('<%s?page=%d>; rel="prev"' % (base_url, page - 1))
+            start = (page - 1) * 2
+            end = page * 2
+            return {
+                "status_code": 200,
+                "headers": {"Link": ", ".join(links)},
+                "content": json.dumps(self.sample_bugs[start:end]),
+                }
+
+        self.issues_requests = []
+        tracker = GitHub("https://github.com/user/repository/issues";)
+        with HTTMock(self._rate_limit_handler, handler):
+            self.assertEqual(
+                {bug["id"]: bug for bug in self.sample_bugs},
+                tracker.getRemoteBugBatch(
+                    [str(bug["id"]) for bug in self.sample_bugs]))
+        expected_urls = [
+            "https://api.github.com/repos/user/repository/issues?state=all";,
+            "https://api.github.com/repos/user/repository/issues?page=2";,
+            "https://api.github.com/repos/user/repository/issues?page=3";,
+            ]
+        self.assertEqual(
+            expected_urls, [request.url for request in self.issues_requests])
+
+    def test_status_open(self):
+        self.sample_bugs = [
+            {"id": 1, "state": "open", "labels": []},
+            # Labels do not affect status, even if names collide.
+            {"id": 2, "state": "open",
+             "labels": [{"name": "feature"}, {"name": "closed"}]},
+            ]
+        tracker = GitHub("https://github.com/user/repository/issues";)
+        with HTTMock(self._rate_limit_handler, self._issues_handler):
+            tracker.initializeRemoteBugDB(["1", "2"])
+        remote_status = tracker.getRemoteStatus("1")
+        self.assertEqual("open", remote_status)
+        lp_status = tracker.convertRemoteStatus(remote_status)
+        self.assertEqual(BugTaskStatus.NEW, lp_status)
+        remote_status = tracker.getRemoteStatus("2")
+        self.assertEqual("open feature closed", remote_status)
+        lp_status = tracker.convertRemoteStatus(remote_status)
+        self.assertEqual(BugTaskStatus.NEW, lp_status)
+
+    def test_status_closed(self):
+        self.sample_bugs = [
+            {"id": 1, "state": "closed", "labels": []},
+            # Labels do not affect status, even if names collide.
+            {"id": 2, "state": "closed",
+             "labels": [{"name": "feature"}, {"name": "open"}]},
+            ]
+        tracker = GitHub("https://github.com/user/repository/issues";)
+        with HTTMock(self._rate_limit_handler, self._issues_handler):
+            tracker.initializeRemoteBugDB(["1", "2"])
+        remote_status = tracker.getRemoteStatus("1")
+        self.assertEqual("closed", remote_status)
+        lp_status = tracker.convertRemoteStatus(remote_status)
+        self.assertEqual(BugTaskStatus.FIXRELEASED, lp_status)
+        remote_status = tracker.getRemoteStatus("2")
+        self.assertEqual("closed feature open", remote_status)
+        lp_status = tracker.convertRemoteStatus(remote_status)
+        self.assertEqual(BugTaskStatus.FIXRELEASED, lp_status)
+
+
+class TestGitHubUpdateBugWatches(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    @urlmatch(path=r"^/rate_limit$")
+    def _rate_limit_handler(self, url, request):
+        self.rate_limit_request = request
+        rate_limit = {"limit": 5000, "remaining": 4000, "reset": 1000000000}
+        return {
+            "status_code": 200,
+            "content": {"resources": {"core": rate_limit}},
+            }
+
+    def test_process_one(self):
+        remote_bug = {"id": 1234, "state": "open", "labels": []}
+
+        @urlmatch(path=r".*/issues/1234$")
+        def handler(url, request):
+            return {"status_code": 200, "content": remote_bug}
+
+        bug = self.factory.makeBug()
+        bug_tracker = self.factory.makeBugTracker(
+            base_url="https://github.com/user/repository/issues";,
+            bugtrackertype=BugTrackerType.GITHUB)
+        bug.addWatch(
+            bug_tracker, "1234", getUtility(ILaunchpadCelebrities).janitor)
+        self.assertEqual(
+            [("1234", None)],
+            [(watch.remotebug, watch.remotestatus)
+             for watch in bug_tracker.watches])
+        transaction.commit()
+        logger = BufferLogger()
+        bug_watch_updater = CheckwatchesMaster(transaction, logger=logger)
+        github = get_external_bugtracker(bug_tracker)
+        with HTTMock(self._rate_limit_handler, handler):
+            bug_watch_updater.updateBugWatches(github, bug_tracker.watches)
+        self.assertEqual(
+            "INFO Updating 1 watches for 1 bugs on "
+            "https://api.github.com/repos/user/repository\n";,
+            logger.getLogBuffer())
+        self.assertEqual(
+            [("1234", BugTaskStatus.NEW)],
+            [(watch.remotebug, github.convertRemoteStatus(watch.remotestatus))
+             for watch in bug_tracker.watches])
+
+    def test_process_many(self):
+        remote_bugs = [
+            {"id": bug_id,
+             "state": "open" if (bug_id % 2) == 0 else "closed",
+             "labels": []}
+            for bug_id in range(1000, 1010)]
+
+        @urlmatch(path=r".*/issues$")
+        def handler(url, request):
+            return {"status_code": 200, "content": json.dumps(remote_bugs)}
+
+        bug = self.factory.makeBug()
+        bug_tracker = self.factory.makeBugTracker(
+            base_url="https://github.com/user/repository/issues";,
+            bugtrackertype=BugTrackerType.GITHUB)
+        for remote_bug in remote_bugs:
+            bug.addWatch(
+                bug_tracker, str(remote_bug["id"]),
+                getUtility(ILaunchpadCelebrities).janitor)
+        transaction.commit()
+        logger = BufferLogger()
+        bug_watch_updater = CheckwatchesMaster(transaction, logger=logger)
+        github = get_external_bugtracker(bug_tracker)
+        with HTTMock(self._rate_limit_handler, handler):
+            bug_watch_updater.updateBugWatches(github, bug_tracker.watches)
+        self.assertEqual(
+            "INFO Updating 10 watches for 10 bugs on "
+            "https://api.github.com/repos/user/repository\n";,
+            logger.getLogBuffer())
+        self.assertContentEqual(
+            [(str(bug_id), BugTaskStatus.NEW)
+             for bug_id in (1000, 1002, 1004, 1006, 1008)] +
+            [(str(bug_id), BugTaskStatus.FIXRELEASED)
+             for bug_id in (1001, 1003, 1005, 1007, 1009)],
+            [(watch.remotebug, github.convertRemoteStatus(watch.remotestatus))
+             for watch in bug_tracker.watches])

=== modified file 'lib/lp/bugs/interfaces/bugtracker.py'
--- lib/lp/bugs/interfaces/bugtracker.py	2013-05-02 18:55:32 +0000
+++ lib/lp/bugs/interfaces/bugtracker.py	2016-07-04 17:24:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug tracker interfaces."""
@@ -186,6 +186,12 @@
         Google.
         """)
 
+    GITHUB = DBItem(12, """
+        GitHub Issues
+
+        The issue tracker for projects hosted on GitHub.
+        """)
+
 
 # A list of the BugTrackerTypes that don't need a remote product to be
 # able to return a bug filing URL. We use a whitelist rather than a
@@ -193,6 +199,7 @@
 # a remote product is required. This saves us from presenting
 # embarrassingly useless URLs to users.
 SINGLE_PRODUCT_BUGTRACKERTYPES = [
+    BugTrackerType.GITHUB,
     BugTrackerType.GOOGLE_CODE,
     BugTrackerType.MANTIS,
     BugTrackerType.PHPPROJECT,

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/model/bugtracker.py	2016-07-04 17:24:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -331,6 +331,8 @@
         BugTrackerType.BUGZILLA: (
             "%(base_url)s/enter_bug.cgi?product=%(remote_product)s"
             "&short_desc=%(summary)s&long_desc=%(description)s"),
+        BugTrackerType.GITHUB: (
+            "%(base_url)s/new?title=%(summary)s&body=%(description)s"),
         BugTrackerType.GOOGLE_CODE: (
             "%(base_url)s/entry?summary=%(summary)s&"
             "comment=%(description)s"),
@@ -360,6 +362,9 @@
         BugTrackerType.BUGZILLA: (
             "%(base_url)s/query.cgi?product=%(remote_product)s"
             "&short_desc=%(summary)s"),
+        BugTrackerType.GITHUB: (
+            "%(base_url)s?utf8=%%E2%%9C%%93"
+            "&q=is%%3Aissue%%20is%%3Aopen%%20%(summary)s"),
         BugTrackerType.GOOGLE_CODE: "%(base_url)s/list?q=%(summary)s",
         BugTrackerType.DEBBUGS: (
             "%(base_url)s/cgi-bin/search.cgi?phrase=%(summary)s"

=== modified file 'lib/lp/bugs/model/bugwatch.py'
--- lib/lp/bugs/model/bugwatch.py	2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/model/bugwatch.py	2016-07-04 17:24:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -81,6 +81,7 @@
 BUG_TRACKER_URL_FORMATS = {
     BugTrackerType.BUGZILLA: 'show_bug.cgi?id=%s',
     BugTrackerType.DEBBUGS: 'cgi-bin/bugreport.cgi?bug=%s',
+    BugTrackerType.GITHUB: '%s',
     BugTrackerType.GOOGLE_CODE: 'detail?id=%s',
     BugTrackerType.MANTIS: 'view.php?id=%s',
     BugTrackerType.ROUNDUP: 'issue%s',
@@ -388,6 +389,7 @@
             BugTrackerType.BUGZILLA: self.parseBugzillaURL,
             BugTrackerType.DEBBUGS: self.parseDebbugsURL,
             BugTrackerType.EMAILADDRESS: self.parseEmailAddressURL,
+            BugTrackerType.GITHUB: self.parseGitHubURL,
             BugTrackerType.GOOGLE_CODE: self.parseGoogleCodeURL,
             BugTrackerType.MANTIS: self.parseMantisURL,
             BugTrackerType.PHPPROJECT: self.parsePHPProjectURL,
@@ -688,6 +690,18 @@
         base_url = urlunsplit((scheme, host, tracker_path, '', ''))
         return base_url, remote_bug
 
+    def parseGitHubURL(self, scheme, host, path, query):
+        """Extract a GitHub Issues base URL and bug ID."""
+        if host != 'github.com':
+            return None
+        match = re.match(r'(.*/issues)/(\d+)$', path)
+        if not match:
+            return None
+        base_path = match.group(1)
+        remote_bug = match.group(2)
+        base_url = urlunsplit((scheme, host, base_path, '', ''))
+        return base_url, remote_bug
+
     def extractBugTrackerAndBug(self, url):
         """See `IBugWatchSet`."""
         for trackertype, parse_func in (

=== modified file 'lib/lp/bugs/stories/bugtracker/xx-bugtracker.txt'
--- lib/lp/bugs/stories/bugtracker/xx-bugtracker.txt	2015-06-27 04:10:49 +0000
+++ lib/lp/bugs/stories/bugtracker/xx-bugtracker.txt	2016-07-04 17:24:09 +0000
@@ -40,6 +40,7 @@
     Savane
     PHP Project Bugtracker
     Google Code
+    GitHub Issues
 
 The bug tracker name is used in URLs and certain characters (like '!')
 aren't allowed.

=== modified file 'lib/lp/bugs/tests/test_bugwatch.py'
--- lib/lp/bugs/tests/test_bugwatch.py	2015-10-15 14:09:50 +0000
+++ lib/lp/bugs/tests/test_bugwatch.py	2016-07-04 17:24:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BugWatchSet."""
@@ -375,6 +375,17 @@
     bug_id = '12345'
 
 
+class GitHubBugTrackerExtractBugTrackerAndBugTest(
+    ExtractBugTrackerAndBugTestBase, unittest.TestCase):
+    """Ensure BugWatchSet.extractBugTrackerAndBug works for GitHub Issues URLs.
+    """
+
+    bugtracker_type = BugTrackerType.GITHUB
+    bug_url = 'https://github.com/user/repository/issues/12345'
+    base_url = 'https://github.com/user/repository/issues'
+    bug_id = '12345'
+
+
 class TestBugWatch(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer


Follow ups