launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-hosting-timeline into lp:launchpad.

Commit message:
Log Git hosting requests to the timeline.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-hosting-timeline/+merge/295137

As external actions, GitHostingClient requests should be logged to the timeline.  I went for the quickest-to-implement encoding of the request parameters, but would be happy with more or less anything reasonable there.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-hosting-timeline into lp:launchpad.

=== modified file 'lib/lp/code/model/githosting.py'
--- lib/lp/code/model/githosting.py	2016-05-13 10:58:59 +0000
+++ lib/lp/code/model/githosting.py	2016-05-19 02:12:34 +0000
@@ -8,9 +8,11 @@
     'GitHostingClient',
     ]
 
+import json
 from urllib import quote
 from urlparse import urljoin
 
+from lazr.restful.utils import get_current_browser_request
 import requests
 from zope.interface import implementer
 
@@ -21,6 +23,7 @@
     )
 from lp.code.interfaces.githosting import IGitHostingClient
 from lp.services.config import config
+from lp.services.timeline.requesttimeline import get_request_timeline
 from lp.services.timeout import urlfetch
 
 
@@ -35,13 +38,18 @@
     def __init__(self):
         self.endpoint = config.codehosting.internal_git_api_endpoint
 
-    def _request(self, method, path, json_data=None, **kwargs):
+    def _request(self, method, path, **kwargs):
+        timeline = get_request_timeline(get_current_browser_request())
+        action = timeline.start(
+            "git-hosting-%s" % method, "%s %s" % (path, json.dumps(kwargs)))
         try:
             response = urlfetch(
                 urljoin(self.endpoint, path), trust_env=False, method=method,
                 **kwargs)
         except requests.HTTPError as e:
             raise HTTPResponseNotOK(e.response.content)
+        finally:
+            action.finish()
         if response.content:
             return response.json()
         else:

=== modified file 'lib/lp/code/model/tests/test_githosting.py'
--- lib/lp/code/model/tests/test_githosting.py	2016-05-13 10:58:59 +0000
+++ lib/lp/code/model/tests/test_githosting.py	2016-05-19 02:12:34 +0000
@@ -18,6 +18,7 @@
     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
@@ -38,6 +39,7 @@
     BaseRunnableJob,
     JobRunner,
     )
+from lp.services.timeline.requesttimeline import get_request_timeline
 from lp.services.timeout import (
     get_default_timeout_function,
     set_default_timeout_function,
@@ -75,11 +77,16 @@
             finally:
                 set_default_timeout_function(original_timeout_function)
 
-    def assertRequest(self, url_suffix, json_data=None, **kwargs):
+    def assertRequest(self, url_suffix, json_data=None, method=None, **kwargs):
         self.assertThat(self.request, MatchesStructure.byEquality(
-            url=urlappend(self.endpoint, url_suffix), **kwargs))
+            url=urlappend(self.endpoint, url_suffix), method=method, **kwargs))
         if json_data is not None:
             self.assertEqual(json_data, json.loads(self.request.body))
+        timeline = get_request_timeline(get_current_browser_request())
+        action = timeline.actions[-1]
+        self.assertEqual("git-hosting-%s" % method.lower(), action.category)
+        self.assertEqual(
+            "/" + url_suffix.split("?", 1)[0], action.detail.split(" ", 1)[0])
 
     def test_create(self):
         with self.mockRequests():