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

Extend webhook notifications with build status #5621

Merged
merged 4 commits into from
Dec 9, 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: 5 additions & 3 deletions docs/guides/build-notifications.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ You should now get notified by email when your builds fail!
Using webhook
-------------

Read the Docs can also send webhooks when builds fail.
Read the Docs can also send webhooks when builds are triggered, successful or failed.

Take these steps to enable build notifications using a webhook:

* Go to :guilabel:`Admin` > :guilabel:`Notifications` in your project.
* Fill in the **URL** field under the **New Webhook Notifications** heading
* Submit the form

The project name, slug and its details for the build that failed will be sent as POST request to your webhook URL:
The project name, slug and its details for the build will be sent as POST request to your webhook URL:

.. code-block:: json

Expand All @@ -35,9 +35,11 @@ The project name, slug and its details for the build that failed will be sent as
"slug": "rtd",
"build": {
"id": 6321373,
"commit": "e8dd17a3f1627dd206d721e4be08ae6766fda40",
"state": "finished",
"success": false,
"date": "2017-02-15 20:35:54"
}
}

You should now get notified on your webhook when your builds fail!
You should now get notified on your webhook when your builds start and finish (failure/success)!
humitos marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from readthedocs.builds.constants import (
BUILD_STATE_TRIGGERED,
BUILD_STATUS_PENDING,
EXTERNAL,
)
from readthedocs.doc_builder.constants import DOCKER_LIMITS

Expand Down Expand Up @@ -86,6 +87,7 @@ def prepare_build(
from readthedocs.projects.tasks import (
update_docs_task,
send_external_build_status,
send_notifications,
)

build = None
Expand Down Expand Up @@ -142,6 +144,10 @@ def prepare_build(
commit=commit, status=BUILD_STATUS_PENDING
)

if build and version.type != EXTERNAL:
# Send Webhook notification for build triggered.
send_notifications.delay(version.pk, build_pk=build.pk, email=False)

return (
update_docs_task.signature(
args=(version.pk,),
Expand Down
28 changes: 17 additions & 11 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ def run(
self.setup_env.update_build(BUILD_STATE_FINISHED)

# Send notifications for unhandled errors
self.send_notifications(version_pk, build_pk)
self.send_notifications(version_pk, build_pk, email=True)
return False

def run_setup(self, record=True):
Expand Down Expand Up @@ -489,7 +489,7 @@ def run_setup(self, record=True):
# triggered before the previous one has finished (e.g. two webhooks,
# one after the other)
if not isinstance(self.setup_env.failure, VersionLockedError):
self.send_notifications(self.version.pk, self.build['id'])
self.send_notifications(self.version.pk, self.build['id'], email=True)
humitos marked this conversation as resolved.
Show resolved Hide resolved

return False

Expand Down Expand Up @@ -592,8 +592,8 @@ def run_build(self, docker, record):
log.warning('No build ID, not syncing files')

if self.build_env.failed:
# TODO: Send RTD Webhook notification for build failure.
self.send_notifications(self.version.pk, self.build['id'])
# Send Webhook and email notification for build failure.
self.send_notifications(self.version.pk, self.build['id'], email=True)

if self.commit:
send_external_build_status(
Expand All @@ -603,7 +603,9 @@ def run_build(self, docker, record):
status=BUILD_STATUS_FAILURE
)
elif self.build_env.successful:
# TODO: Send RTD Webhook notification for build success.
# Send Webhook notification for build success.
self.send_notifications(self.version.pk, self.build['id'], email=False)

if self.commit:
send_external_build_status(
version_type=self.version.type,
Expand Down Expand Up @@ -1065,10 +1067,10 @@ def build_docs_class(self, builder_class):
builder.move()
return success

def send_notifications(self, version_pk, build_pk):
def send_notifications(self, version_pk, build_pk, email=False):
"""Send notifications on build failure."""
if self.version.type != EXTERNAL:
send_notifications.delay(version_pk, build_pk=build_pk)
send_notifications.delay(version_pk, build_pk=build_pk, email=email)

def is_type_sphinx(self):
"""Is documentation type Sphinx."""
Expand Down Expand Up @@ -1613,7 +1615,7 @@ def _sync_imported_files(version, build, changed_files):


@app.task(queue='web')
def send_notifications(version_pk, build_pk):
def send_notifications(version_pk, build_pk, email=False):
version = Version.objects.get_object_or_log(pk=version_pk)

if not version:
Expand All @@ -1623,11 +1625,13 @@ def send_notifications(version_pk, build_pk):

for hook in version.project.webhook_notifications.all():
webhook_notification(version, build, hook.url)
for email in version.project.emailhook_notifications.all().values_list(

if email:
for email_address in version.project.emailhook_notifications.all().values_list(
'email',
flat=True,
):
email_notification(version, build, email)
):
email_notification(version, build, email_address)


def email_notification(version, build, email):
Expand Down Expand Up @@ -1703,6 +1707,8 @@ def webhook_notification(version, build, hook_url):
'slug': project.slug,
'build': {
'id': build.id,
'commit': build.commit,
'state': build.state,
Copy link
Member

Choose a reason for hiding this comment

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

We should define what data we send here in the docs as part of this PR, it will be much more useful to users that way :)

'success': build.success,
'date': build.date.strftime('%Y-%m-%d %H:%M:%S'),
},
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/rtd_tests/tests/test_build_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ def test_send_webhook_notification(self):

def test_send_email_notification(self):
fixture.get(EmailHook, project=self.project)
send_notifications(self.version.pk, self.build.pk)
send_notifications(self.version.pk, self.build.pk, email=True)
self.assertEqual(len(mail.outbox), 1)

def test_send_email_and_webhook__notification(self):
fixture.get(EmailHook, project=self.project)
fixture.get(WebHook, project=self.project)
with patch('readthedocs.projects.tasks.requests.post') as mock:
mock.return_value = None
send_notifications(self.version.pk, self.build.pk)
send_notifications(self.version.pk, self.build.pk, email=True)
mock.assert_called_once()
self.assertEqual(len(mail.outbox), 1)

Expand Down