Skip to content

Remove broadcast function #7044

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 7 commits into from
Mar 24, 2021
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
7 changes: 0 additions & 7 deletions docs/development/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,6 @@ Default: :djangosetting:`RTD_INTERSPHINX_URL`
This is the domain that is used to fetch the intersphinx inventory file.
If not set explicitly this is the ``PRODUCTION_DOMAIN``.

MULTIPLE_APP_SERVERS
--------------------

Default: :djangosetting:`MULTIPLE_APP_SERVERS`

This is a list of application servers that built documentation is copied to. This allows you to run an independent build server, and then have it rsync your built documentation across multiple front end documentation/app servers.

DEFAULT_PRIVACY_LEVEL
---------------------

Expand Down
55 changes: 12 additions & 43 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,63 +6,32 @@
import os
import re

from celery import chord, group
from django.conf import settings
from django.utils import timezone
from django.utils.functional import keep_lazy
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,
BUILD_STATE_FINISHED,
BUILD_STATE_TRIGGERED,
BUILD_STATUS_PENDING,
EXTERNAL,
)
from readthedocs.doc_builder.constants import DOCKER_LIMITS
from readthedocs.projects.constants import CELERY_LOW, CELERY_MEDIUM, CELERY_HIGH
from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError, DuplicatedBuildError

from readthedocs.doc_builder.exceptions import (
BuildMaxConcurrencyError,
DuplicatedBuildError,
)
from readthedocs.projects.constants import (
CELERY_HIGH,
CELERY_LOW,
CELERY_MEDIUM,
)

log = logging.getLogger(__name__)


def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable=redefined-builtin
"""
Run a broadcast across our servers.

Returns a task group that can be checked for results.

`callback` should be a task signature that will be run once,
after all of the broadcast tasks have finished running.
"""
if type not in ['web', 'app', 'build']:
raise ValueError('allowed value of `type` are web, app and build.')
if kwargs is None:
kwargs = {}

if type in ['web', 'app']:
servers = settings.MULTIPLE_APP_SERVERS
elif type in ['build']:
servers = settings.MULTIPLE_BUILD_SERVERS

tasks = []
for server in servers:
task_sig = task.s(*args, **kwargs).set(queue=server)
tasks.append(task_sig)
if callback:
task_promise = chord(tasks, callback).apply_async()
else:
# Celery's Group class does some special handling when an iterable with
# len() == 1 is passed in. This will be hit if there is only one server
# defined in the above queue lists
if len(tasks) > 1:
task_promise = group(*tasks).apply_async()
else:
task_promise = group(tasks).apply_async()
return task_promise


def prepare_build(
project,
version=None,
Expand All @@ -88,11 +57,11 @@ def prepare_build(
"""
# Avoid circular import
from readthedocs.builds.models import Build
from readthedocs.projects.models import Project, Feature
from readthedocs.projects.models import Feature, Project
from readthedocs.projects.tasks import (
update_docs_task,
send_external_build_status,
send_notifications,
update_docs_task,
)

build = None
Expand Down
14 changes: 0 additions & 14 deletions readthedocs/core/utils/general.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
# -*- coding: utf-8 -*-

import os

from django.shortcuts import get_object_or_404

from readthedocs.core.utils import broadcast
from readthedocs.projects.tasks import remove_dirs
from readthedocs.builds.models import Version
from readthedocs.storage import build_environment_storage

Expand All @@ -17,14 +11,6 @@ def wipe_version_via_slugs(version_slug, project_slug):
slug=version_slug,
project__slug=project_slug,
)
del_dirs = [
os.path.join(version.project.doc_path, 'checkouts', version.slug),
os.path.join(version.project.doc_path, 'envs', version.slug),
os.path.join(version.project.doc_path, 'conda', version.slug),
os.path.join(version.project.doc_path, '.cache'),
]
for del_dir in del_dirs:
broadcast(type='build', task=remove_dirs, args=[(del_dir,)])

# Delete the cache environment from storage
build_environment_storage.delete(version.get_storage_environment_cache_path())
7 changes: 1 addition & 6 deletions readthedocs/projects/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,7 @@ def ban_owner(self, request, queryset):
ban_owner.short_description = 'Ban project owner'

def delete_selected_and_artifacts(self, request, queryset):
"""
Remove HTML/etc artifacts from storage.

Prior to the query delete, broadcast tasks to delete HTML artifacts from
application instances.
"""
"""Remove HTML/etc artifacts from storage."""
if request.POST.get('post'):
for project in queryset:
clean_project_resources(project)
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from readthedocs.builds.constants import EXTERNAL, INTERNAL, LATEST, STABLE
from readthedocs.constants import pattern_opts
from readthedocs.core.resolver import resolve, resolve_domain
from readthedocs.core.utils import broadcast, slugify
from readthedocs.core.utils import slugify
from readthedocs.doc_builder.constants import DOCKER_LIMITS
from readthedocs.projects import constants
from readthedocs.projects.exceptions import ProjectConfigurationError
Expand Down
9 changes: 2 additions & 7 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1138,12 +1138,7 @@ def update_app_instances(
pdf=False,
epub=False,
):
"""
Update application instances with build artifacts.

This triggers updates across application instances for html, pdf, epub,
downloads, and search. Tasks are broadcast to all web servers from here.
"""
"""Update build artifacts and index search data."""
# Update version if we have successfully built HTML output
# And store whether the build had other media types
try:
Expand All @@ -1162,7 +1157,7 @@ def update_app_instances(
self.version,
)

# Broadcast finalization steps to web application instances
# Index search data
fileify.delay(
version_pk=self.version.pk,
commit=self.build['commit'],
Expand Down
7 changes: 5 additions & 2 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,11 @@
Version,
VersionAutomationRule,
)
from readthedocs.core.mixins import ListViewWithForm, PrivateViewMixin
from readthedocs.core.utils import broadcast, trigger_build
from readthedocs.core.mixins import (
ListViewWithForm,
PrivateViewMixin,
)
from readthedocs.core.utils import trigger_build
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.integrations.models import HttpExchange, Integration
from readthedocs.oauth.services import registry
Expand Down
7 changes: 3 additions & 4 deletions readthedocs/rtd_tests/tests/test_core_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ def setUp(self):
self.pip_latest_document_url = url_base.format(version='/en/latest/document')
self.pip_latest_document_page_url = url_base.format(version='/en/latest/document.html')

with mock.patch('readthedocs.projects.models.broadcast'):
self.client.login(username='eric', password='test')
self.pip = Project.objects.get(slug='pip')
self.pip_fr = Project.objects.create(name='PIP-FR', slug='pip-fr', language='fr', main_language_project=self.pip)
self.client.login(username='eric', password='test')
self.pip = Project.objects.get(slug='pip')
self.pip_fr = Project.objects.create(name='PIP-FR', slug='pip-fr', language='fr', main_language_project=self.pip)

def test_project_only(self):
proj = Project.objects.get(slug='pip')
Expand Down
76 changes: 10 additions & 66 deletions readthedocs/rtd_tests/tests/test_core_utils.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
"""Test core util functions."""

import os
from unittest import mock

import pytest
from unittest import mock
from django.http import Http404
from django.test import TestCase
from django_dynamic_fixture import get
from unittest.mock import call

from readthedocs.builds.constants import LATEST, BUILD_STATE_BUILDING
from readthedocs.builds.models import Version, Build
from readthedocs.core.utils import slugify, trigger_build, prepare_build
from readthedocs.core.utils.general import wipe_version_via_slugs
from readthedocs.builds.constants import BUILD_STATE_BUILDING, LATEST
from readthedocs.builds.models import Build, Version
from readthedocs.core.utils import slugify, trigger_build
from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError
from readthedocs.projects.constants import CELERY_LOW, CELERY_MEDIUM, CELERY_HIGH
from readthedocs.projects.models import Project, Feature
from readthedocs.projects.tasks import remove_dirs
from readthedocs.projects.constants import (
CELERY_HIGH,
CELERY_LOW,
CELERY_MEDIUM,
)
from readthedocs.projects.models import Feature, Project


class CoreUtilTests(TestCase):
Expand Down Expand Up @@ -296,58 +295,3 @@ def test_slugify(self):
slugify('A title_-_with separated parts', dns_safe=False),
'a-title_-_with-separated-parts',
)

@mock.patch('readthedocs.core.utils.general.broadcast')
def test_wipe_version_via_slug(self, mock_broadcast):
wipe_version_via_slugs(
version_slug=self.version.slug,
project_slug=self.version.project.slug
)
expected_del_dirs = [
os.path.join(self.version.project.doc_path, 'checkouts', self.version.slug),
os.path.join(self.version.project.doc_path, 'envs', self.version.slug),
os.path.join(self.version.project.doc_path, 'conda', self.version.slug),
]

mock_broadcast.assert_has_calls(
[
call(type='build', task=remove_dirs, args=[(expected_del_dirs[0],)]),
call(type='build', task=remove_dirs, args=[(expected_del_dirs[1],)]),
call(type='build', task=remove_dirs, args=[(expected_del_dirs[2],)]),
],
any_order=False
)

@mock.patch('readthedocs.core.utils.general.broadcast')
def test_wipe_version_via_slug_wrong_param(self, mock_broadcast):
self.assertFalse(Version.objects.filter(slug='wrong-slug').exists())
with self.assertRaises(Http404):
wipe_version_via_slugs(
version_slug='wrong-slug',
project_slug=self.version.project.slug
)
mock_broadcast.assert_not_called()

@mock.patch('readthedocs.core.utils.general.broadcast')
def test_wipe_version_via_slugs_same_version_slug_with_diff_proj(self, mock_broadcast):
project_2 = get(Project)
version_2 = get(Version, project=project_2, slug=self.version.slug)
wipe_version_via_slugs(
version_slug=version_2.slug,
project_slug=project_2.slug,
)

expected_del_dirs = [
os.path.join(version_2.project.doc_path, 'checkouts', version_2.slug),
os.path.join(version_2.project.doc_path, 'envs', version_2.slug),
os.path.join(version_2.project.doc_path, 'conda', version_2.slug),
]

mock_broadcast.assert_has_calls(
[
call(type='build', task=remove_dirs, args=[(expected_del_dirs[0],)]),
call(type='build', task=remove_dirs, args=[(expected_del_dirs[1],)]),
call(type='build', task=remove_dirs, args=[(expected_del_dirs[2],)]),
],
any_order=False
)
Loading