feat(docker): decouple NestBot service with separate container #3939
feat(docker): decouple NestBot service with separate container #3939Ani07-05 wants to merge 15 commits intoOWASP:feature/nestbot-ai-assistantfrom
Conversation
…#3865) - Create dedicated nestbot service running on port 8001 - Move docs service to port 8002 to avoid port conflicts - Migrate from Alpine to Debian slim base images for smaller size - Disable Slack tokens in backend service (routes not registered) - Enable Slack tokens in nestbot service for Slack integration - Configure service dependencies to ensure migrations run first - Update user/group creation for Debian compatibility Closes OWASP#3865
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughAdds a new nestbot service and image, moves docs to port 8002, reassigns AI-related dependencies into a nestbot Poetry group, introduces a nestbot-specific Dockerfile and venv volume, updates compose envs/depends_on for backend/worker, and shifts several Makefile targets to run via the nestbot container. (49 words) Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docker-compose/local/compose.yaml (1)
147-175:⚠️ Potential issue | 🟡 MinorPre-existing inconsistency: worker uses
cachewhile other services usenest-cachefor Redis host.Line 168 has
DJANGO_REDIS_HOST: ${DJANGO_REDIS_HOST:-cache}while backend (Line 25) and nestbot (Line 60) use${DJANGO_REDIS_HOST:-nest-cache}. The Redis container name isnest-cache, so the worker's default fallback would fail DNS resolution within the Docker network.This is pre-existing, but since you're touching this file and adding a new service with the correct value, it'd be good to fix it while you're here.
Proposed fix
DJANGO_REDIS_HOST: ${DJANGO_REDIS_HOST:-cache} + DJANGO_REDIS_HOST: ${DJANGO_REDIS_HOST:-nest-cache}
🤖 Fix all issues with AI agents
In `@docker-compose/local/compose.yaml`:
- Around line 46-48: The compose depends_on currently uses condition:
service_started which only waits for the backend container to start; add a
proper healthcheck to the backend service (healthcheck test using
["CMD","curl","-f","http://localhost:8000/"] with interval 5s, timeout 5s,
retries 10, start_period 30s) and update the nestbot depends_on entry to use
condition: service_healthy so NestBot waits until backend migrations/health are
complete before starting; look for the backend service block and the nestbot
depends_on block in the compose.yaml to apply these changes.
🧹 Nitpick comments (3)
docker/backend/Dockerfile.local (2)
56-57: Poetry installed in the runtime stage — is this intentional?Poetry is already installed and used in the builder stage. Since this is a local dev Dockerfile and the source is volume-mounted at runtime, I assume this is intentional so developers can run
poetry add/poetry installinside the container. If so, a brief comment (like the one on Line 1) would clarify the intent for future readers.
46-54: Minor inconsistency: builder uses ENV vars for UID/GID, runtime hardcodes them.The builder stage references
${OWASP_GID}and${OWASP_UID}from the ENV block, but the runtime stage hardcodes1000directly ingroupadd/useradd. Consider using the same ENV-based approach for consistency, or at least document the assumption.docker-compose/local/compose.yaml (1)
39-69: Sharedbackend-venvvolume across backend, nestbot, and worker containers.All three services mount the same
backend-venv:/home/owasp/.venvvolume. While this avoids duplicating the venv, concurrent writes (e.g., two developers runningpoetry installfrom different containers) could corrupt the venv. For local dev this is likely acceptable, but worth documenting as a known constraint.Also, since
nestbothas nobuilddirective and referencesimage: nest-local-backend, ensure Docker Compose builds the backend service first (e.g.,docker compose up --buildshould work since the image is defined by thebackendservice).
…e startup reliability * Updated worker Redis host fallback to `nest-cache` for consistent DNS resolution across services * Changed dependency condition from `service_started` → `service_healthy` to ensure backend migrations complete before dependent services start * Aligns docker-compose configuration with expected container readiness behavior * Resolves automated review feedback from CodeRabbit
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docker-compose/local/compose.yaml">
<violation number="1" location="docker-compose/local/compose.yaml:32">
P2: Backend healthcheck doesn't verify HTTP server readiness. `manage.py check` validates Django config but can pass before the server is listening on port 8000 (e.g., while migrations are still running). Since nestbot and worker depend on `service_healthy`, they may start before backend is actually serving. Consider using an HTTP-based check consistent with the nestbot service, e.g., `curl -f http://localhost:8000/`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docker-compose/local/compose.yaml`:
- Around line 31-36: The current healthcheck uses "manage.py check" which can
pass before migrations finish, allowing services like nestbot and worker to
start too early; change the service healthcheck block (healthcheck/test) to
perform an HTTP probe against the Django server (e.g., attempt an HTTP GET to
the runserver root or a simple health endpoint using Python's urllib) so the
check only succeeds once "migrate && exec runserver" has completed and the
server is actually serving; update the healthcheck command (replace the
manage.py check invocation) and keep sensible
interval/timeout/retries/start_period values so the container only becomes
healthy after runserver is up.
🧹 Nitpick comments (1)
docker-compose/local/compose.yaml (1)
60-69: Consider extracting shared environment variables to reduce duplication.The
nestbot,backend, andworkerservices repeat the same DB/Redis environment block. A YAML anchor orx-extension field would centralize these and reduce drift risk.Example using x- extension
x-common-env: &common-env PYTHONUNBUFFERED: 1 DJANGO_DB_HOST: ${DJANGO_DB_HOST:-db} DJANGO_DB_NAME: ${DJANGO_DB_NAME:-nest_db_dev} DJANGO_DB_PASSWORD: ${DJANGO_DB_PASSWORD:-nest_user_dev_password} DJANGO_DB_PORT: ${DJANGO_DB_PORT:-5432} DJANGO_DB_USER: ${DJANGO_DB_USER:-nest_user_dev} DJANGO_REDIS_HOST: ${DJANGO_REDIS_HOST:-nest-cache} DJANGO_REDIS_PASSWORD: ${DJANGO_REDIS_PASSWORD:-nest-cache-password} services: backend: environment: <<: *common-env DJANGO_SLACK_BOT_TOKEN: "" DJANGO_SLACK_SIGNING_SECRET: "" nestbot: environment: <<: *common-env worker: environment: <<: *common-env
Ani07-05
changed the title
rudransh-shrivastava
left a comment
rudransh-shrivastava
left a comment
There was a problem hiding this comment.
Hi, we need to separate it further:
- we should have a separate poetry dependency group for NestBot.
- we should also separate it on the docker level (backend excludes NestBot related dependencies).
Hi, should NestBot get its own Dockerfile or should I handle it via poetry groups + conditional install in the existing one? |
Let's create another Dockerfile for now. Edit: I think we can also use the main Dockerfile as base for this one. |
|
Hi @rudransh-shrivastava , I'm creating a new the worker service runs |
|
Yes, I think so. The worker should use the nestbot image. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/pyproject.toml (1)
71-75: Consider marking thenestbotgroup asoptional = trueto prevent unintentional installation.By default, dependencies across all non-optional groups will be installed when executing
poetry install. Sincenestbotis not marked optional, any developer running barepoetry install(outside Docker) will pull in the heavy AI/ML stack (crewai, langchain, langgraph). Given the explicit purpose of this PR is to isolate these deps, marking the group optional keeps that boundary clean for non-Docker workflows:+[tool.poetry.group.nestbot] +optional = true + [tool.poetry.group.nestbot.dependencies] crewai = { version = "^1.7.2", python = ">=3.10,<3.14" }If you do this, update
Dockerfile.nestbot.localto use--with nestbotinstead of relying on default install:- poetry install --no-root --without dev --without test --without video + poetry install --no-root --without dev --without test --without video --with nestbot🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/pyproject.toml` around lines 71 - 75, Mark the nestbot dependency group as optional in pyproject.toml by setting [tool.poetry.group.nestbot] optional = true (so the existing [tool.poetry.group.nestbot.dependencies] entries for crewai, langchain, langchain-community and langgraph are not installed by default), and update Dockerfile.nestbot.local to run poetry install --with nestbot to ensure the group is installed in the nestbot Docker build.docker/backend/Dockerfile.nestbot.local (1)
56-57: Poetry installation in the runtime stage is unnecessary.The
.venvis fully built in the builder stage and copied at Line 64. The runtime container only runspython manage.py runserver; Poetry is never invoked. This adds ~50–80 MB to the runtime image with no benefit.If Poetry is kept for developer convenience (e.g.,
docker execto add packages), add an explanatory comment; otherwise remove:-RUN --mount=type=cache,target=${PIP_CACHE_DIR} \ - python -m pip install poetry --cache-dir ${PIP_CACHE_DIR} - EXPOSE 8001🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/backend/Dockerfile.nestbot.local` around lines 56 - 57, The runtime stage currently installs Poetry with the RUN --mount=type=cache... python -m pip install poetry step even though the built .venv is copied from the builder (the runtime only executes python manage.py runserver and never invokes Poetry); remove that RUN line to slim the image, or if you intentionally want Poetry in the runtime for interactive docker exec workflows, keep the line but add a clear comment above it explaining it's only for developer convenience and not required for runtime execution (refer to the poetry install RUN and the later COPY of the built .venv into the runtime).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose/local/compose.yaml`:
- Around line 28-30: The YAML currently sets DJANGO_SLACK_BOT_TOKEN and
DJANGO_SLACK_SIGNING_SECRET to bare None (YAML null), which allows host-shell
values to leak into the container; change those environment entries to explicit
empty strings (DJANGO_SLACK_BOT_TOKEN: "" and DJANGO_SLACK_SIGNING_SECRET: "")
so the environment section unconditionally overrides any env_file or host
variables and guarantees Slack is disabled in the backend.
In `@docker/backend/Dockerfile.nestbot.local`:
- Line 2: The Dockerfile uses a floating base image tag in the FROM instruction
("FROM python:3.13-slim AS builder"); change that to a pinned patch-level tag to
ensure reproducible builds (e.g., align with production and use
"python:3.13.7-slim" or "python:3.13.7-alpine" as appropriate) by replacing the
floating tag in the FROM line of the builder stage.
---
Duplicate comments:
In `@docker/backend/Dockerfile.local`:
- Line 2: The Dockerfile uses an unpinned base image ("FROM python:3.13-slim AS
builder"); update this to the same pinned python image tag used in
Dockerfile.nestbot.local (replace "python:3.13-slim" with the exact pinned tag
used there) so both Dockerfiles reference the identical, version-pinned base
image.
- Around line 56-57: Remove the redundant Poetry installation in the runtime
stage by deleting the RUN that executes "python -m pip install poetry
--cache-dir ${PIP_CACHE_DIR}" (the RUN
--mount=type=cache,target=${PIP_CACHE_DIR} ... python -m pip install poetry ...
line) and ensure Poetry is only installed in the build/builder stage as done in
Dockerfile.nestbot.local so the runtime image remains lean; if any runtime steps
expect poetry, adjust them to use artifacts produced in the builder stage
instead.
---
Nitpick comments:
In `@backend/pyproject.toml`:
- Around line 71-75: Mark the nestbot dependency group as optional in
pyproject.toml by setting [tool.poetry.group.nestbot] optional = true (so the
existing [tool.poetry.group.nestbot.dependencies] entries for crewai, langchain,
langchain-community and langgraph are not installed by default), and update
Dockerfile.nestbot.local to run poetry install --with nestbot to ensure the
group is installed in the nestbot Docker build.
In `@docker/backend/Dockerfile.nestbot.local`:
- Around line 56-57: The runtime stage currently installs Poetry with the RUN
--mount=type=cache... python -m pip install poetry step even though the built
.venv is copied from the builder (the runtime only executes python manage.py
runserver and never invokes Poetry); remove that RUN line to slim the image, or
if you intentionally want Poetry in the runtime for interactive docker exec
workflows, keep the line but add a clear comment above it explaining it's only
for developer convenience and not required for runtime execution (refer to the
poetry install RUN and the later COPY of the built .venv into the runtime).
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docker-compose/local/compose.yaml">
<violation number="1" location="docker-compose/local/compose.yaml:29">
P2: Slack disablement relies on the "None" sentinel; empty strings will still initialize Slack and negate the intended “Disable Slack” behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Hey @rudransh-shrivastava CodeRabbit flagged |
|
Hi @Ani07-05 if settings.SLACK_BOT_TOKEN != "None" and settings.SLACK_SIGNING_SECRET != "None" # noqa: S105This is a string |
|
hey @rudransh-shrivastava please do review whenever you can and let me know if I need to change anything else , thanks ! |
rudransh-shrivastava
left a comment
rudransh-shrivastava
left a comment
There was a problem hiding this comment.
The backend still imports crewai and langchain in many places. You need to lazy load them to prevent a crash:
nest-backend | File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
nest-backend | File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
nest-backend | File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
nest-backend | File "<frozen importlib._bootstrap_external>", line 1023, in exec_module
nest-backend | File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
nest-backend | File "/home/owasp/apps/ai/models/__init__.py", line 1, in <module>
nest-backend | from .chunk import Chunk
nest-backend | File "/home/owasp/apps/ai/models/chunk.py", line 4, in <module>
nest-backend | from langchain.text_splitter import RecursiveCharacterTextSplitter
nest-backend | ModuleNotFoundError: No module named 'langchain'
nest-backend exited with code 1
Container nest-backend Error dependency backend failed to start
dependency failed to start: container nest-backend exited (1)
make: *** [Makefile:53: run] Error 1
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docker-compose/local/compose.yaml">
<violation number="1" location="docker-compose/local/compose.yaml:42">
P2: Both backend and nestbot run `python manage.py migrate` at startup. Since the services start independently, they can run migrations concurrently against the same database, which risks race conditions or failed migrations. Only one service should run migrations or startup should be serialized.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docker-compose/local/compose.yaml (1)
28-29:⚠️ Potential issue | 🟠 MajorYAML bare
Noneis parsed asnull, not the string"None".This was raised in a prior review and remains unresolved. In YAML, bare
Noneisnull. Docker Compose treats anullenvironment value as "take from host shell," so if a developer hasDJANGO_SLACK_BOT_TOKENexported in their shell, it leaks into the container.Per PR discussion, the Django code in
apps/slack/apps.pychecks against the string"None". A YAML null won't match that check, so Slack would not be disabled as intended.Use quoted strings to get deterministic behavior:
# Disable Slack in backend - routes won't be registered. - DJANGO_SLACK_BOT_TOKEN: None - DJANGO_SLACK_SIGNING_SECRET: None + DJANGO_SLACK_BOT_TOKEN: "None" + DJANGO_SLACK_SIGNING_SECRET: "None"Or use empty strings if the app also treats empty as disabled.
#!/bin/bash # Check how apps/slack/apps.py evaluates the Slack token to determine if "None" string or null is expected fd "apps.py" --path '*slack*' --exec cat {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose/local/compose.yaml` around lines 28 - 29, The environment values DJANGO_SLACK_BOT_TOKEN and DJANGO_SLACK_SIGNING_SECRET are currently bare None (YAML null) which causes Docker to inherit host envs and won't match the string check in apps/slack/apps.py; update the compose YAML to set those variables as explicit quoted strings (e.g., "None") or as empty quoted strings "" depending on which the Slack initializer expects, ensuring deterministic behavior and preventing accidental host token leakage.
🧹 Nitpick comments (2)
docker/backend/Dockerfile.nestbot.local (2)
46-54:postgresql-clientmay be unnecessary for the nestbot runtime.The nestbot service runs
manage.py runserver— it doesn't invokepsqlor other PostgreSQL CLI tools directly. Database access goes throughlibpq5(the C library), which is already included. Thepostgresql-clientpackage adds ~20 MB and is only useful for debugging.If it's intentionally included for developer convenience during local dev, that's fine — just flagging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/backend/Dockerfile.nestbot.local` around lines 46 - 54, The Dockerfile installs postgresql-client unnecessarily for runtime; remove postgresql-client from the RUN apt-get install list (leave libpq5) or make it optional via a build-arg/DEV_PACKAGES variable so the image for nestbot runtime only includes libpq5; update the RUN line that currently installs "curl libpq5 passwd postgresql-client" to exclude "postgresql-client" (or gate it behind a build-time flag) so the runtime image is smaller while preserving developer convenience when needed.
1-65: Consider sharing the builder stage withDockerfile.localto reduce duplication.This Dockerfile is nearly identical to
Dockerfile.local— the only meaningful differences are:
poetry installincludesnestbotdeps here but excludes them in the backend Dockerfile (line 34 vs--without nestbot).EXPOSE 8001vsEXPOSE 8000.A multi-stage base image (or a shared builder target with a build-arg for the
--withoutflags) would eliminate ~60 lines of duplicated Dockerfile logic and keep both images in sync when OS packages or Poetry configuration change.This was also discussed in the PR comments — rudransh-shrivastava suggested the new Dockerfile can use the main Dockerfile as a base.
Example: parameterize with build args
# In a shared base or using build args: - poetry install --no-root --without dev --without test --without video +ARG POETRY_GROUPS_EXCLUDE="dev test video" +RUN ... poetry install --no-root $(echo $POETRY_GROUPS_EXCLUDE | sed 's/[^ ]*/--without &/g')Or use a shared builder stage that both Dockerfiles extend via
FROM shared-builder AS builder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/backend/Dockerfile.nestbot.local` around lines 1 - 65, The Dockerfile.nestbot.local duplicates the builder stage; extract that builder into a shared stage (e.g., "shared-builder") or a separate base Dockerfile and make the current files use FROM shared-builder AS builder to avoid duplication. Add build ARGs like WITHOUT_FLAGS (default "--without dev --without test --without video") and APP_PORT (default 8001) and use them in the shared builder's poetry install invocation (poetry install --no-root ${WITHOUT_FLAGS}) and in the runtime stage EXPOSE ${APP_PORT}; ensure unique envs/dirs (OWASP_UID/OWASP_GID, POETRY_CACHE_DIR) and the COPY --from=shared-builder behavior remain the same so both Dockerfile.nestbot.local and Dockerfile.local can pass different WITHOUT_FLAGS/APP_PORT values when building.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docker-compose/local/compose.yaml`:
- Around line 28-29: The environment values DJANGO_SLACK_BOT_TOKEN and
DJANGO_SLACK_SIGNING_SECRET are currently bare None (YAML null) which causes
Docker to inherit host envs and won't match the string check in
apps/slack/apps.py; update the compose YAML to set those variables as explicit
quoted strings (e.g., "None") or as empty quoted strings "" depending on which
the Slack initializer expects, ensuring deterministic behavior and preventing
accidental host token leakage.
---
Nitpick comments:
In `@docker/backend/Dockerfile.nestbot.local`:
- Around line 46-54: The Dockerfile installs postgresql-client unnecessarily for
runtime; remove postgresql-client from the RUN apt-get install list (leave
libpq5) or make it optional via a build-arg/DEV_PACKAGES variable so the image
for nestbot runtime only includes libpq5; update the RUN line that currently
installs "curl libpq5 passwd postgresql-client" to exclude "postgresql-client"
(or gate it behind a build-time flag) so the runtime image is smaller while
preserving developer convenience when needed.
- Around line 1-65: The Dockerfile.nestbot.local duplicates the builder stage;
extract that builder into a shared stage (e.g., "shared-builder") or a separate
base Dockerfile and make the current files use FROM shared-builder AS builder to
avoid duplication. Add build ARGs like WITHOUT_FLAGS (default "--without dev
--without test --without video") and APP_PORT (default 8001) and use them in the
shared builder's poetry install invocation (poetry install --no-root
${WITHOUT_FLAGS}) and in the runtime stage EXPOSE ${APP_PORT}; ensure unique
envs/dirs (OWASP_UID/OWASP_GID, POETRY_CACHE_DIR) and the COPY
--from=shared-builder behavior remain the same so both Dockerfile.nestbot.local
and Dockerfile.local can pass different WITHOUT_FLAGS/APP_PORT values when
building.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
backend/Makefilebackend/apps/ai/models/chunk.pybackend/apps/slack/common/handlers/ai.pybackend/pyproject.tomldocker-compose/local/compose.yamldocker/backend/Dockerfiledocker/backend/Dockerfile.localdocker/backend/Dockerfile.nestbot.local
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/pyproject.toml
- docker/backend/Dockerfile
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docker-compose/local/compose.yaml (1)
27-29:⚠️ Potential issue | 🟠 MajorYAML
Noneis parsed asnull, not the string"None".This was flagged in a previous review. Bare
Nonein YAML is a null value. Docker Compose treats null environment values as unset, meaning theenv_filevalue (or host shell value) will take precedence — defeating the intent to disable Slack in the backend.If
backend/apps/slack/apps.pychecks for the string"None", you need to quote it:- DJANGO_SLACK_BOT_TOKEN: None - DJANGO_SLACK_SIGNING_SECRET: None + DJANGO_SLACK_BOT_TOKEN: "None" + DJANGO_SLACK_SIGNING_SECRET: "None"Alternatively, use empty strings (
"") if the app treats empty as disabled.#!/bin/bash # Check how the Slack app handles these env vars fd -t f 'apps.py' -p 'slack' --exec rg -n -C5 'SLACK_BOT_TOKEN\|SLACK_SIGNING_SECRET\|None' {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose/local/compose.yaml` around lines 27 - 29, The YAML currently sets DJANGO_SLACK_BOT_TOKEN and DJANGO_SLACK_SIGNING_SECRET to bare None which YAML parses as null (unset) so env_file/host values can override; change those values to the literal string "None" (quoted) or to an explicit empty string "" depending on how backend/apps/slack/apps.py checks for disablement (it looks for the string "None" in that module), so replace the bare None entries with the quoted value that matches the app's check to ensure Slack is actually disabled.
🧹 Nitpick comments (2)
docker/backend/Dockerfile.nestbot.local (1)
1-65: Consider reducing duplication betweenDockerfile.localandDockerfile.nestbot.local.These two Dockerfiles are nearly identical — they differ only in
poetry installflags (Line 34) andEXPOSEport (Line 60). You could consolidate them using build args:ARG POETRY_EXTRAS="--without nestbot" ARG EXPOSE_PORT=8000 ... RUN poetry install --no-root --without dev --without test --without video ${POETRY_EXTRAS} ... EXPOSE ${EXPOSE_PORT}This is optional since the current approach was explicitly recommended in the PR discussion and is arguably more readable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/backend/Dockerfile.nestbot.local` around lines 1 - 65, The two Dockerfiles duplicate almost all contents; consolidate by adding build args (e.g., ARG POETRY_EXTRAS and ARG EXPOSE_PORT) near the top and use them in the builder stage's RUN poetry install line (replace the hard-coded extra flags in the RUN that calls poetry install) and in the runtime stage's EXPOSE directive (replace the fixed port 8001). Update invocation or CI to pass appropriate POETRY_EXTRAS and EXPOSE_PORT for each variant while keeping USER, WORKDIR, COPY and other steps unchanged.docker-compose/local/compose.yaml (1)
114-146: Nestbot service setup looks solid; consider adding explicit Slack env vars.The service is correctly wired with its own Dockerfile, port 8001, and nestbot-venv volume. However, the Slack-specific environment variables (
DJANGO_SLACK_BOT_TOKEN,DJANGO_SLACK_SIGNING_SECRET) are not explicitly listed in theenvironmentblock — they're inherited implicitly fromenv_file. Since the whole purpose of this service is Slack integration, making these explicit (as the issue specifies) would improve clarity and prevent accidental misconfiguration if.envis incomplete:DJANGO_REDIS_HOST: ${DJANGO_REDIS_HOST:-nest-cache} DJANGO_REDIS_PASSWORD: ${DJANGO_REDIS_PASSWORD:-nest-cache-password} + DJANGO_SLACK_BOT_TOKEN: ${DJANGO_SLACK_BOT_TOKEN} + DJANGO_SLACK_SIGNING_SECRET: ${DJANGO_SLACK_SIGNING_SECRET}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-compose/local/compose.yaml` around lines 114 - 146, Add explicit Slack env vars to the nestbot service environment block: update the nestbot service (service name "nestbot") in the compose YAML to include DJANGO_SLACK_BOT_TOKEN and DJANGO_SLACK_SIGNING_SECRET alongside the existing DJANGO_* and DJANGO_REDIS_* entries so they are documented and can be overridden; keep the existing env_file but add these two keys under the environment section to make Slack credentials explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docker-compose/local/compose.yaml`:
- Around line 27-29: The YAML currently sets DJANGO_SLACK_BOT_TOKEN and
DJANGO_SLACK_SIGNING_SECRET to bare None which YAML parses as null (unset) so
env_file/host values can override; change those values to the literal string
"None" (quoted) or to an explicit empty string "" depending on how
backend/apps/slack/apps.py checks for disablement (it looks for the string
"None" in that module), so replace the bare None entries with the quoted value
that matches the app's check to ensure Slack is actually disabled.
---
Nitpick comments:
In `@docker-compose/local/compose.yaml`:
- Around line 114-146: Add explicit Slack env vars to the nestbot service
environment block: update the nestbot service (service name "nestbot") in the
compose YAML to include DJANGO_SLACK_BOT_TOKEN and DJANGO_SLACK_SIGNING_SECRET
alongside the existing DJANGO_* and DJANGO_REDIS_* entries so they are
documented and can be overridden; keep the existing env_file but add these two
keys under the environment section to make Slack credentials explicit.
In `@docker/backend/Dockerfile.nestbot.local`:
- Around line 1-65: The two Dockerfiles duplicate almost all contents;
consolidate by adding build args (e.g., ARG POETRY_EXTRAS and ARG EXPOSE_PORT)
near the top and use them in the builder stage's RUN poetry install line
(replace the hard-coded extra flags in the RUN that calls poetry install) and in
the runtime stage's EXPOSE directive (replace the fixed port 8001). Update
invocation or CI to pass appropriate POETRY_EXTRAS and EXPOSE_PORT for each
variant while keeping USER, WORKDIR, COPY and other steps unchanged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
backend/Makefilebackend/apps/ai/Makefilebackend/apps/ai/models/chunk.pybackend/apps/slack/common/handlers/ai.pybackend/pyproject.tomldocker-compose/local/compose.yamldocker/backend/Dockerfiledocker/backend/Dockerfile.localdocker/backend/Dockerfile.nestbot.local
🚧 Files skipped from review as they are similar to previous changes (1)
- docker/backend/Dockerfile
rudransh-shrivastava
dismissed stale reviews from coderabbitai[bot] and themself
via
90c1b9e
|
2 participants
Proposed change
Resolves #3865
Decouples NestBot Slack integration into a separate Docker container for independent scaling and deployment.
Key Changes
nestbotservice on port 8001 with Slack integrationdocsservice to port 8002 to avoid conflictspython:3.13-slim)Checklist
make check-testlocally: all warnings addressed, tests passed