Begin moving to postgres spilo + adding pgvector #8309

FelixMalfait · 2024-11-04T15:43:46Z

We will remove the twenty-postgres image that was used for local development and only use twenty-postgres-pilo (which we use in prod), bringing the development environment closer to prod and avoiding having to maintain 2 images.

Instead of provisioning the super user after the db initialization, we directly rely on the superuser provided by Spilo for simplicity. We also introduce a change that tries to create the right database (default or test) based on the context.

How to test:

docker build -t twentycrm/twenty-postgres-spilo:latest -f ./packages/twenty-docker/twenty-postgres-spilo/Dockerfile .
docker images --no-trunc | grep twenty-postgres-spilo
postgres-on-docker:
	docker run \
	--name twenty_pg \
	-e PGUSER_SUPERUSER=twenty \
	-e PGPASSWORD_SUPERUSER=twenty \
	-e ALLOW_NOSSL=true \
	-v twenty_db_data:/home/postgres/pgdata \
	-p 5432:5432 \
	REPLACE_WITH_IMAGE_ID

github-actions · 2024-11-13T15:02:24Z

	Warnings
⚠️	Changes were made to the environment variables, but not to the documentation - Please review your changes and check if a change needs to be documented!

Generated by 🚫 dangerJS against 0c37a83

greptile-apps

PR Summary

This PR transitions from a custom PostgreSQL image to Spilo PostgreSQL across development and production environments, standardizing database management and adding pgvector support.

Replaces POSTGRES_ADMIN_PASSWORD with PGUSER_SUPERUSER/PGPASSWORD_SUPERUSER across all configurations to align with Spilo's environment variables
Changes database volume mount path from /var/lib/postgresql/data to /home/postgres/pgdata in Docker and K8s configurations
Adds vector extension support in setup-db.ts for pgvector functionality
Modifies database connection strings to use postgres user instead of twenty across all environments
Preserves critical schemas (metric_helpers, user_management, public) during database truncation

_{28 file(s) reviewed, 20 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-11-15T08:39:12Z

.github/workflows/ci-server.yaml

+      - name: Server / Create DB
+        if: steps.changed-files.outputs.any_changed == 'true'
+        run: |
+          PGPASSWORD=twenty psql -h localhost -p 5432 -U postgres -d postgres -c 'CREATE DATABASE "default";'
+          PGPASSWORD=twenty psql -h localhost -p 5432 -U postgres -d postgres -c 'CREATE DATABASE "test";'


logic: Database creation step should be moved before worker run but after environment setup to ensure proper initialization order

greptile-apps · 2024-11-15T08:39:13Z

.github/workflows/ci-server.yaml

    services:
      postgres:
-        image: twentycrm/twenty-postgres
+        image: twentycrm/twenty-postgres-spilo
        env:
-          POSTGRES_PASSWORD: postgres
-          POSTGRES_USER: postgres
+          PGUSER_SUPERUSER: postgres
+          PGPASSWORD_SUPERUSER: twenty
+          ALLOW_NOSSL: "true"
+          SPILO_PROVIDER: "local"
        ports:
          - 5432:5432
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5


logic: Missing database creation step in server-integration-test job that's present in server-setup job

greptile-apps · 2024-11-15T08:39:53Z

.github/workflows/ci-test-docker-compose.yaml

+                docker compose logs db
+                exit 1
+            fi
+            echo "Still waiting for database... (${count}/60)"


logic: Message shows '(${count}/60)' but timeout is 300 - inconsistent messaging

greptile-apps · 2024-11-15T08:39:54Z

.github/workflows/ci-test-docker-compose.yaml

        docker compose logs db server -f &
        pid=$!


logic: Background process pid is never killed if containers start successfully - potential resource leak

greptile-apps · 2024-11-15T08:40:27Z

.github/workflows/ci-website.yaml

@@ -37,16 +44,20 @@ jobs:
        if: steps.changed-files.outputs.changed == 'true' 
        uses: ./.github/workflows/actions/yarn-install

+      - name: Server / Create DB
+        if: steps.changed-files.outputs.any_changed == 'true'


logic: Condition uses any_changed but previous steps use changed - this inconsistency could cause the database creation to run when it shouldn't or vice versa

greptile-apps · 2024-11-15T08:45:51Z

packages/twenty-docker/k8s/manifests/deployment-worker.yaml

            - name: PG_DATABASE_URL
-              value: "postgres://twenty:[email protected]/default"
+              value: "postgres://postgres:[email protected]/default"


logic: Ensure this connection string matches the credentials configured in the Spilo database deployment. The superuser password should be securely stored and accessed via a secret rather than hardcoded.

greptile-apps · 2024-11-15T08:46:25Z

packages/twenty-docker/k8s/terraform/README.md

 | <a name="input_twentycrm_app_hostname"></a> [twentycrm\_app\_hostname](#input\_twentycrm\_app\_hostname) | The protocol, DNS fully qualified hostname, and port used to access TwentyCRM in your environment. Ex: https://crm.example.com:443 | `string` | n/a | yes |
 | <a name="input_twentycrm_pgdb_admin_password"></a> [twentycrm\_pgdb\_admin\_password](#input\_twentycrm\_pgdb\_admin\_password) | TwentyCRM password for postgres database. | `string` | n/a | yes |


style: check if pgdb_admin_password variable needs to be renamed to match new Spilo environment variable PGPASSWORD_SUPERUSER

greptile-apps · 2024-11-15T08:51:01Z

packages/twenty-docker/twenty/entrypoint.sh

+    PGUSER=$(echo $PG_DATABASE_URL | awk -F '//' '{print $2}' | awk -F ':' '{print $1}')
+    PGPASS=$(echo $PG_DATABASE_URL | awk -F ':' '{print $3}' | awk -F '@' '{print $1}')
+    PGHOST=$(echo $PG_DATABASE_URL | awk -F '@' '{print $2}' | awk -F ':' '{print $1}')
+    PGPORT=$(echo $PG_DATABASE_URL | awk -F ':' '{print $4}' | awk -F '/' '{print $1}')


logic: URL parsing with awk is fragile - will break if credentials contain special characters or if URL format changes. Consider using a more robust parsing method.

greptile-apps · 2024-11-15T08:51:02Z

packages/twenty-docker/twenty/entrypoint.sh

+    PGPASSWORD=${PGPASS} psql -h ${PGHOST} -p ${PGPORT} -U ${PGUSER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'default'" | grep -q 1 || \
+    PGPASSWORD=${PGPASS} psql -h ${PGHOST} -p ${PGPORT} -U ${PGUSER} -d postgres -c "CREATE DATABASE \"default\""


logic: Missing error handling if psql commands fail. Script continues even if database creation fails.

greptile-apps · 2024-11-15T08:54:33Z

packages/twenty-server/scripts/truncate-db.ts

+      if (
+        schema.schema_name === 'metric_helpers' ||
+        schema.schema_name === 'user_management' ||
+        schema.schema_name === 'public'
+      ) {
+        continue;
+      }


style: Consider making protected schemas configurable via environment variables rather than hardcoding them

FelixMalfait added 3 commits November 4, 2024 16:43

Begin postgres spilo

4fbb514

More cleanup

c88e176

Fix extensions

9bf9be0

FelixMalfait changed the title ~~Begin moving to postgres spilo + adding pgvector/hydra~~ Begin moving to postgres spilo + adding pgvector Nov 6, 2024

FelixMalfait added 3 commits November 13, 2024 12:54

Revert

dfda7e9

Fix password

6e70ce4

Lets juste use the superuser

a5ccee3

FelixMalfait added 5 commits November 13, 2024 17:45

Merge branch 'main' into move-to-postgres-spilo

edccbcf

Update timeout

2850fae

Create test DB in CI

c8792a2

Update docker compose

828d4c3

Update server CI

4af3211

charlesBochet added the -PR: awaiting author label Nov 13, 2024

charlesBochet approved these changes Nov 14, 2024

View reviewed changes

FelixMalfait added 14 commits November 14, 2024 14:21

Update server CI

e1b828e

CI

9b24a8e

Change user to postgres

f13db8d

Fix user

1fbcfaf

Add more loging

9035a09

Fix

fd5c301

Fix

64736a6

CI

5912f91

CI

bee8aab

Add psql client

5323a4a

Fix entrypoint

541edde

Password

e0b1aa9

Fix

832d609

Fix

0c37a83

FelixMalfait marked this pull request as ready for review November 15, 2024 08:38

FelixMalfait merged commit 736635a into main Nov 15, 2024
19 checks passed

FelixMalfait deleted the move-to-postgres-spilo branch November 15, 2024 08:38

greptile-apps bot reviewed Nov 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Begin moving to postgres spilo + adding pgvector #8309

Begin moving to postgres spilo + adding pgvector #8309

FelixMalfait commented Nov 4, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

greptile-apps bot Nov 15, 2024

		\| <a name="input_twentycrm_app_hostname"></a> [twentycrm\_app\_hostname](#input\_twentycrm\_app\_hostname) \| The protocol, DNS fully qualified hostname, and port used to access TwentyCRM in your environment. Ex: https://crm.example.com:443 \| `string` \| n/a \| yes \|
		\| <a name="input_twentycrm_pgdb_admin_password"></a> [twentycrm\_pgdb\_admin\_password](#input\_twentycrm\_pgdb\_admin\_password) \| TwentyCRM password for postgres database. \| `string` \| n/a \| yes \|

		PGPASSWORD=${PGPASS} psql -h ${PGHOST} -p ${PGPORT} -U ${PGUSER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'default'" \| grep -q 1 \|\| \
		PGPASSWORD=${PGPASS} psql -h ${PGHOST} -p ${PGPORT} -U ${PGUSER} -d postgres -c "CREATE DATABASE \"default\""

Begin moving to postgres spilo + adding pgvector #8309

Begin moving to postgres spilo + adding pgvector #8309

Conversation

FelixMalfait commented Nov 4, 2024 • edited Loading

github-actions bot commented Nov 13, 2024 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 15, 2024

Choose a reason for hiding this comment

FelixMalfait commented Nov 4, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading