Skip to content

Commit 7634195

Browse files
authored
Remove broadcast function (#7044)
1 parent 27b1f02 commit 7634195

File tree

13 files changed

+88
-268
lines changed

13 files changed

+88
-268
lines changed

docs/development/settings.rst

-7
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,6 @@ Default: :djangosetting:`RTD_INTERSPHINX_URL`
4646
This is the domain that is used to fetch the intersphinx inventory file.
4747
If not set explicitly this is the ``PRODUCTION_DOMAIN``.
4848

49-
MULTIPLE_APP_SERVERS
50-
--------------------
51-
52-
Default: :djangosetting:`MULTIPLE_APP_SERVERS`
53-
54-
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.
55-
5649
DEFAULT_PRIVACY_LEVEL
5750
---------------------
5851

readthedocs/core/utils/__init__.py

+12-43
Original file line numberDiff line numberDiff line change
@@ -6,63 +6,32 @@
66
import os
77
import re
88

9-
from celery import chord, group
109
from django.conf import settings
1110
from django.utils import timezone
1211
from django.utils.functional import keep_lazy
1312
from django.utils.safestring import SafeText, mark_safe
1413
from django.utils.text import slugify as slugify_base
1514

1615
from readthedocs.builds.constants import (
17-
BUILD_STATE_TRIGGERED,
1816
BUILD_STATE_FINISHED,
17+
BUILD_STATE_TRIGGERED,
1918
BUILD_STATUS_PENDING,
2019
EXTERNAL,
2120
)
2221
from readthedocs.doc_builder.constants import DOCKER_LIMITS
23-
from readthedocs.projects.constants import CELERY_LOW, CELERY_MEDIUM, CELERY_HIGH
24-
from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError, DuplicatedBuildError
25-
22+
from readthedocs.doc_builder.exceptions import (
23+
BuildMaxConcurrencyError,
24+
DuplicatedBuildError,
25+
)
26+
from readthedocs.projects.constants import (
27+
CELERY_HIGH,
28+
CELERY_LOW,
29+
CELERY_MEDIUM,
30+
)
2631

2732
log = logging.getLogger(__name__)
2833

2934

30-
def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable=redefined-builtin
31-
"""
32-
Run a broadcast across our servers.
33-
34-
Returns a task group that can be checked for results.
35-
36-
`callback` should be a task signature that will be run once,
37-
after all of the broadcast tasks have finished running.
38-
"""
39-
if type not in ['web', 'app', 'build']:
40-
raise ValueError('allowed value of `type` are web, app and build.')
41-
if kwargs is None:
42-
kwargs = {}
43-
44-
if type in ['web', 'app']:
45-
servers = settings.MULTIPLE_APP_SERVERS
46-
elif type in ['build']:
47-
servers = settings.MULTIPLE_BUILD_SERVERS
48-
49-
tasks = []
50-
for server in servers:
51-
task_sig = task.s(*args, **kwargs).set(queue=server)
52-
tasks.append(task_sig)
53-
if callback:
54-
task_promise = chord(tasks, callback).apply_async()
55-
else:
56-
# Celery's Group class does some special handling when an iterable with
57-
# len() == 1 is passed in. This will be hit if there is only one server
58-
# defined in the above queue lists
59-
if len(tasks) > 1:
60-
task_promise = group(*tasks).apply_async()
61-
else:
62-
task_promise = group(tasks).apply_async()
63-
return task_promise
64-
65-
6635
def prepare_build(
6736
project,
6837
version=None,
@@ -88,11 +57,11 @@ def prepare_build(
8857
"""
8958
# Avoid circular import
9059
from readthedocs.builds.models import Build
91-
from readthedocs.projects.models import Project, Feature
60+
from readthedocs.projects.models import Feature, Project
9261
from readthedocs.projects.tasks import (
93-
update_docs_task,
9462
send_external_build_status,
9563
send_notifications,
64+
update_docs_task,
9665
)
9766

9867
build = None

readthedocs/core/utils/general.py

-14
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
1-
# -*- coding: utf-8 -*-
2-
3-
import os
4-
51
from django.shortcuts import get_object_or_404
62

7-
from readthedocs.core.utils import broadcast
8-
from readthedocs.projects.tasks import remove_dirs
93
from readthedocs.builds.models import Version
104
from readthedocs.storage import build_environment_storage
115

@@ -17,14 +11,6 @@ def wipe_version_via_slugs(version_slug, project_slug):
1711
slug=version_slug,
1812
project__slug=project_slug,
1913
)
20-
del_dirs = [
21-
os.path.join(version.project.doc_path, 'checkouts', version.slug),
22-
os.path.join(version.project.doc_path, 'envs', version.slug),
23-
os.path.join(version.project.doc_path, 'conda', version.slug),
24-
os.path.join(version.project.doc_path, '.cache'),
25-
]
26-
for del_dir in del_dirs:
27-
broadcast(type='build', task=remove_dirs, args=[(del_dir,)])
2814

2915
# Delete the cache environment from storage
3016
build_environment_storage.delete(version.get_storage_environment_cache_path())

readthedocs/projects/admin.py

+1-6
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,7 @@ def ban_owner(self, request, queryset):
221221
ban_owner.short_description = 'Ban project owner'
222222

223223
def delete_selected_and_artifacts(self, request, queryset):
224-
"""
225-
Remove HTML/etc artifacts from storage.
226-
227-
Prior to the query delete, broadcast tasks to delete HTML artifacts from
228-
application instances.
229-
"""
224+
"""Remove HTML/etc artifacts from storage."""
230225
if request.POST.get('post'):
231226
for project in queryset:
232227
clean_project_resources(project)

readthedocs/projects/models.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
from readthedocs.builds.constants import EXTERNAL, INTERNAL, LATEST, STABLE
2727
from readthedocs.constants import pattern_opts
2828
from readthedocs.core.resolver import resolve, resolve_domain
29-
from readthedocs.core.utils import broadcast, slugify
29+
from readthedocs.core.utils import slugify
3030
from readthedocs.doc_builder.constants import DOCKER_LIMITS
3131
from readthedocs.projects import constants
3232
from readthedocs.projects.exceptions import ProjectConfigurationError

readthedocs/projects/tasks.py

+2-7
Original file line numberDiff line numberDiff line change
@@ -1138,12 +1138,7 @@ def update_app_instances(
11381138
pdf=False,
11391139
epub=False,
11401140
):
1141-
"""
1142-
Update application instances with build artifacts.
1143-
1144-
This triggers updates across application instances for html, pdf, epub,
1145-
downloads, and search. Tasks are broadcast to all web servers from here.
1146-
"""
1141+
"""Update build artifacts and index search data."""
11471142
# Update version if we have successfully built HTML output
11481143
# And store whether the build had other media types
11491144
try:
@@ -1162,7 +1157,7 @@ def update_app_instances(
11621157
self.version,
11631158
)
11641159

1165-
# Broadcast finalization steps to web application instances
1160+
# Index search data
11661161
fileify.delay(
11671162
version_pk=self.version.pk,
11681163
commit=self.build['commit'],

readthedocs/projects/views/private.py

+5-2
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,11 @@
4040
Version,
4141
VersionAutomationRule,
4242
)
43-
from readthedocs.core.mixins import ListViewWithForm, PrivateViewMixin
44-
from readthedocs.core.utils import broadcast, trigger_build
43+
from readthedocs.core.mixins import (
44+
ListViewWithForm,
45+
PrivateViewMixin,
46+
)
47+
from readthedocs.core.utils import trigger_build
4548
from readthedocs.core.utils.extend import SettingsOverrideObject
4649
from readthedocs.integrations.models import HttpExchange, Integration
4750
from readthedocs.oauth.services import registry

readthedocs/rtd_tests/tests/test_core_tags.py

+3-4
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,9 @@ def setUp(self):
2929
self.pip_latest_document_url = url_base.format(version='/en/latest/document')
3030
self.pip_latest_document_page_url = url_base.format(version='/en/latest/document.html')
3131

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

3736
def test_project_only(self):
3837
proj = Project.objects.get(slug='pip')

readthedocs/rtd_tests/tests/test_core_utils.py

+10-66
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,21 @@
11
"""Test core util functions."""
22

3-
import os
3+
from unittest import mock
44

55
import pytest
6-
from unittest import mock
7-
from django.http import Http404
86
from django.test import TestCase
97
from django_dynamic_fixture import get
10-
from unittest.mock import call
118

12-
from readthedocs.builds.constants import LATEST, BUILD_STATE_BUILDING
13-
from readthedocs.builds.models import Version, Build
14-
from readthedocs.core.utils import slugify, trigger_build, prepare_build
15-
from readthedocs.core.utils.general import wipe_version_via_slugs
9+
from readthedocs.builds.constants import BUILD_STATE_BUILDING, LATEST
10+
from readthedocs.builds.models import Build, Version
11+
from readthedocs.core.utils import slugify, trigger_build
1612
from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError
17-
from readthedocs.projects.constants import CELERY_LOW, CELERY_MEDIUM, CELERY_HIGH
18-
from readthedocs.projects.models import Project, Feature
19-
from readthedocs.projects.tasks import remove_dirs
13+
from readthedocs.projects.constants import (
14+
CELERY_HIGH,
15+
CELERY_LOW,
16+
CELERY_MEDIUM,
17+
)
18+
from readthedocs.projects.models import Feature, Project
2019

2120

2221
class CoreUtilTests(TestCase):
@@ -296,58 +295,3 @@ def test_slugify(self):
296295
slugify('A title_-_with separated parts', dns_safe=False),
297296
'a-title_-_with-separated-parts',
298297
)
299-
300-
@mock.patch('readthedocs.core.utils.general.broadcast')
301-
def test_wipe_version_via_slug(self, mock_broadcast):
302-
wipe_version_via_slugs(
303-
version_slug=self.version.slug,
304-
project_slug=self.version.project.slug
305-
)
306-
expected_del_dirs = [
307-
os.path.join(self.version.project.doc_path, 'checkouts', self.version.slug),
308-
os.path.join(self.version.project.doc_path, 'envs', self.version.slug),
309-
os.path.join(self.version.project.doc_path, 'conda', self.version.slug),
310-
]
311-
312-
mock_broadcast.assert_has_calls(
313-
[
314-
call(type='build', task=remove_dirs, args=[(expected_del_dirs[0],)]),
315-
call(type='build', task=remove_dirs, args=[(expected_del_dirs[1],)]),
316-
call(type='build', task=remove_dirs, args=[(expected_del_dirs[2],)]),
317-
],
318-
any_order=False
319-
)
320-
321-
@mock.patch('readthedocs.core.utils.general.broadcast')
322-
def test_wipe_version_via_slug_wrong_param(self, mock_broadcast):
323-
self.assertFalse(Version.objects.filter(slug='wrong-slug').exists())
324-
with self.assertRaises(Http404):
325-
wipe_version_via_slugs(
326-
version_slug='wrong-slug',
327-
project_slug=self.version.project.slug
328-
)
329-
mock_broadcast.assert_not_called()
330-
331-
@mock.patch('readthedocs.core.utils.general.broadcast')
332-
def test_wipe_version_via_slugs_same_version_slug_with_diff_proj(self, mock_broadcast):
333-
project_2 = get(Project)
334-
version_2 = get(Version, project=project_2, slug=self.version.slug)
335-
wipe_version_via_slugs(
336-
version_slug=version_2.slug,
337-
project_slug=project_2.slug,
338-
)
339-
340-
expected_del_dirs = [
341-
os.path.join(version_2.project.doc_path, 'checkouts', version_2.slug),
342-
os.path.join(version_2.project.doc_path, 'envs', version_2.slug),
343-
os.path.join(version_2.project.doc_path, 'conda', version_2.slug),
344-
]
345-
346-
mock_broadcast.assert_has_calls(
347-
[
348-
call(type='build', task=remove_dirs, args=[(expected_del_dirs[0],)]),
349-
call(type='build', task=remove_dirs, args=[(expected_del_dirs[1],)]),
350-
call(type='build', task=remove_dirs, args=[(expected_del_dirs[2],)]),
351-
],
352-
any_order=False
353-
)

0 commit comments

Comments
 (0)