Skip to content

Commit

Permalink
Merge pull request #1120 from kaedroho/kill-celery
Browse files Browse the repository at this point in the history
No longer use celery for sending notifications
  • Loading branch information
davecranwell committed Mar 31, 2015
2 parents 7d74978 + f077fb0 commit bbeb6cc
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 142 deletions.
15 changes: 0 additions & 15 deletions docs/howto/performance.rst
Expand Up @@ -28,21 +28,6 @@ We recommend `Redis <http://redis.io/>`_ as a fast, persistent cache. Install Re
Without a persistent cache, Wagtail will recreate all compressable assets at each server start, e.g. when any files change under ``./manage.py runserver``.


Sending emails in the background using Celery
---------------------------------------------

Various actions in the Wagtail admin backend can trigger notification emails - for example, submitting a page for moderation. In Wagtail's default configuration, these are sent as part of the page request/response cycle, which means that web server threads can get tied up for long periods if many emails are being sent. To avoid this, Wagtail can be configured to do this as a background task, using `Celery <http://www.celeryproject.org/>`_ as a task queue. To install Celery, add ``django-celery`` to your requirements.txt. A sample configuration, using Redis as the queue backend, would look like::

import djcelery

djcelery.setup_loader()

CELERY_SEND_TASK_ERROR_EMAILS = True
BROKER_URL = 'redis://'

See the Celery documentation for instructions on running the worker process in development or production.


Search
------

Expand Down
13 changes: 0 additions & 13 deletions wagtail/project_template/project_name/settings/production.py
Expand Up @@ -13,19 +13,6 @@
COMPRESS_OFFLINE = True


# Send notification emails as a background task using Celery,
# to prevent this from blocking web server threads
# (requires the django-celery package):
# http://celery.readthedocs.org/en/latest/configuration.html

# import djcelery
#
# djcelery.setup_loader()
#
# CELERY_SEND_TASK_ERROR_EMAILS = True
# BROKER_URL = 'redis://'


# Use Redis as the cache backend for extra performance
# (requires the django-redis-cache package):
# http://wagtail.readthedocs.org/en/latest/howto/performance.html#cache
Expand Down
1 change: 0 additions & 1 deletion wagtail/project_template/requirements.txt
Expand Up @@ -8,4 +8,3 @@ wagtail==1.0b1

# Recommended components to improve performance in production:
# django-redis==3.8.2
# django-celery==3.1.10
101 changes: 0 additions & 101 deletions wagtail/wagtailadmin/tasks.py

This file was deleted.

41 changes: 37 additions & 4 deletions wagtail/wagtailadmin/tests/tests.py
@@ -1,10 +1,10 @@
from django.test import TestCase
from django.test import TestCase, override_settings
from django.core.urlresolvers import reverse
from django.core import mail

from wagtail.tests.utils import WagtailTestUtils
from wagtail.wagtailcore.models import Page
from wagtail.wagtailadmin.tasks import send_email_task
from wagtail.wagtailadmin.utils import send_mail


class TestHome(TestCase, WagtailTestUtils):
Expand Down Expand Up @@ -56,15 +56,48 @@ def test_editor_css_and_js_hooks_on_edit(self):
self.assertContains(response, '<script src="/path/to/my/custom.js"></script>')


class TestSendEmailTask(TestCase):
class TestSendMail(TestCase):
def test_send_email(self):
send_email_task("Test subject", "Test content", ["nobody@email.com"], "test@email.com")
send_mail("Test subject", "Test content", ["nobody@email.com"], "test@email.com")

# Check that the email was sent
self.assertEqual(len(mail.outbox), 1)
self.assertEqual(mail.outbox[0].subject, "Test subject")
self.assertEqual(mail.outbox[0].body, "Test content")
self.assertEqual(mail.outbox[0].to, ["nobody@email.com"])
self.assertEqual(mail.outbox[0].from_email, "test@email.com")

