Skip to content

GitLab Build Status Reporting for PR Builder #6076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions readthedocs/builds/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,26 @@
GITHUB_BUILD_STATUS_PENDING = 'pending'
GITHUB_BUILD_STATUS_SUCCESS = 'success'

# GitLab Build Statuses
GITLAB_BUILD_STATUS_FAILURE = 'failed'
GITLAB_BUILD_STATUS_PENDING = 'pending'
GITLAB_BUILD_STATUS_SUCCESS = 'success'

# Used to select correct Build status and description to be sent to each service API
SELECT_BUILD_STATUS = {
BUILD_STATUS_FAILURE: {
'github': GITHUB_BUILD_STATUS_FAILURE,
'gitlab': GITLAB_BUILD_STATUS_FAILURE,
'description': 'Read the Docs build failed!',
},
BUILD_STATUS_PENDING: {
'github': GITHUB_BUILD_STATUS_PENDING,
'gitlab': GITLAB_BUILD_STATUS_PENDING,
'description': 'Read the Docs build is in progress!',
},
BUILD_STATUS_SUCCESS: {
'github': GITHUB_BUILD_STATUS_SUCCESS,
'gitlab': GITLAB_BUILD_STATUS_SUCCESS,
'description': 'Read the Docs build succeeded!',
},
}
Expand Down
92 changes: 92 additions & 0 deletions readthedocs/oauth/services/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
from django.urls import reverse
from requests.exceptions import RequestException

from readthedocs.builds.constants import (
BUILD_STATUS_SUCCESS,
RTD_BUILD_STATUS_API_NAME,
SELECT_BUILD_STATUS,
)
from readthedocs.builds.utils import get_gitlab_username_repo
from readthedocs.integrations.models import Integration
from readthedocs.projects.models import Project
Expand Down Expand Up @@ -387,3 +392,90 @@ def update_webhook(self, project, integration):
debug_data = resp.content
log.debug('GitLab webhook update failure response: %s', debug_data)
return (False, resp)

def send_build_status(self, build, commit, state):
"""
Create GitLab commit status for project.

:param build: Build to set up commit status for
:type build: Build
:param state: build state failure, pending, or success.
:type state: str
:param commit: commit sha of the pull request
:type commit: str
:returns: boolean based on commit status creation was successful or not.
:rtype: Bool
"""
session = self.get_session()
project = build.project

repo_id = self._get_repo_id(project)

if repo_id is None:
return (False, None)

# select the correct state and description.
gitlab_build_state = SELECT_BUILD_STATUS[state]['gitlab']
description = SELECT_BUILD_STATUS[state]['description']

target_url = build.get_full_url()

if state == BUILD_STATUS_SUCCESS:
target_url = build.version.get_absolute_url()

data = {
'state': gitlab_build_state,
'target_url': target_url,
'description': description,
'context': RTD_BUILD_STATUS_API_NAME
}
url = self.adapter.provider_base_url

resp = None

try:
resp = session.post(
f'{url}/api/v4/projects/{repo_id}/statuses/{commit}',
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,
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,
)
return False

return False

# Catch exceptions with request or deserializing JSON
except (RequestException, ValueError):
log.exception(
'GitLab commit status creation failed for project: %s',
project,
)
# Response data should always be JSON, still try to log if not
# though
if resp is not None:
try:
debug_data = resp.json()
except ValueError:
debug_data = resp.content
else:
debug_data = resp

log.debug(
'GitLab commit status creation failure response: %s',
debug_data,
)
return False
7 changes: 3 additions & 4 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@
from readthedocs.doc_builder.python_environments import Conda, Virtualenv
from readthedocs.oauth.models import RemoteRepository
from readthedocs.oauth.notifications import GitBuildStatusFailureNotification
from readthedocs.oauth.services.github import GitHubService
from readthedocs.projects.constants import GITHUB_BRAND
from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND
from readthedocs.projects.models import APIProject, Feature
from readthedocs.search.utils import index_new_files, remove_indexed_files
from readthedocs.sphinx_domains.models import SphinxDomain
Expand Down Expand Up @@ -1936,7 +1935,7 @@ def send_build_status(build_pk, commit, status):
build = Build.objects.get(pk=build_pk)
provider_name = build.project.git_provider_name

if provider_name == GITHUB_BRAND:
if provider_name in [GITHUB_BRAND, GITLAB_BRAND]:
# get the service class for the project e.g: GitHubService.
service_class = build.project.git_service_class()
try:
Expand Down Expand Up @@ -1984,7 +1983,7 @@ def send_build_status(build_pk, commit, status):

return False

# TODO: Send build status for other providers.
# TODO: Send build status for BitBucket.


def send_external_build_status(version_type, build_pk, commit, status):
Expand Down
69 changes: 63 additions & 6 deletions readthedocs/rtd_tests/tests/test_celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ def test_fileify_logging_when_wrong_version_pk(self, mock_logger):
tasks.fileify(version_pk=345343, commit=None, build=1)
mock_logger.warning.assert_called_with("Version not found for given kwargs. {'pk': 345343}")

@patch('readthedocs.projects.tasks.GitHubService.send_build_status')
def test_send_build_status_task_with_remote_repo(self, send_build_status):
@patch('readthedocs.oauth.services.github.GitHubService.send_build_status')
def test_send_build_status_with_remote_repo_github(self, send_build_status):
self.project.repo = 'https://github.com/test/test/'
self.project.save()

Expand All @@ -356,8 +356,8 @@ def test_send_build_status_task_with_remote_repo(self, send_build_status):
)
self.assertEqual(Message.objects.filter(user=self.eric).count(), 0)

