Merge lp:~cjwatson/jenkins-launchpad-plugin/log-user into lp:jenkins-launchpad-plugin

[Colin Watson]

Log the LP username in response to Unauthorized exceptions.

Log the LP username in response to Unauthorized exceptions. This will make problems like https://mir-jenkins.ubuntu.com/job/trigger-ci/9/console easier to track down.

[Evan (ev)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'jlp/jenkinsutils.py'
--- jlp/jenkinsutils.py	2015-09-15 15:10:52 +0000
+++ jlp/jenkinsutils.py	2016-01-08 11:11:04 +0000
@@ -612,7 +612,7 @@
     params['test_result'] = "PASSED"
 
 
-def start_jenkins_job(launchpaduser, jenkins_url, jenkins_job, mp,
+def start_jenkins_job(lp_handle, launchpaduser, jenkins_url, jenkins_job, mp,
                       fasttrack_merge=False):
     """Start a given jenkins job on a jenkins as defined in config file
 
@@ -642,8 +642,12 @@
             reviewer=launchpaduser,
             review_type=get_config_option('launchpad_review_type'))
     except Unauthorized:
-        logger.info('You are not authorized to review this merge ' +
-                    'proposal. CI process will now be stopped.')
+        try:
+            username = lp_handle.me.name
+        except Unauthorized:
+            username = '<anonymous>'
+        logger.info('You (' + username + ') are not authorized to review '
+                    'this merge proposal. CI process will now be stopped.')
         return False
 
     try:
@@ -815,7 +819,8 @@
         if launchpadutils.testing_in_progress(mp, jenkins_job):
             continue
 
-        ret = start_jenkins_job(launchpad_user, jenkins_url, jenkins_job, mp)
+        ret = start_jenkins_job(lp_handle, launchpad_user, jenkins_url,
+                                jenkins_job, mp)
         result = ret and result
     return result
 
@@ -865,7 +870,7 @@
 
         launchpad_user = lp_handle.people(
             get_config_option('launchpad_login'))
-        ret = start_jenkins_job(launchpad_user, jenkins_url, jenkins_job,
-                                mp, fasttrack_merge)
+        ret = start_jenkins_job(lp_handle, launchpad_user, jenkins_url,
+                                jenkins_job, mp, fasttrack_merge)
         result = ret and result
     return result

=== modified file 'tests/test_jenkinsutils.py'
--- tests/test_jenkinsutils.py	2013-11-22 21:56:39 +0000
+++ tests/test_jenkinsutils.py	2016-01-08 11:11:04 +0000
@@ -888,6 +888,7 @@
 
 class TestStartJenkinsJob(unittest.TestCase):
     def setUp(self):
+        self.lp_handle = MagicMock()
         self.launchpad_user = MagicMock()
 
     def test_start_jenkins_job(self):
@@ -898,8 +899,8 @@
         jenkins = MagicMock()
         jenkins.Jenkins = MagicMock(return_value=j)
         with patch('jlp.jenkinsutils.jenkins', new=jenkins):
-            jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
-                                           'jenkins_job', mp)
+            jenkinsutils.start_jenkins_job(self.lp_handle, self.launchpad_user,
+                                           'url', 'jenkins_job', mp)
             self.assertEqual(j.build_job.call_count, 1)
             self.assertEqual(mp.nominateReviewer.call_count, 1)
 
@@ -913,7 +914,8 @@
         jenkins.Jenkins = MagicMock(return_value=j)
 
         with patch('jlp.jenkinsutils.jenkins', new=jenkins):
-            ret = jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
+            ret = jenkinsutils.start_jenkins_job(self.lp_handle,
+                                                 self.launchpad_user, 'url',
                                                  'jenkins_job', mp)
             self.assertFalse(ret)
             self.assertEqual(j.build_job.call_count, 0)
@@ -928,8 +930,8 @@
         jenkinsInstance.Jenkins = MagicMock(return_value=j)
         jenkinsInstance.JenkinsException = JenkinsException
         with patch('jlp.jenkinsutils.jenkins', new=jenkinsInstance):
-            jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
-                                           'jenkins_job', mp)
+            jenkinsutils.start_jenkins_job(self.lp_handle, self.launchpad_user,
+                                           'url', 'jenkins_job', mp)
             self.assertEqual(mp.createComment.call_count, 1)
 
     def test_start_jenkins_job_and_raise_unauthorized(self):
@@ -941,7 +943,8 @@
         jenkinsInstance = MagicMock()
         jenkinsInstance.Jenkins = MagicMock(return_value=j)
         with patch('jlp.jenkinsutils.jenkins', new=jenkinsInstance):
-            ret = jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
+            ret = jenkinsutils.start_jenkins_job(self.lp_handle,
+                                                 self.launchpad_user, 'url',
                                                  'jenkins_job', mp)
             self.assertEqual(mp.createComment.call_count, 0)
             self.assertEqual(ret, False)
@@ -954,7 +957,8 @@
         jenkins.Jenkins = MagicMock(return_value=j)
         with patch('jlp.jenkinsutils.jenkins', new=jenkins):
             self.assertEqual(
-                jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
+                jenkinsutils.start_jenkins_job(self.lp_handle,
+                                               self.launchpad_user, 'url',
                                                'jenkins_job', mp),
                 False)
             self.assertEqual(j.build_job.call_count, 0)
@@ -982,8 +986,8 @@
         with patch('jlp.jenkinsutils.jenkins', new=jenkins), \
                 patch('jlp.jenkinsutils.get_json_jenkins',
                       new=get_json_jenkins):
-            jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
-                                           'jenkins_job', mp, True)
+            jenkinsutils.start_jenkins_job(self.lp_handle, self.launchpad_user,
+                                           'url', 'jenkins_job', mp, True)
             self.assertEqual(j.build_job.call_count, 1)
             self.assertEqual(j.build_job.call_args[0][0], 'generic-land')
 
@@ -997,8 +1001,8 @@
         jenkins = MagicMock()
         jenkins.Jenkins = MagicMock(return_value=j)
         with patch('jlp.jenkinsutils.jenkins', new=jenkins):
-            jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
-                                           'jenkins_job', mp, True)
+            jenkinsutils.start_jenkins_job(self.lp_handle, self.launchpad_user,
+                                           'url', 'jenkins_job', mp, True)
             self.assertEqual(j.build_job.call_count, 1)
             self.assertEqual(j.build_job.call_args[0][0], 'jenkins_job')
 
@@ -1013,8 +1017,8 @@
         jenkins = MagicMock()
         jenkins.Jenkins = MagicMock(return_value=j)
         with patch('jlp.jenkinsutils.jenkins', new=jenkins):
-            jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
-                                           'jenkins_job', mp)
+            jenkinsutils.start_jenkins_job(self.lp_handle, self.launchpad_user,
+                                           'url', 'jenkins_job', mp)
             self.assertEqual(j.build_job.call_args[0][2], None)
 
     def test_start_jenkins_job_with_jenkins_build_token_set(self):
@@ -1031,8 +1035,8 @@
         _config['jenkins_build_token'] = token
         jenkins.Jenkins = MagicMock(return_value=j)
         with patch('jlp.jenkinsutils.jenkins', new=jenkins):
-            jenkinsutils.start_jenkins_job(self.launchpad_user, 'url',
-                                           'jenkins_job', mp)
+            jenkinsutils.start_jenkins_job(self.lp_handle, self.launchpad_user,
+                                           'url', 'jenkins_job', mp)
             self.assertEqual(j.build_job.call_args[0][2], token)