Skip to content

add local email service docker compse#888

Open
theothersideofgod wants to merge 2 commits intomainfrom
feat/add-local-email-service-docker-compse
Open

add local email service docker compse#888
theothersideofgod wants to merge 2 commits intomainfrom
feat/add-local-email-service-docker-compse

Conversation

@theothersideofgod
Copy link
Copy Markdown
Contributor

Add Local Email Service Docker Compose

with Mailpit

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a dedicated Docker Compose stack for local email testing in development using Mailpit, alongside the existing Constructive admin API, send-email-link function, and knative-job-service.

Changes:

  • Introduces docker-compose.local-email.yml to run Mailpit (SMTP + Web UI).
  • Configures send-email-link to send via SMTP to Mailpit (instead of provider-based sending).
  • Includes the jobs runtime (knative-job-service) to poll app_jobs and invoke the function.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PGDATABASE: constructive
JOBS_SCHEMA: app_jobs
# Worker config
JOBS_SUPPORT_ANY: "true"
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JOBS_SUPPORT_ANY is set to "true" while JOBS_SUPPORTED is also set. When JOBS_SUPPORT_ANY is true, the knative job worker/scheduler will poll for and attempt to execute all task identifiers in app_jobs (ignoring the supported list), which can cause repeated failures if other job types exist. Set JOBS_SUPPORT_ANY to "false" here to ensure only send-email-link is processed (matching the intent of this compose file).

Suggested change
JOBS_SUPPORT_ANY: "true"
JOBS_SUPPORT_ANY: "false"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding JOBS_SUPPORT_ANY = true:

Keeping it as true is intentional for extensibility. When adding new job services in the future (e.g., send-sms), we only need to update INTERNAL_GATEWAY_DEVELOPMENT_MAP - no need to maintain two separate configs.

If an unconfigured job type appears in app_jobs, the error serves as a reminder to add the gateway mapping.

Comment on lines +40 to +41
# Postgres - use host.docker.internal to connect to local postgres
PGHOST: host.docker.internal
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PGHOST: host.docker.internal is not resolvable by default on many Linux Docker setups (and differs from the repo’s other compose stacks which use the postgres service name on constructive-net). If this stack is meant to work with the postgres container started by docker-compose.yml, set PGHOST to postgres; if it’s meant to connect to a host-installed Postgres, add the required extra_hosts: ["host.docker.internal:host-gateway"] (or document the Docker Desktop requirement).

Suggested change
# Postgres - use host.docker.internal to connect to local postgres
PGHOST: host.docker.internal
# Postgres - use postgres service on constructive-net
PGHOST: postgres

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added extra_hosts: ["host.docker.internal:host-gateway"] to support Linux Docker Engine.

macOS/Windows Docker Desktop will ignore this setting (already has built-in support), so no impact on existing usage.

Comment on lines +97 to +102
# Postgres - use host.docker.internal to connect to local postgres
PGHOST: host.docker.internal
PGPORT: "5432"
PGUSER: ${PGUSER:-postgres}
PGPASSWORD: ${PGPASSWORD:-password}
PGDATABASE: constructive
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as above: PGHOST: host.docker.internal can break on Linux and is inconsistent with the docker-compose.yml Postgres container workflow. Consider using PGHOST=postgres when relying on the shared constructive-net, or add an extra_hosts mapping for host.docker.internal if connecting to host Postgres is intentional.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added extra_hosts: ["host.docker.internal:host-gateway"] to support Linux Docker Engine.

macOS/Windows Docker Desktop will ignore this setting (already has built-in support), so no impact on existing usage.

Comment on lines +29 to +32
constructive-admin-server:
container_name: constructive-admin-server
image: constructive:dev
entrypoint: ["constructive", "server", "--host", "0.0.0.0", "--port", "3000", "--origin", "*"]
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This compose file pins container_name for constructive-admin-server (and also send-email-link / knative-job-service below). Because these names are also used in docker-compose.jobs.yml, running both stacks (or reusing containers across projects) will fail with container-name conflicts. Consider removing container_name so Compose can namespace by project, or prefix these names (e.g., local-email-...) and update internal URLs accordingly.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added extra_hosts: ["host.docker.internal:host-gateway"] to support Linux Docker Engine.

macOS/Windows Docker Desktop will ignore this setting (already has built-in support), so no impact on existing usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants