Skip to content

Conversation

@leoisadev8
Copy link
Member

Summary

  • pass the nested ElectricSQL port flag so new releases listen on 3010 as expected
  • update production env example to include ELECTRIC__HTTP__PORT and correct service URL
  • drop outdated Railway deployment doc since we deploy via Dokploy/docker-compose

Testing

  • docker-compose config lint (implicit)
  • manual verification that env defaults now contain ELECTRIC__HTTP__PORT

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR updates ElectricSQL HTTP port configuration to support newer releases that use nested environment variables. The changes add the ELECTRIC__HTTP__PORT variable (with double underscore) alongside the legacy ELECTRIC_HTTP_PORT to ensure compatibility across ElectricSQL versions. In docker-compose.yml, both variables are set to resolve from the same environment value, defaulting to port 3010. The env.example file is updated to include both variants and corrects the ELECTRIC_SERVICE_URL from port 3000 to 3010. Additionally, the outdated Railway deployment documentation is removed since the project now standardizes on Dokploy/docker-compose for deployments. This aligns with the repository's docker-compose.yml anchor pattern (x-required-db-env) that enforces environment variable consistency across services.


Changed Files
Filename Score Overview
docs/deployment/railway.md 5/5 Complete removal of outdated Railway deployment guide - clean documentation cleanup
env.example 4/5 Adds nested ELECTRIC__HTTP__PORT variable and corrects service URL from port 3000 to 3010
docker-compose.yml 4/5 Adds ELECTRIC__HTTP__PORT environment variable for ElectricSQL service compatibility

Confidence score: 4/5

  • This PR is safe to merge with low risk of production issues
  • Score reflects solid configuration hygiene but deducted one point for: (1) potential configuration drift from maintaining dual variables without clear deprecation path, (2) the expose directive still references only the legacy variable name which could cause confusion, (3) missing validation that downstream services/health checks have been updated to reference port 3010 instead of 3000
  • Pay close attention to env.example and docker-compose.yml to verify both variables consistently default to 3010 and that no services still hardcode references to port 3000

Individual issue scores:

  1. Configuration drift from dual variables (3/5 correctness score): Maintaining both ELECTRIC_HTTP_PORT and ELECTRIC__HTTP__PORT without a documented deprecation timeline or migration guide could lead to users setting only one variable and experiencing unexpected behavior. The expose directive still uses ${ELECTRIC_HTTP_PORT:-3010}, creating asymmetry.

  2. Port mismatch risk (4/5 correctness score): While the PR fixes the env.example service URL from 3000 to 3010, there's no evidence in the diff that downstream health checks or service discovery logic have been audited to ensure they no longer reference the old port.

  3. Missing version documentation (3/5 correctness score): The PR body mentions "new releases" of ElectricSQL but doesn't specify which version introduced the nested variable format. This makes it difficult for operators to understand when they can safely drop the legacy variable.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Compose as docker-compose.yml
    participant Postgres as PostgreSQL Container
    participant Electric as ElectricSQL Container
    participant Server as Server Container
    participant Web as Web Container

    Dev->>Compose: "docker-compose up"
    Compose->>Postgres: "Start with wal_level=logical"
    
    activate Postgres
    Postgres->>Postgres: "Initialize database & shadow DB"
    Postgres->>Postgres: "Run healthcheck (pg_isready)"
    Postgres-->>Compose: "Healthy"
    deactivate Postgres
    
    Compose->>Electric: "Start (depends on Postgres healthy)"
    activate Electric
    Electric->>Electric: "Read ELECTRIC__HTTP__PORT=${ELECTRIC_HTTP_PORT:-3010}"
    Electric->>Electric: "Bind HTTP on 0.0.0.0:3010"
    Electric->>Electric: "Bind PG_PROXY on 0.0.0.0:5133"
    Electric-->>Compose: "Started"
    deactivate Electric
    
    Compose->>Server: "Build & start (depends on Postgres & Electric)"
    activate Server
    Server->>Server: "Wait for Postgres connection"
    Server->>Server: "Run migrations (if RUN_DB_MIGRATIONS=1)"
    Server->>Electric: "Connect to ELECTRIC_SERVICE_URL=http://openchat-electric:3010"
    Server->>Server: "Start API on port 3000"
    Server-->>Compose: "Started"
    deactivate Server
    
    Compose->>Web: "Build & start (depends on Server)"
    activate Web
    Web->>Web: "Read NEXT_PUBLIC_ELECTRIC_URL=http://openchat-electric:3010"
    Web->>Web: "Start Next.js on port 3001"
    Web-->>Compose: "Started"
    deactivate Web
    
    Dev->>Web: "Access application"
    Web->>Server: "API requests"
    Server->>Electric: "Sync operations via port 3010"
    Electric->>Postgres: "Logical replication queries"
    Postgres-->>Electric: "Data changes"
    Electric-->>Server: "Real-time updates"
    Server-->>Web: "Response with sync data"
    Web-->>Dev: "Render UI"
Loading

Additional Comments (1)

  1. docker-compose.yml, line 54 (link)

    logic: expose directive references ${ELECTRIC_HTTP_PORT:-3010}, not ${ELECTRIC__HTTP__PORT:-3010}. If ElectricSQL 1.1.13 only reads the nested variable (line 48) and a future user removes the legacy ELECTRIC_HTTP_PORT (line 47), the container port exposure will break.

    Issue score: 3/5 – medium risk. Recommend aligning this to the canonical variable once you confirm which format ElectricSQL 1.1.13 requires. For now, the legacy variable is still set, so no immediate breakage. Does ElectricSQL 1.1.13 require ELECTRIC_HTTP_PORT, ELECTRIC__HTTP__PORT, or both? This will determine which variable should drive the expose directive.

Context used:

  • Context from dashboard - AGENTS.md (source)

3 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

ELECTRIC_INSECURE: ${ELECTRIC_INSECURE:-false}
ELECTRIC_LOG_LEVEL: ${ELECTRIC_LOG_LEVEL:-info}
ELECTRIC_HTTP_PORT: ${ELECTRIC_HTTP_PORT:-3010}
ELECTRIC__HTTP__PORT: ${ELECTRIC_HTTP_PORT:-3010}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: both ELECTRIC_HTTP_PORT and ELECTRIC__HTTP__PORT resolve to the same environment variable ${ELECTRIC_HTTP_PORT:-3010}. If a user overrides only ELECTRIC__HTTP__PORT in their .env, the legacy ELECTRIC_HTTP_PORT on line 47 will still default to 3010, potentially causing confusion.

Issue score: 2/5 – low risk in practice (most users set one variable), but the fallback chain isn't symmetric. Consider using separate variable names or documenting the precedence. Should ELECTRIC__HTTP__PORT read from a distinct env var (e.g., ${ELECTRIC__HTTP__PORT:-3010}) to allow independent override, or is the shared fallback intentional?

@leoisadev8 leoisadev8 merged commit f20fff4 into main Oct 19, 2025
4 checks passed
@leoisadev8 leoisadev8 deleted the fix/electric-http-port branch November 2, 2025 23:33
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