Skip to content
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

Send Build Status Report Using GitHub Status API #5865

Merged
26 changes: 26 additions & 0 deletions readthedocs/builds/constants.py
Expand Up @@ -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!',
},
}
10 changes: 10 additions & 0 deletions readthedocs/builds/models.py
Expand Up @@ -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."""
Expand Down
14 changes: 12 additions & 2 deletions readthedocs/core/utils/__init__.py
Expand Up @@ -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


Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -125,6 +131,10 @@ def prepare_build(
options['soft_time_limit'] = time_limit
options['time_limit'] = int(time_limit * 1.2)

if build:
saadmk11 marked this conversation as resolved.
Show resolved Hide resolved
# 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,),
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/doc_builder/environments.py
Expand Up @@ -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
ericholscher marked this conversation as resolved.
Show resolved Hide resolved
self.build['state'] == BUILD_STATE_FINISHED
)

Expand Down
47 changes: 47 additions & 0 deletions readthedocs/oauth/services/base.py
Expand Up @@ -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):
Expand All @@ -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):
stsewd marked this conversation as resolved.
Show resolved Hide resolved
"""
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
Expand Down
75 changes: 75 additions & 0 deletions readthedocs/oauth/services/github.py
Expand Up @@ -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
Expand Down Expand Up @@ -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

stsewd marked this conversation as resolved.
Show resolved Hide resolved
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

stsewd marked this conversation as resolved.
Show resolved Hide resolved
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."""
Expand Down
71 changes: 70 additions & 1 deletion readthedocs/projects/tasks.py
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

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

What is the other state here? Should this just be an else? If not, we should do an else with a log.warning.

Copy link
Member Author

@saadmk11 saadmk11 Jul 9, 2019

Choose a reason for hiding this comment

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

@ericholscher there are no other states (BuildEnvironment) , it will succeed or fail (in the Build Environment) at this point of the build. we could add an warning if we want. but I don't think we need to :)

# 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)

Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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):
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this function be in utils instead of tasks?

Copy link
Member Author

Choose a reason for hiding this comment

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

This will just create a circular import error, as the send_build_status task is already in the task.py file

"""
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)