[GSoC 2026] fix(settings): single, env-driven definition for CHATBOT_QUEUE - #3919
Conversation
…QUEUE CHATBOT_QUEUE was assigned twice: in settings/chatbot.py from the environment, and in settings/celery.py as a hardcoded literal. Both modules are wildcard imported by settings/__init__.py, so the literal (imported last) shadowed the env read and CHATBOT_QUEUE in the environment had no effect on settings.CHATBOT_QUEUE. Keep a single definition in settings/celery.py, next to the other queue names and the CELERY_QUEUES loop that registers it. One assignment makes the value independent of the wildcard import order, so re-ordering those lines cannot resurface the bug. settings/celery.py is also the lower-level module (it must run before the others and imports only ._util and .aws), so this adds no new import edge; the reverse direction would have pulled .cache into it. The consumer side was hardcoded too: docker/entrypoints/celery_chatbot.sh pinned "chatbot" in its -Q list, so a custom queue name would have published chat turns to a queue no worker drains. It now reads the same variable, with the same unset/blank fallback, and the variable is documented in env_file_app_template. tests/intel_owl/test_settings.py boots Django in a subprocess with a custom CHATBOT_QUEUE and asserts it reaches settings, CELERY_QUEUES and the Celery route of process_chat_message; plus an AST guard that fails if any setting is ever again assigned in two wildcard-imported settings modules. Refs #3910
|
Moved to draft: #3911 by @Aditya30ag was already open for the same issue (#3910) and I only noticed after opening |
|
we'll merge this once ready |
…HATBOT_QUEUE
Split tests/intel_owl/test_settings.py into one file per concern, so a settings
PR touching the chatbot queue does not collide with one touching the shadowing
invariant: test_settings_chatbot_queue.py (the queue reaches settings and the
Celery route) and test_settings_shadowing.py (no setting is assigned twice).
Add the coupling guard the fix was missing. Django publishes the chat task and
the worker entrypoint consumes it, each reading CHATBOT_QUEUE on its own side,
so the default queue name now lives in two layers: flipping one of them silently
sends turns to a queue nobody drains -- the same failure this setting already
had, one layer down. The new test extracts ${CHATBOT_QUEUE:-...} from
celery_chatbot.sh, compares it with what the settings fall back to, and fails if
either side hardcodes the queue again. Verified by mutation: flipping the shell
default to "chat" fails with 'chat' != 'chatbot'.
The shadowing guard now also audits __init__.py's own assignments (TEST_RUNNER,
INSTALLED_APPS), which are made before the wildcard block and could be shadowed
by a submodule -- the same bug in the opposite direction. Verified by mutation
on a copy of the settings package.
Also: the probe reads the routed task name from process_chat_message.name
instead of repeating the dotted path, the subprocess timeout is a named constant
with its rationale, and celery.py states why the lookup is environment-only (the
worker side is a shell script and cannot resolve an AWS secret).
|
Ready. Addressed the coupling the fix itself introduced: with both Django and the worker entrypoint reading Note the red |
|
can you please pull from develop and try again? I pushed the fix for the shellcheck URL |
…into gsoc-2026/chatbot-queue-setting
Done. Thanks |
Description
Fixes the duplicate
CHATBOT_QUEUEdefinition reported by @Aditya30ag in #3910 — thanks forcatching it, the analysis in the issue was exactly right.
@Aditya30ag also proposed a fix in #3911, which @mlodic decided to supersede with this one. The
settings change there was correct; this PR additionally covers the worker entrypoint, the env
template and the regression tests.
CHATBOT_QUEUEwas assigned in two settings modules:intel_owl/settings/chatbot.py:19secrets.get_secret("CHATBOT_QUEUE", "chatbot")(env-driven)intel_owl/settings/celery.py:21"chatbot"(hardcoded literal)Both are wildcard-imported by
intel_owl/settings/__init__.py(.chatbotat line 76,.celeryatline 79). The last wildcard wins, so the literal shadowed the env read and
CHATBOT_QUEUEin theenvironment never reached
settings.CHATBOT_QUEUE. Reproduced before the fix:The fix
The surviving definition is the env-driven one, moved into
settings/celery.py:settings/celery.pyalready ownsDEFAULT_QUEUE/BROADCAST_QUEUE/CONFIG_QUEUEand theCELERY_QUEUESloop that consumes them, and it is the lower-level module (it carries# this module must run before the othersand imports only._utiland.aws). Keeping thedefinition in
chatbot.pyinstead would have forcedcelery.pytofrom .chatbot import CHATBOT_QUEUE, inverting that layering and pulling.cacheinto the module that must run first.The fix is order-independent by construction: with a single assignment, no arrangement of the
wildcard imports in
__init__.pycan change the resulting value — which is the property the issueasks for.
Trade-off worth flagging: the lookup moves from
intel_owl.secrets.get_secret(AWS Secrets Managerfallback) to
settings/_util.get_secret(os.environ.get), matching the siblingBROKER_URLandCELERY_QUEUES. A queue name is not a secret, and the AWS fallback for this key never workedanyway since the value was being overwritten.
The other half of the bug
Making the setting configurable is not enough on its own:
docker/entrypoints/celery_chatbot.shhardcoded the consumer side (
queues="chatbot,broadcast,config", orchatbot.fifo,config.fifounder SQS), and
CHATBOT_QUEUEwas missing fromdocker/env_file_app_template. A custom queue namewould therefore have published chat turns to a queue no worker drains, hanging the chat. The
entrypoint now reads the same variable, with
${CHATBOT_QUEUE:-chatbot}matching theorfallbackon the Python side so both ends agree on unset and on blank.
test.ollama.override.ymlandci.ollama.override.ymlinherit the entrypoint fromollama.override.yml, so they are covered.api_app/chatbot_manager/health.pyneeds no change — it already derives the queue fromsettings.CHATBOT_QUEUE, and it remains the detector for a producer/consumer mismatch.Audit: is any other setting defined twice?
No.
CHATBOT_QUEUEwas the only name assigned in two wildcard-imported settings modules. Theother 15 name collisions (
DEBUG,CACHES,AWS_SQS,get_secret,WEB_CLIENT_URL, …) arere-exports of the same object via
from .X import Y, so every import order yields the same value.Tests
Two new files under
tests/intel_owl/, one per concern. The first two tests below fail on the codebefore this PR:
test_settings_chatbot_queue.py—test_custom_chatbot_queue_reaches_settings_and_task_routingboots Django in a subprocess with
CHATBOT_QUEUE=my_custom_queueand asserts the value reachessettings.CHATBOT_QUEUE(
AssertionError: 'chatbot' != 'my_custom_queue'before the fix), lands inCELERY_QUEUES, isthe routing target of
process_chat_messageviaget_queue_name, and that a queue is actuallydeclared for it. A second case pins the fallback for unset and blank values. The subprocess is
necessary: settings are read at import time, so
override_settingswould assign the value thetest is trying to prove is computed, and reloading the settings package in-process corrupts it
for the rest of the suite.
test_settings_shadowing.py—test_no_setting_is_assigned_in_two_wildcard_imported_modulesisan AST guard over
intel_owl/settings/: it fails if any setting is ever again assigned in two ofthose modules (before the fix it reports
{'CHATBOT_QUEUE': ['celery.py:21', 'chatbot.py:19']}). It audits__init__.py's ownassignments too —
TEST_RUNNERandINSTALLED_APPSare set before the wildcard block and asubmodule shadowing one of them is the same bug in the opposite direction. It distinguishes
assignment from re-export, so the 15 benign collisions above do not trip it.
test_settings_chatbot_queue.py—test_worker_entrypoint_derives_its_queue_from_the_settingcloses the gap the fix itself opens: with both layers reading
CHATBOT_QUEUE, the default queuename now exists in two places, and letting them drift reproduces this very bug one layer down. It
extracts
${CHATBOT_QUEUE:-…}from the entrypoint, compares it with the settings fallback, andfails if either side hardcodes the queue again.
Both guards were verified by mutation rather than assumed: flipping the shell default to
chatfails with
'chat' != 'chatbot', and assigningTEST_RUNNERin a submodule (on a throwaway copy ofthe settings package) is reported as
{'TEST_RUNNER': ['__init__.py:7', 'websocket.py:20']}.Ruff (
check+format --check) andshellcheckare clean on the changed files.Type of change
Checklist
developRuff) gave 0 errors.testsfolder). All the tests (new and old ones) gave 0 errors.DeepSource,Django Doctorsor other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.Refs #3910