diff --git a/readthedocs/builds/constants.py b/readthedocs/builds/constants.py index bdc9a70fe79..3606bd7415b 100644 --- a/readthedocs/builds/constants.py +++ b/readthedocs/builds/constants.py @@ -61,3 +61,29 @@ LATEST, STABLE, ) + +# GitHub Build Statuses +GITHUB_BUILD_STATE_FAILURE = 'failure' +GITHUB_BUILD_STATE_PENDING = 'pending' +GITHUB_BUILD_STATE_SUCCESS = 'success' + +# General Build Statuses +BUILD_STATUS_FAILURE = 'failed' +BUILD_STATUS_PENDING = 'pending' +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_STATE_FAILURE, + 'description': 'The build failed!', + }, + BUILD_STATUS_PENDING: { + 'github': GITHUB_BUILD_STATE_PENDING, + 'description': 'The build is pending!', + }, + BUILD_STATUS_SUCCESS: { + 'github': GITHUB_BUILD_STATE_SUCCESS, + 'description': 'The build succeeded!', + }, +} diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 8e857d5c0e1..f4cb9310db0 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -734,6 +734,16 @@ def __str__(self): def get_absolute_url(self): return reverse('builds_detail', args=[self.project.slug, self.pk]) + def get_full_url(self): + """Get full url including domain""" + scheme = 'http' if settings.DEBUG else 'https' + full_url = '{scheme}://{domain}{absolute_url}'.format( + scheme=scheme, + domain=settings.PRODUCTION_DOMAIN, + absolute_url=self.get_absolute_url() + ) + return full_url + @property def finished(self): """Return if build has a finished state.""" diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index dbb8245c6fb..8b8834d1f20 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -11,7 +11,10 @@ from django.utils.safestring import SafeText, mark_safe from django.utils.text import slugify as slugify_base -from readthedocs.builds.constants import BUILD_STATE_TRIGGERED +from readthedocs.builds.constants import ( + BUILD_STATE_TRIGGERED, + BUILD_STATUS_PENDING, +) from readthedocs.doc_builder.constants import DOCKER_LIMITS @@ -78,7 +81,10 @@ def prepare_build( # Avoid circular import from readthedocs.builds.models import Build from readthedocs.projects.models import Project - from readthedocs.projects.tasks import update_docs_task + from readthedocs.projects.tasks import ( + update_docs_task, + send_external_build_status, + ) build = None @@ -125,6 +131,10 @@ def prepare_build( options['soft_time_limit'] = time_limit options['time_limit'] = int(time_limit * 1.2) + if build: + # Send pending Build Status using Git Status API for External Builds. + send_external_build_status(build.id, BUILD_STATUS_PENDING) + return ( update_docs_task.signature( args=(version.pk,), diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 4d9fcb65e50..db47edf41cd 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -658,7 +658,7 @@ def failed(self): def done(self): """Is build in finished state.""" return ( - self.build is not None and + self.build and self.build['state'] == BUILD_STATE_FINISHED ) diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py index 8bf95707b6c..ba21e56c085 100644 --- a/readthedocs/oauth/services/base.py +++ b/readthedocs/oauth/services/base.py @@ -184,12 +184,28 @@ def paginate(self, url, **kwargs): return [] def sync(self): + """Sync repositories and organizations.""" raise NotImplementedError def create_repository(self, fields, privacy=None, organization=None): + """ + Update or create a repository from API response. + + :param fields: dictionary of response data from API + :param privacy: privacy level to support + :param organization: remote organization to associate with + :type organization: RemoteOrganization + :rtype: RemoteRepository + """ raise NotImplementedError def create_organization(self, fields): + """ + Update or create remote organization from API response. + + :param fields: dictionary response of data from API + :rtype: RemoteOrganization + """ raise NotImplementedError def get_next_url_to_paginate(self, response): @@ -211,9 +227,40 @@ def get_paginated_results(self, response): raise NotImplementedError def setup_webhook(self, project): + """ + Setup webhook for project. + + :param project: project to set up webhook for + :type project: Project + :returns: boolean based on webhook set up success, and requests Response object + :rtype: (Bool, Response) + """ raise NotImplementedError def update_webhook(self, project, integration): + """ + Update webhook integration. + + :param project: project to set up webhook for + :type project: Project + :param integration: Webhook integration to update + :type integration: Integration + :returns: boolean based on webhook update success, and requests Response object + :rtype: (Bool, Response) + """ + raise NotImplementedError + + def send_build_status(self, build, state): + """ + Create 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 + :returns: boolean based on commit status creation was successful or not. + :rtype: Bool + """ raise NotImplementedError @classmethod diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py index ce0ed3b9163..ce978f99996 100644 --- a/readthedocs/oauth/services/github.py +++ b/readthedocs/oauth/services/github.py @@ -12,6 +12,7 @@ from readthedocs.api.v2.client import api from readthedocs.builds import utils as build_utils +from readthedocs.builds.constants import SELECT_BUILD_STATUS from readthedocs.integrations.models import Integration from ..models import RemoteOrganization, RemoteRepository @@ -311,6 +312,80 @@ def update_webhook(self, project, integration): ) return (False, resp) + def send_build_status(self, build, state): + """ + Create GitHub 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 + :returns: boolean based on commit status creation was successful or not. + :rtype: Bool + """ + session = self.get_session() + project = build.project + owner, repo = build_utils.get_github_username_repo(url=project.repo) + build_sha = build.version.identifier + + # select the correct state and description. + github_build_state = SELECT_BUILD_STATUS[state]['github'] + description = SELECT_BUILD_STATUS[state]['description'] + + data = { + 'state': github_build_state, + 'target_url': build.get_full_url(), + 'description': description, + 'context': 'continuous-documentation/read-the-docs' + } + + resp = None + + try: + resp = session.post( + f'https://api.github.com/repos/{owner}/{repo}/statuses/{build_sha}', + data=json.dumps(data), + headers={'content-type': 'application/json'}, + ) + if resp.status_code == 201: + log.info( + 'GitHub commit status for project: %s', + project, + ) + return True + + if resp.status_code in [401, 403, 404]: + log.info( + 'GitHub 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( + 'GitHub 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( + 'GitHub commit status creation failure response: %s', + debug_data, + ) + return False + @classmethod def get_token_for_project(cls, project, force_local=False): """Get access token for project by iterating over project users.""" diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 12fe740c600..8a68836e911 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -34,6 +34,9 @@ LATEST, LATEST_VERBOSE_NAME, STABLE_VERBOSE_NAME, + EXTERNAL, + BUILD_STATUS_SUCCESS, + BUILD_STATUS_FAILURE, ) from readthedocs.builds.models import APIVersion, Build, Version from readthedocs.builds.signals import build_complete @@ -59,6 +62,8 @@ ) from readthedocs.doc_builder.loader import get_builder_class from readthedocs.doc_builder.python_environments import Conda, Virtualenv +from readthedocs.oauth.models import RemoteRepository +from readthedocs.oauth.services.github import GitHubService 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 @@ -573,6 +578,25 @@ def run_build(self, docker, record): if self.build_env.failed: self.send_notifications(self.version.pk, self.build['id']) + # send build failure status to git Status API + send_external_build_status( + self.build['id'], BUILD_STATUS_FAILURE + ) + elif self.build_env.successful: + # send build successful status to git Status API + send_external_build_status( + self.build['id'], BUILD_STATUS_SUCCESS + ) + else: + msg = 'Unhandled Build State' + log.warning( + LOG_TEMPLATE, + { + 'project': self.project.slug, + 'version': self.version.slug, + 'msg': msg, + } + ) build_complete.send(sender=Build, build=self.build_env.build) @@ -1513,8 +1537,11 @@ def _manage_imported_files(version, path, commit, build): @app.task(queue='web') def send_notifications(version_pk, build_pk): version = Version.objects.get_object_or_log(pk=version_pk) - if not version: + + # only send notification for Internal versions + if not version or version.type == EXTERNAL: return + build = Build.objects.get(pk=build_pk) for hook in version.project.webhook_notifications.all(): @@ -1773,3 +1800,45 @@ def retry_domain_verification(domain_pk): sender=domain.__class__, domain=domain, ) + + +@app.task(queue='web') +def send_build_status(build, state): + """ + Send Build Status to Git Status API for project external versions. + + :param build: Build + :param state: build state failed, pending, or success to be sent. + """ + try: + if build.project.remote_repository.account.provider == 'github': + service = GitHubService( + build.project.remote_repository.users.first(), + build.project.remote_repository.account + ) + + # send Status report using the API. + service.send_build_status(build, state) + + except RemoteRepository.DoesNotExist: + log.info('Remote repository does not exist for %s', build.project) + + except Exception: + log.exception('Send build status task failed for %s', build.project) + + # TODO: Send build status for other providers. + + +def send_external_build_status(build_pk, state): + """ + Check if build is external and Send Build Status for project external versions. + + :param build_pk: Build pk + :param state: build state failed, pending, or success to be sent. + """ + build = Build.objects.get(pk=build_pk) + + # Send status reports for only External (pull/merge request) Versions. + if build.version.type == EXTERNAL: + # call the task that actually send the build status. + send_build_status.delay(build, state) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index c1a78d496da..0cfa3de8721 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -906,7 +906,8 @@ def test_github_create_event(self, sync_repository_task, trigger_build): latest_version = self.project.versions.get(slug=LATEST) sync_repository_task.delay.assert_called_with(latest_version.pk) - def test_github_pull_request_opened_event(self, trigger_build): + @mock.patch('readthedocs.core.utils.trigger_build') + def test_github_pull_request_opened_event(self, trigger_build, core_trigger_build): client = APIClient() headers = {GITHUB_EVENT_HEADER: GITHUB_PULL_REQUEST} @@ -925,12 +926,13 @@ def test_github_pull_request_opened_event(self, trigger_build): self.assertTrue(resp.data['build_triggered']) self.assertEqual(resp.data['project'], self.project.slug) self.assertEqual(resp.data['versions'], [external_version.verbose_name]) - trigger_build.assert_called_once_with( + core_trigger_build.assert_called_once_with( force=True, project=self.project, version=external_version ) self.assertTrue(external_version) - def test_github_pull_request_reopened_event(self, trigger_build): + @mock.patch('readthedocs.core.utils.trigger_build') + def test_github_pull_request_reopened_event(self, trigger_build, core_trigger_build): client = APIClient() # Update the payload for `reopened` webhook event @@ -955,12 +957,13 @@ def test_github_pull_request_reopened_event(self, trigger_build): self.assertTrue(resp.data['build_triggered']) self.assertEqual(resp.data['project'], self.project.slug) self.assertEqual(resp.data['versions'], [external_version.verbose_name]) - trigger_build.assert_called_once_with( + core_trigger_build.assert_called_once_with( force=True, project=self.project, version=external_version ) self.assertTrue(external_version) - def test_github_pull_request_synchronize_event(self, trigger_build): + @mock.patch('readthedocs.core.utils.trigger_build') + def test_github_pull_request_synchronize_event(self, trigger_build, core_trigger_build): client = APIClient() pull_request_number = '6' @@ -998,13 +1001,14 @@ def test_github_pull_request_synchronize_event(self, trigger_build): self.assertTrue(resp.data['build_triggered']) self.assertEqual(resp.data['project'], self.project.slug) self.assertEqual(resp.data['versions'], [external_version.verbose_name]) - trigger_build.assert_called_once_with( + core_trigger_build.assert_called_once_with( force=True, project=self.project, version=external_version ) # `synchronize` webhook event updated the identifier (commit hash) self.assertNotEqual(prev_identifier, external_version.identifier) - def test_github_pull_request_closed_event(self, trigger_build): + @mock.patch('readthedocs.core.utils.trigger_build') + def test_github_pull_request_closed_event(self, trigger_build, core_trigger_build): client = APIClient() pull_request_number = '7' @@ -1044,7 +1048,7 @@ def test_github_pull_request_closed_event(self, trigger_build): self.assertTrue(resp.data['version_deleted']) self.assertEqual(resp.data['project'], self.project.slug) self.assertEqual(resp.data['versions'], [version.verbose_name]) - trigger_build.assert_not_called() + core_trigger_build.assert_not_called() def test_github_pull_request_no_action(self, trigger_build): client = APIClient() diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index e3036367e57..022c76a3294 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -7,11 +7,14 @@ from django_dynamic_fixture import get from mock import MagicMock, patch -from readthedocs.builds.constants import LATEST +from allauth.socialaccount.models import SocialAccount + +from readthedocs.builds.constants import LATEST, BUILD_STATUS_SUCCESS, EXTERNAL from readthedocs.builds.models import Build from readthedocs.doc_builder.exceptions import VersionLockedError from readthedocs.projects import tasks from readthedocs.builds.models import Version +from readthedocs.oauth.models import RemoteRepository from readthedocs.projects.exceptions import RepositoryError from readthedocs.projects.models import Project from readthedocs.rtd_tests.base import RTDTestCase @@ -329,3 +332,27 @@ def test_fileify_logging_when_wrong_version_pk(self, mock_logger): self.assertFalse(Version.objects.filter(pk=345343).exists()) 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(self, send_build_status): + social_account = get(SocialAccount, provider='github') + 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, BUILD_STATUS_SUCCESS) + + send_build_status.assert_called_once_with(external_build, BUILD_STATUS_SUCCESS) + + @patch('readthedocs.projects.tasks.GitHubService.send_build_status') + def test_send_build_status_task_without_remote_repo(self, send_build_status): + 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, BUILD_STATUS_SUCCESS) + + send_build_status.assert_not_called() diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py index 1ef675cc42c..b928e6b0aa5 100644 --- a/readthedocs/rtd_tests/tests/test_oauth.py +++ b/readthedocs/rtd_tests/tests/test_oauth.py @@ -5,6 +5,10 @@ from django.test import TestCase from django.test.utils import override_settings +from django_dynamic_fixture import get + +from readthedocs.builds.constants import EXTERNAL, BUILD_STATUS_SUCCESS +from readthedocs.builds.models import Version, Build from readthedocs.oauth.models import RemoteOrganization, RemoteRepository from readthedocs.oauth.services import ( BitbucketService, @@ -26,6 +30,10 @@ def setUp(self): self.org = RemoteOrganization.objects.create(slug='rtfd', json='') self.privacy = self.project.version_privacy_level self.service = GitHubService(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 test_make_project_pass(self): repo_json = { @@ -142,6 +150,51 @@ def test_multiple_users_same_repo(self): self.assertEqual(github_project, github_project_5) self.assertEqual(github_project_2, github_project_6) + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_send_build_status_successful(self, session, mock_logger): + session().post.return_value.status_code = 201 + success = self.service.send_build_status( + self.external_build, + BUILD_STATUS_SUCCESS + ) + + self.assertTrue(success) + mock_logger.info.assert_called_with( + 'GitHub commit status for project: %s', + self.project + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_send_build_status_404_error(self, session, mock_logger): + session().post.return_value.status_code = 404 + success = self.service.send_build_status( + self.external_build, + BUILD_STATUS_SUCCESS + ) + + self.assertFalse(success) + mock_logger.info.assert_called_with( + 'GitHub project does not exist or user does not have ' + 'permissions: project=%s', + self.project + ) + + @mock.patch('readthedocs.oauth.services.github.log') + @mock.patch('readthedocs.oauth.services.github.GitHubService.get_session') + def test_send_build_status_value_error(self, session, mock_logger): + session().post.side_effect = ValueError + success = self.service.send_build_status( + self.external_build, BUILD_STATUS_SUCCESS + ) + + self.assertFalse(success) + mock_logger.exception.assert_called_with( + 'GitHub commit status creation failed for project: %s', + self.project, + ) + @override_settings(DEFAULT_PRIVACY_LEVEL='private') def test_make_private_project(self): """ diff --git a/readthedocs/rtd_tests/tests/test_projects_tasks.py b/readthedocs/rtd_tests/tests/test_projects_tasks.py index c91f9b1534c..1ec1d8d0ae7 100644 --- a/readthedocs/rtd_tests/tests/test_projects_tasks.py +++ b/readthedocs/rtd_tests/tests/test_projects_tasks.py @@ -4,9 +4,14 @@ from django_dynamic_fixture import get from mock import patch -from readthedocs.builds.models import Version +from readthedocs.builds.constants import EXTERNAL, BUILD_STATUS_SUCCESS +from readthedocs.builds.models import Version, Build from readthedocs.projects.models import Project -from readthedocs.projects.tasks import sync_files +from readthedocs.projects.tasks import ( + sync_files, + send_external_build_status, + send_build_status, +) class SyncFilesTests(TestCase): @@ -121,3 +126,25 @@ def test_sync_files_search(self, rmtree, copy): self.assertIn('artifacts', args[0]) self.assertIn('sphinx_search', args[0]) self.assertIn('media/json', args[1]) + + +class SendBuildStatusTests(TestCase): + + def setUp(self): + self.project = get(Project) + self.internal_version = get(Version, project=self.project) + self.external_version = get(Version, project=self.project, type=EXTERNAL) + self.external_build = get(Build, project=self.project, version=self.external_version) + self.internal_build = get(Build, project=self.project, version=self.internal_version) + + @patch('readthedocs.projects.tasks.send_build_status') + def test_send_external_build_status_with_external_version(self, send_build_status): + send_external_build_status(self.external_build.id, BUILD_STATUS_SUCCESS) + + send_build_status.delay.assert_called_once_with(self.external_build, BUILD_STATUS_SUCCESS) + + @patch('readthedocs.projects.tasks.send_build_status') + def test_send_external_build_status_with_internal_version(self, send_build_status): + send_external_build_status(self.internal_build.id, BUILD_STATUS_SUCCESS) + + send_build_status.delay.assert_not_called()