Restore forgot-password email delivery in Docker deployments#236
Restore forgot-password email delivery in Docker deployments#236Atlas-SZ wants to merge 1 commit intodataelement:mainfrom
Conversation
The Docker Compose backend service was not inheriting the SMTP and reset-password settings from .env, and the email background-task bridge tried to create an asyncio task from a Starlette threadpool worker. Together those two issues produced a success response without a delivered reset email. This change injects the .env file into the backend container and makes async email jobs runnable both with and without an active event loop. It also adds a regression test for the no-running-loop path. Constraint: Docker Compose only interpolates .env by default; it does not inject arbitrary keys into container environments Rejected: Execute reset emails inline in the request path | adds avoidable latency and couples delivery to response timing Rejected: Change forgot-password success masking behavior | out of scope and would alter account-enumeration protections Confidence: high Scope-risk: narrow Directive: Keep async email jobs compatible with both request event loops and Starlette threadpool background tasks Related: dataelement#235 Tested: cd backend && .venv/bin/python -m pytest tests/test_password_reset_and_notifications.py -k 'run_background_email_job_executes_awaitable_without_running_loop or hides_email_delivery_failures or send_system_email_uses_configured_timeout' -q Tested: docker compose config | rg 'SYSTEM_EMAIL_FROM_ADDRESS|PUBLIC_BASE_URL|PASSWORD_RESET_TOKEN_EXPIRE_MINUTES' Not-tested: Full backend test suite Not-tested: Live forgot-password flow against deployed Docker backend
|
已实测,合并前辛苦再测一下。 |
|
Thanks for the thorough analysis and the well-structured PR — the root cause diagnosis is accurate for the architecture you were working against. After reviewing our current codebase, both issues have already been addressed through a different architectural approach: Bug 1 (SMTP env vars not injected into Docker): Bug 2 (async email job / no running event loop): Because both fixes are already covered by the current implementation, we will close this PR. We appreciate the effort and the regression test — the test structure is clean and the PR description is a good reference. |
Fixes #235
Summary
.envmail/reset settings into the Dockerbackendservice withenv_fileRoot Cause
There were two separate issues affecting forgot-password in Docker deployments:
docker-compose.ymldid not inject the SMTP and reset-password related values from.envinto thebackendcontainer, so the backend could start without the expectedmail configuration.
asyncio.create_task()from a Starlette threadpool worker, which raisedRuntimeError: no running event loopand prevented the mail from being sent even though the API returned a success response.Changes
env_file: ./.envto the Dockerbackendservicerun_background_email_job()to:asyncio.run()when no running event loop existsTest Plan
cd backend && .venv/bin/python -m pytest tests/test_password_reset_and_notifications.py -k 'run_background_email_job_executes_awaitable_without_running_loop or hides_email_delivery_failures or send_system_email_uses_configured_timeout' -qdocker compose config | rg 'SYSTEM_EMAIL_FROM_ADDRESS|PUBLIC_BASE_URL|PASSWORD_RESET_TOKEN_EXPIRE_MINUTES'Notes