@patch('readthedocs.projects.tasks.GitHubService.send_build_status')
def test_send_build_status_task_with_social_account(self, send_build_status):
@patch('readthedocs.oauth.services.github.GitHubService.send_build_status')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this change? It seems like it was wrong before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we are not directly importing GitHubService to the tasks we are using project.git_service_class()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was working before because I forgot to remove an unused import of GitHubService

def test_send_build_status_with_social_account_github(self, send_build_status):
social_account = get(SocialAccount, user=self.eric, provider='github')

self.project.repo = 'https://github.com/test/test/'
Expand All @@ -376,8 +376,8 @@ def test_send_build_status_task_with_social_account(self, send_build_status):
)
self.assertEqual(Message.objects.filter(user=self.eric).count(), 0)

@patch('readthedocs.projects.tasks.GitHubService.send_build_status')
def test_send_build_status_task_without_remote_repo_or_social_account(self, send_build_status):
@patch('readthedocs.oauth.services.github.GitHubService.send_build_status')
def test_send_build_status_no_remote_repo_or_social_account_github(self, send_build_status):
self.project.repo = 'https://github.com/test/test/'
self.project.save()
external_version = get(Version, project=self.project, type=EXTERNAL)
Expand All @@ -390,3 +390,60 @@ def test_send_build_status_task_without_remote_repo_or_social_account(self, send

send_build_status.assert_not_called()
self.assertEqual(Message.objects.filter(user=self.eric).count(), 1)

@patch('readthedocs.oauth.services.gitlab.GitLabService.send_build_status')
def test_send_build_status_with_remote_repo_gitlab(self, send_build_status):
self.project.repo = 'https://gitlab.com/test/test/'
self.project.save()

social_account = get(SocialAccount, provider='gitlab')
remote_repo = get(RemoteRepository, account=social_account, project=self.project)
remote_repo.users.add(self.eric)

external_version = get(Version, project=self.project, type=EXTERNAL)
external_build = get(
Build, project=self.project, version=external_version
)
tasks.send_build_status(
external_build.id, external_build.commit, BUILD_STATUS_SUCCESS
)

send_build_status.assert_called_once_with(
external_build, external_build.commit, BUILD_STATUS_SUCCESS
)
self.assertEqual(Message.objects.filter(user=self.eric).count(), 0)

@patch('readthedocs.oauth.services.gitlab.GitLabService.send_build_status')
def test_send_build_status_with_social_account_gitlab(self, send_build_status):
social_account = get(SocialAccount, user=self.eric, provider='gitlab')

self.project.repo = 'https://gitlab.com/test/test/'
self.project.save()

external_version = get(Version, project=self.project, type=EXTERNAL)
external_build = get(
Build, project=self.project, version=external_version
)
tasks.send_build_status(
external_build.id, external_build.commit, BUILD_STATUS_SUCCESS
)

send_build_status.assert_called_once_with(
external_build, external_build.commit, BUILD_STATUS_SUCCESS
)
self.assertEqual(Message.objects.filter(user=self.eric).count(), 0)

@patch('readthedocs.oauth.services.gitlab.GitLabService.send_build_status')
def test_send_build_status_no_remote_repo_or_social_account_gitlab(self, send_build_status):
self.project.repo = 'https://gitlab.com/test/test/'
self.project.save()
external_version = get(Version, project=self.project, type=EXTERNAL)
external_build = get(
Build, project=self.project, version=external_version
)
tasks.send_build_status(
external_build.id, external_build.commit, BUILD_STATUS_SUCCESS
)

send_build_status.assert_not_called()
self.assertEqual(Message.objects.filter(user=self.eric).count(), 1)
61 changes: 61 additions & 0 deletions readthedocs/rtd_tests/tests/test_oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,10 @@ def setUp(self):
self.org = RemoteOrganization.objects.create(slug='testorga', json='')
self.privacy = self.project.version_privacy_level
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
)

def get_private_repo_data(self):
"""Manipulate repo response data to get private repo data."""
Expand Down Expand Up @@ -568,3 +572,60 @@ def test_setup_webhook(self):
success, response = self.service.setup_webhook(self.project)
self.assertFalse(success)
self.assertIsNone(response)

@mock.patch('readthedocs.oauth.services.gitlab.log')
@mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session')
@mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id')
def test_send_build_status_successful(self, repo_id, session, mock_logger):
session().post.return_value.status_code = 201
repo_id().return_value = '9999'

success = self.service.send_build_status(
self.external_build,
self.external_build.commit,
BUILD_STATUS_SUCCESS
)

self.assertTrue(success)
mock_logger.info.assert_called_with(
"GitLab commit status created for project: %s, commit status: %s",
self.project,
BUILD_STATUS_SUCCESS
)

@mock.patch('readthedocs.oauth.services.gitlab.log')
@mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session')
@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'

success = self.service.send_build_status(
self.external_build,
self.external_build.commit,
BUILD_STATUS_SUCCESS
)

self.assertFalse(success)
mock_logger.info.assert_called_with(
'GitLab project does not exist or user does not have '
'permissions: project=%s',
self.project
)

@mock.patch('readthedocs.oauth.services.gitlab.log')
@mock.patch('readthedocs.oauth.services.gitlab.GitLabService.get_session')
@mock.patch('readthedocs.oauth.services.gitlab.GitLabService._get_repo_id')
def test_send_build_status_value_error(self, repo_id, session, mock_logger):
session().post.side_effect = ValueError
repo_id().return_value = '9999'

success = self.service.send_build_status(
self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
)

self.assertFalse(success)
mock_logger.exception.assert_called_with(
'GitLab commit status creation failed for project: %s',
self.project,
)