External providers: better logging for GitLab (#7385)

* External providers: better logging for GitLab

Ref #7370

Author: stsewd

readthedocs/oauth/services/github.py

@@ -6,9 +6,8 @@
 
 from allauth.socialaccount.models import SocialToken
 from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter
-
-from django.db.models import Q
 from django.conf import settings
+from django.db.models import Q
 from django.urls import reverse
 from requests.exceptions import RequestException
 
@@ -23,7 +22,6 @@
 from ..models import RemoteOrganization, RemoteRepository
 from .base import Service, SyncServiceError
 
-
 log = logging.getLogger(__name__)
 
 
@@ -465,7 +463,7 @@ def send_build_status(self, build, commit, state):
             if resp.status_code == 201:
                 log.info(
                     "GitHub commit status created for project: %s, commit status: %s",
-                    project,
+                    project.slug,
                     github_build_state,
                 )
                 return True
@@ -474,16 +472,16 @@ def send_build_status(self, build, commit, state):
                 log.info(
                     'GitHub project does not exist or user does not have '
                     'permissions: project=%s, user=%s, status=%s, url=%s',
-                    project,
-                    self.user,
+                    project.slug,
+                    self.user.username,
                     resp.status_code,
                     statuses_url,
                 )
                 return False
 
             log.warning(
                 'Unknown GitHub status API response: project=%s, user=%s, status_code=%s',
-                project,
+                project.slug,
                 self.user,
                 resp.status_code
             )
@@ -493,7 +491,7 @@ def send_build_status(self, build, commit, state):
         except (RequestException, ValueError):
             log.exception(
                 'GitHub commit status creation failed for project: %s',
-                project,
+                project.slug,
             )
             # Response data should always be JSON, still try to log if not
             # though

readthedocs/oauth/services/gitlab.py

@@ -3,29 +3,24 @@
 import json
 import logging
 import re
+from urllib.parse import urljoin, urlparse
 
 from allauth.socialaccount.providers.gitlab.views import GitLabOAuth2Adapter
 from django.conf import settings
 from django.urls import reverse
 from requests.exceptions import RequestException
 
+from readthedocs.builds import utils as build_utils
 from readthedocs.builds.constants import (
     BUILD_STATUS_SUCCESS,
     SELECT_BUILD_STATUS,
 )
-from readthedocs.builds import utils as build_utils
 from readthedocs.integrations.models import Integration
 from readthedocs.projects.models import Project
 
 from ..models import RemoteOrganization, RemoteRepository
 from .base import Service, SyncServiceError
 
-
-try:
-    from urlparse import urljoin, urlparse
-except ImportError:
-    from urllib.parse import urljoin, urlparse  # noqa
-
 log = logging.getLogger(__name__)
 
 
@@ -530,25 +525,26 @@ def send_build_status(self, build, commit, state):
         url = self.adapter.provider_base_url
 
         try:
+            statuses_url = f'{url}/api/v4/projects/{repo_id}/statuses/{commit}'
             resp = session.post(
-                f'{url}/api/v4/projects/{repo_id}/statuses/{commit}',
+                statuses_url,
                 data=json.dumps(data),
                 headers={'content-type': 'application/json'},
             )
 
             if resp.status_code == 201:
                 log.info(
                     "GitLab commit status created for project: %s, commit status: %s",
-                    project,
+                    project.slug,
                     gitlab_build_state,
                 )
                 return True
 
             if resp.status_code in [401, 403, 404]:
                 log.info(
-                    'GitLab project does not exist or user does not have '
-                    'permissions: project=%s',
-                    project,
+                    'GitLab project does not exist or user does not have permissions: '
+                    'project=%s, user=%s, status=%s, url=%s',
+                    project.slug, self.user.username, resp.status_code, statuses_url,
                 )
                 return False
 
@@ -558,7 +554,7 @@ def send_build_status(self, build, commit, state):
         except (RequestException, ValueError):
             log.exception(
                 'GitLab commit status creation failed for project: %s',
-                project,
+                project.slug,
             )
             # Response data should always be JSON, still try to log if not
             # though

readthedocs/projects/tasks.py

@@ -1941,7 +1941,6 @@ def send_build_status(build_pk, commit, status):
             user_accounts = service_class.for_user(user)
             # Try to loop through users all social accounts to send a successful request
             for account in user_accounts:
-                # Currently we only support GitHub Status API
                 if account.provider_name == provider_name:
                     success = account.send_build_status(build, commit, status)
                     if success:

readthedocs/rtd_tests/tests/test_oauth.py

@@ -1,19 +1,18 @@
-# -*- coding: utf-8 -*-
 from unittest import mock
+
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.test import TestCase
 from django.test.utils import override_settings
 from django.urls import reverse
-
 from django_dynamic_fixture import get
 
-from readthedocs.builds.constants import EXTERNAL, BUILD_STATUS_SUCCESS
-from readthedocs.builds.models import Version, Build
+from readthedocs.builds.constants import BUILD_STATUS_SUCCESS, EXTERNAL
+from readthedocs.builds.models import Build, Version
 from readthedocs.integrations.models import (
+    BitbucketWebhook,
     GitHubWebhook,
     GitLabWebhook,
-    BitbucketWebhook
 )
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
 from readthedocs.oauth.services import (
@@ -184,7 +183,7 @@ def test_send_build_status_successful(self, session, mock_logger):
         self.assertTrue(success)
         mock_logger.info.assert_called_with(
             "GitHub commit status created for project: %s, commit status: %s",
-            self.project,
+            self.project.slug,
             BUILD_STATUS_SUCCESS
         )
 
@@ -202,8 +201,8 @@ def test_send_build_status_404_error(self, session, mock_logger):
         mock_logger.info.assert_called_with(
             'GitHub project does not exist or user does not have '
             'permissions: project=%s, user=%s, status=%s, url=%s',
-            self.project,
-            self.user,
+            self.project.slug,
+            self.user.username,
             404,
             'https://api.github.com/repos/pypa/pip/statuses/1234'
         )
@@ -219,7 +218,7 @@ def test_send_build_status_value_error(self, session, mock_logger):
         self.assertFalse(success)
         mock_logger.exception.assert_called_with(
             'GitHub commit status creation failed for project: %s',
-            self.project,
+            self.project.slug,
         )
 
     @override_settings(DEFAULT_PRIVACY_LEVEL='private')
@@ -927,7 +926,7 @@ def setUp(self):
         self.service = GitLabService(user=self.user, account=None)
         self.external_version = get(Version, project=self.project, type=EXTERNAL)
         self.external_build = get(
-            Build, project=self.project, version=self.external_version
+            Build, project=self.project, version=self.external_version, commit=1234,
         )
         self.integration = get(
             GitLabWebhook,
@@ -1031,7 +1030,7 @@ def test_send_build_status_successful(self, repo_id, session, mock_logger):
         self.assertTrue(success)
         mock_logger.info.assert_called_with(
             "GitLab commit status created for project: %s, commit status: %s",
-            self.project,
+            self.project.slug,
             BUILD_STATUS_SUCCESS
         )
 
@@ -1040,7 +1039,7 @@ def test_send_build_status_successful(self, repo_id, session, mock_logger):
     @mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id')
     def test_send_build_status_404_error(self, repo_id, session, mock_logger):
         session().post.return_value.status_code = 404
-        repo_id().return_value = '9999'
+        repo_id.return_value = '9999'
 
         success = self.service.send_build_status(
             self.external_build,
@@ -1050,9 +1049,12 @@ def test_send_build_status_404_error(self, repo_id, session, mock_logger):
 
         self.assertFalse(success)
         mock_logger.info.assert_called_with(
-            'GitLab project does not exist or user does not have '
-            'permissions: project=%s',
-            self.project
+            'GitLab project does not exist or user does not have permissions: '
+            'project=%s, user=%s, status=%s, url=%s',
+            self.project.slug,
+            self.user.username,
+            404,
+            'https://gitlab.com/api/v4/projects/9999/statuses/1234',
         )
 
     @mock.patch('readthedocs.oauth.services.gitlab.log')
@@ -1069,7 +1071,7 @@ def test_send_build_status_value_error(self, repo_id, session, mock_logger):
         self.assertFalse(success)
         mock_logger.exception.assert_called_with(
             'GitLab commit status creation failed for project: %s',
-            self.project,
+            self.project.slug,
         )
 
     @mock.patch('readthedocs.oauth.services.gitlab.log')