Skip to content

Commit 3fef273

Browse files
emylleremyller
committed
Move task processor database settings validation to the right place
1 parent 6bb822c commit 3fef273

File tree

4 files changed

+110
-75
lines changed

4 files changed

+110
-75
lines changed

‎src/task_processor/apps.py‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from django.apps import AppConfig
22
from django.conf import settings
3+
from django.core.exceptions import ImproperlyConfigured
34
from health_check.plugins import plugin_dir # type: ignore[import-untyped]
45

56
from task_processor.task_run_method import TaskRunMethod
@@ -9,10 +10,43 @@ class TaskProcessorAppConfig(AppConfig):
910
name = "task_processor"
1011

1112
def ready(self) -> None:
13+
self._validate_database_settings()
14+
self._register_health_check()
15+
16+
def _register_health_check(self) -> None:
17+
"""
18+
Register the health check for the task processor
19+
"""
20+
if not settings.ENABLE_TASK_PROCESSOR_HEALTH_CHECK:
21+
return
22+
23+
if settings.TASK_RUN_METHOD != TaskRunMethod.TASK_PROCESSOR:
24+
return
25+
26+
from .health import TaskProcessorHealthCheckBackend
27+
28+
plugin_dir.register(TaskProcessorHealthCheckBackend)
29+
30+
def _validate_database_settings(self) -> None:
31+
"""
32+
Validate that multi-database is setup correctly
33+
"""
34+
if settings.TASK_RUN_METHOD != TaskRunMethod.TASK_PROCESSOR:
35+
return
36+
37+
if "task_processor" not in settings.TASK_PROCESSOR_DATABASES:
38+
return # Not using a separate database
39+
40+
if "task_processor" not in settings.DATABASES:
41+
raise ImproperlyConfigured(
42+
"DATABASES must include 'task_processor' when using a separate task processor database."
43+
)
44+
1245
if (
13-
settings.ENABLE_TASK_PROCESSOR_HEALTH_CHECK
14-
and settings.TASK_RUN_METHOD == TaskRunMethod.TASK_PROCESSOR
46+
"task_processor.routers.TaskProcessorRouter"
47+
not in settings.DATABASE_ROUTERS
1548
):
16-
from .health import TaskProcessorHealthCheckBackend
17-
18-
plugin_dir.register(TaskProcessorHealthCheckBackend)
49+
raise ImproperlyConfigured(
50+
"DATABASE_ROUTERS must include 'task_processor.routers.TaskProcessorRouter' "
51+
"when using a separate task processor database."
52+
)