@override_settings(WAGTAILADMIN_NOTIFICATION_FROM_EMAIL='anothertest@email.com')
def test_send_fallback_to_wagtailadmin_notification_from_email_setting(self):
send_mail("Test subject", "Test content", ["nobody@email.com"])

# Check that the email was sent
self.assertEqual(len(mail.outbox), 1)
self.assertEqual(mail.outbox[0].subject, "Test subject")
self.assertEqual(mail.outbox[0].body, "Test content")
self.assertEqual(mail.outbox[0].to, ["nobody@email.com"])
self.assertEqual(mail.outbox[0].from_email, "anothertest@email.com")

@override_settings(DEFAULT_FROM_EMAIL='yetanothertest@email.com')
def test_send_fallback_to_default_from_email_setting(self):
send_mail("Test subject", "Test content", ["nobody@email.com"])

# Check that the email was sent
self.assertEqual(len(mail.outbox), 1)
self.assertEqual(mail.outbox[0].subject, "Test subject")
self.assertEqual(mail.outbox[0].body, "Test content")
self.assertEqual(mail.outbox[0].to, ["nobody@email.com"])
self.assertEqual(mail.outbox[0].from_email, "yetanothertest@email.com")

def test_send_default_from_email(self):
send_mail("Test subject", "Test content", ["nobody@email.com"])

# Check that the email was sent
self.assertEqual(len(mail.outbox), 1)
self.assertEqual(mail.outbox[0].subject, "Test subject")
self.assertEqual(mail.outbox[0].body, "Test content")
self.assertEqual(mail.outbox[0].to, ["nobody@email.com"])
self.assertEqual(mail.outbox[0].from_email, "webmaster@localhost")


class TestExplorerNavView(TestCase, WagtailTestUtils):
Expand Down
71 changes: 70 additions & 1 deletion wagtail/wagtailadmin/utils.py
@@ -1,6 +1,13 @@
from django.template.loader import render_to_string
from django.core.mail import send_mail as django_send_mail
from django.conf import settings
from django.contrib.auth import get_user_model
from django.db.models import Q

from modelcluster.fields import ParentalKey

from wagtail.wagtailcore.models import Page
from wagtail.wagtailcore.models import Page, PageRevision, GroupPagePermission
from wagtail.wagtailusers.models import UserProfile


def get_object_usage(obj):
Expand Down Expand Up @@ -34,3 +41,65 @@ def get_object_usage(obj):
)

return pages


def users_with_page_permission(page, permission_type, include_superusers=True):
# Get user model
User = get_user_model()

# Find GroupPagePermission records of the given type that apply to this page or an ancestor
ancestors_and_self = list(page.get_ancestors()) + [page]
perm = GroupPagePermission.objects.filter(permission_type=permission_type, page__in=ancestors_and_self)
q = Q(groups__page_permissions=perm)

# Include superusers
if include_superusers:
q |= Q(is_superuser=True)

return User.objects.filter(is_active=True).filter(q).distinct()


def send_mail(email_subject, email_content, email_addresses, from_email=None):
if not from_email:
if hasattr(settings, 'WAGTAILADMIN_NOTIFICATION_FROM_EMAIL'):
from_email = settings.WAGTAILADMIN_NOTIFICATION_FROM_EMAIL
elif hasattr(settings, 'DEFAULT_FROM_EMAIL'):
from_email = settings.DEFAULT_FROM_EMAIL
else:
from_email = 'webmaster@localhost'

django_send_mail(email_subject, email_content, from_email, email_addresses)


def send_notification(page_revision_id, notification, excluded_user_id):
# Get revision
revision = PageRevision.objects.get(id=page_revision_id)

# Get list of recipients
if notification == 'submitted':
# Get list of publishers
recipients = users_with_page_permission(revision.page, 'publish')
elif notification in ['rejected', 'approved']:
# Get submitter
recipients = [revision.user]
else:
return

# Get list of email addresses
email_addresses = [
recipient.email for recipient in recipients
if recipient.email and recipient.id != excluded_user_id and getattr(UserProfile.get_for_user(recipient), notification + '_notifications')
]

# Return if there are no email addresses
if not email_addresses:
return

# Get email subject and content
template = 'wagtailadmin/notifications/' + notification + '.html'
rendered_template = render_to_string(template, dict(revision=revision, settings=settings)).split('\n')
email_subject = rendered_template[0]
email_content = '\n'.join(rendered_template[1:])

# Send email
send_mail(email_subject, email_content, email_addresses)
11 changes: 6 additions & 5 deletions wagtail/wagtailadmin/views/pages.py
Expand Up @@ -15,7 +15,8 @@

from wagtail.wagtailadmin.edit_handlers import TabbedInterface, ObjectList
from wagtail.wagtailadmin.forms import SearchForm, CopyForm
from wagtail.wagtailadmin import tasks, signals
from wagtail.wagtailadmin.utils import send_notification
from wagtail.wagtailadmin import signals

from wagtail.wagtailcore import hooks
from wagtail.wagtailcore.models import Page, PageRevision, get_navigation_menu_items
Expand Down Expand Up @@ -225,7 +226,7 @@ def clean():
messages.success(request, _("Page '{0}' published.").format(page.title))
elif is_submitting:
messages.success(request, _("Page '{0}' submitted for moderation.").format(page.title))
tasks.send_notification.delay(page.get_latest_revision().id, 'submitted', request.user.id)
send_notification(page.get_latest_revision().id, 'submitted', request.user.id)
else:
messages.success(request, _("Page '{0}' created.").format(page.title))

Expand Down Expand Up @@ -361,7 +362,7 @@ def clean():
messages.button(reverse('wagtailadmin_pages_view_draft', args=(page_id,)), _('View draft')),
messages.button(reverse('wagtailadmin_pages_edit', args=(page_id,)), _('Edit'))
])
tasks.send_notification.delay(page.get_latest_revision().id, 'submitted', request.user.id)
send_notification(page.get_latest_revision().id, 'submitted', request.user.id)
else:
messages.success(request, _("Page '{0}' updated.").format(page.title))

Expand Down Expand Up @@ -782,7 +783,7 @@ def approve_moderation(request, revision_id):
if request.method == 'POST':
revision.approve_moderation()
messages.success(request, _("Page '{0}' published.").format(revision.page.title))
tasks.send_notification.delay(revision.id, 'approved', request.user.id)
send_notification(revision.id, 'approved', request.user.id)

return redirect('wagtailadmin_home')

Expand All @@ -799,7 +800,7 @@ def reject_moderation(request, revision_id):
if request.method == 'POST':
revision.reject_moderation()
messages.success(request, _("Page '{0}' rejected for publication.").format(revision.page.title))
tasks.send_notification.delay(revision.id, 'rejected', request.user.id)
send_notification(revision.id, 'rejected', request.user.id)

return redirect('wagtailadmin_home')

Expand Down
4 changes: 2 additions & 2 deletions wagtail/wagtailforms/models.py
Expand Up @@ -16,7 +16,7 @@

from wagtail.wagtailcore.models import Page, Orderable, UserPagePermissionsProxy, get_page_types
from wagtail.wagtailadmin.edit_handlers import FieldPanel
from wagtail.wagtailadmin import tasks
from wagtail.wagtailadmin.utils import send_mail

from .forms import FormBuilder

Expand Down Expand Up @@ -194,7 +194,7 @@ def process_form_submission(self, form):

if self.to_address:
content = '\n'.join([x[1].label + ': ' + form.data.get(x[0]) for x in form.fields.items()])
tasks.send_email_task.delay(self.subject, content, [self.to_address], self.from_address,)
send_mail(self.subject, content, [self.to_address], self.from_address,)


class Meta:
Expand Down

0 comments on commit bbeb6cc

Please sign in to comment.