‎src/task_processor/threads.py‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,6 @@ def run_iteration(self) -> None:
104104
separate database setup.
105105
"""
106106
database_is_separate = "task_processor" in settings.TASK_PROCESSOR_DATABASES
107-
if database_is_separate:
108-
assert (
109-
"task_processor.routers.TaskProcessorRouter"
110-
in settings.DATABASE_ROUTERS
111-
), (
112-
"DATABASE_ROUTERS must include 'task_processor.routers.TaskProcessorRouter' "
113-
"when using a separate task processor database."
114-
) # This is for our own sanity
115-
assert "task_processor" in settings.DATABASES, (
116-
"DATABASES must include 'task_processor' when using a separate task processor database."
117-
) # ¯\_(ツ)_/¯ One has to read the documentation and fix it: https://docs.flagsmith.com/deployment/configuration/task-processor
118107

119108
for database in settings.TASK_PROCESSOR_DATABASES:
120109
try:

‎tests/unit/task_processor/test_unit_task_processor_apps.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import typing
2+
3+
import pytest
4+
from django.core.exceptions import ImproperlyConfigured
5+
from pytest_django.fixtures import SettingsWrapper
6+
from pytest_mock import MockerFixture
7+
8+
from task_processor import apps
9+
10+
11+
@pytest.mark.parametrize(
12+
"task_processor_databases",
13+
[
14+
# Default mode (try and consume remaining tasks from "default")
15+
["default", "task_processor"],
16+
# Opt-out mode (try and consume remaining tasks from "task_processor")
17+
["task_processor", "default"],
18+
# Single database mode
19+
["task_processor"],
20+
],
21+
)
22+
@pytest.mark.parametrize(
23+
"setup, expected_error",
24+
[
25+
(
26+
{ # Missing router and database
27+
"DATABASE_ROUTERS": [],
28+
"DATABASES": {"default": "exists but not task processor"},
29+
},
30+
"DATABASES must include 'task_processor' when using a separate task processor database.",
31+
),
32+
(
33+
{ # Missing router
34+
"DATABASE_ROUTERS": [],
35+
"DATABASES": {
36+
"default": "hooray",
37+
"task_processor": "exists",
38+
},
39+
},
40+
"DATABASE_ROUTERS must include 'task_processor.routers.TaskProcessorRouter' when using a separate task processor database.",
41+
),
42+
(
43+
{ # Missing database
44+
"DATABASE_ROUTERS": ["task_processor.routers.TaskProcessorRouter"],
45+
"DATABASES": {"default": "exists but not task processor"},
46+
},
47+
"DATABASES must include 'task_processor' when using a separate task processor database.",
48+
),
49+
],
50+
)
51+
def test_validates_django_settings_are_compatible_with_multi_database_setup(
52+
expected_error: str,
53+
mocker: MockerFixture,
54+
settings: SettingsWrapper,
55+
setup: dict[str, typing.Any],
56+
task_processor_databases: list[str],
57+
) -> None:
58+
# Given
59+
settings.TASK_PROCESSOR_DATABASES = task_processor_databases
60+
settings.DATABASE_ROUTERS = setup["DATABASE_ROUTERS"]
61+
settings.DATABASES = setup["DATABASES"]
62+
task_processor_app = mocker.Mock()
63+
64+
# When
65+
apps.TaskProcessorAppConfig.ready(task_processor_app)
66+
with pytest.raises(ImproperlyConfigured) as excinfo:
67+
apps.TaskProcessorAppConfig._validate_database_settings(task_processor_app)
68+
69+
# Then
70+
task_processor_app._validate_database_settings.assert_called_once_with()
71+
assert str(excinfo.value) == expected_error

‎tests/unit/task_processor/test_unit_task_processor_threads.py‎

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import logging
2-
import typing
32

43
import pytest
54
from django.db import DatabaseError
@@ -93,61 +92,3 @@ def test_task_runner__single_database___consumes_tasks_from_one_databases(
9392
assert run_recurring_tasks.call_args_list == [
9493
mocker.call(database),
9594
]
96-
97-
98-
@pytest.mark.parametrize(
99-
"task_processor_databases",
100-
[
101-
["default", "task_processor"], # Default mode (try and consume remaining tasks)
102-
["task_processor", "default"], # Opt-out mode (try and consume remaining tasks)
103-
["task_processor"], # Single database mode
104-
],
105-
)
106-
@pytest.mark.parametrize(
107-
"setup, expected_error",
108-
[
109-
(
110-
{ # Missing router and database
111-
"DATABASE_ROUTERS": [],
112-
"DATABASES": {"default": "exists but not task processor"},
113-
},
114-
"DATABASE_ROUTERS must include 'task_processor.routers.TaskProcessorRouter' when using a separate task processor database.",
115-
),
116-
(
117-
{ # Missing router
118-
"DATABASE_ROUTERS": [],
119-
"DATABASES": {
120-
"default": "hooray",
121-
"task_processor": "exists",
122-
},
123-
},
124-
"DATABASE_ROUTERS must include 'task_processor.routers.TaskProcessorRouter' when using a separate task processor database.",
125-
),
126-
(
127-
{ # Missing database
128-
"DATABASE_ROUTERS": ["task_processor.routers.TaskProcessorRouter"],
129-
"DATABASES": {"default": "exists but not task processor"},
130-
},
131-
"DATABASES must include 'task_processor' when using a separate task processor database.",
132-
),
133-
],
134-
)
135-
def test_validates_django_settings_are_compatible_with_multi_database_setup(
136-
expected_error: str,
137-
mocker: MockerFixture,
138-
settings: SettingsWrapper,
139-
setup: dict[str, typing.Any],
140-
task_processor_databases: list[str],
141-
) -> None:
142-
# Given
143-
settings.TASK_PROCESSOR_DATABASES = task_processor_databases
144-
settings.DATABASE_ROUTERS = setup["DATABASE_ROUTERS"]
145-
settings.DATABASES = setup["DATABASES"]
146-
task_runner = mocker.Mock()
147-
148-
# When
149-
with pytest.raises(AssertionError) as excinfo:
150-
threads.TaskRunner.run_iteration(task_runner)
151-
152-
# Then
153-
assert str(excinfo.value) == expected_error

0 commit comments

Comments
 (0)