Refactor Redis connections to use Redis URL - closes #7421 #7736

thomasmol · 2024-10-16T07:52:43Z

This pull request consolidates Redis connection parameters into a single REDIS_URL environment variable across various configuration files and code modules. The most important changes include updates to environment variable files, Docker and Kubernetes configurations, and code modules to utilize the new REDIS_URL format.

Environment Variable Updates:

packages/twenty-docker/.env.example: Replaced REDIS_HOST and REDIS_PORT with REDIS_URL.
packages/twenty-server/.env.example: Replaced REDIS_HOST, REDIS_PORT, REDIS_USERNAME, and REDIS_PASSWORD with REDIS_URL.
packages/twenty-server/.env.test: Replaced REDIS_HOST, REDIS_PORT, REDIS_USERNAME, and REDIS_PASSWORD with REDIS_URL.

Docker and Kubernetes Configuration Updates:

packages/twenty-docker/docker-compose.yml: Replaced REDIS_HOST and REDIS_PORT with REDIS_URL in multiple service definitions. [1] [2]
packages/twenty-docker/k8s/manifests/deployment-server.yaml: Replaced REDIS_HOST and REDIS_PORT with REDIS_URL.
packages/twenty-docker/k8s/manifests/deployment-worker.yaml: Replaced REDIS_HOST and REDIS_PORT with REDIS_URL.
packages/twenty-docker/k8s/terraform/deployment-server.tf: Replaced REDIS_HOST and REDIS_PORT with REDIS_URL.
packages/twenty-docker/k8s/terraform/deployment-worker.tf: Replaced REDIS_HOST and REDIS_PORT with REDIS_URL.

Code Module Updates:

packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts: Replaced REDIS_HOST and REDIS_PORT with REDIS_URL.
packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts: Added validation for REDIS_URL.
packages/twenty-server/src/engine/core-modules/message-queue/message-queue.module-factory.ts: Replaced REDIS_HOST, REDIS_PORT, REDIS_USERNAME, and REDIS_PASSWORD with REDIS_URL. [1] [2]

Documentation Updates:

packages/twenty-website/src/content/developers/self-hosting/self-hosting-var.mdx: Updated documentation to reflect the change to REDIS_URL.
packages/twenty-website/src/content/developers/self-hosting/upgrade-guide.mdx: Added upgrade instructions for the new REDIS_URL variable.

greptile-apps

PR Summary

This PR consolidates Redis connection parameters into a single REDIS_URL environment variable across various configuration files and code modules, simplifying Redis connection handling and potentially enabling IPv6 support.

Replaced separate Redis connection parameters (host, port, username, password) with REDIS_URL in environment files and Kubernetes configurations
Updated cache-storage.module-factory.ts and message-queue.module-factory.ts to use REDIS_URL for connection
Modified environment-variables.ts to validate the new REDIS_URL format
Updated self-hosting documentation and upgrade guide to reflect the REDIS_URL change
Ensured consistency across Docker Compose, Kubernetes, and Terraform configurations

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

greptile-apps · 2024-10-16T07:53:16Z

packages/twenty-docker/.env.example

@@ -5,8 +5,7 @@ TAG=latest
 PG_DATABASE_HOST=db:5432

 SERVER_URL=http://localhost:3000
-# REDIS_HOST=redis
-# REDIS_PORT=6379
+# REDIS_URL=redis://localhost:6379


style: Add a comment explaining the REDIS_URL format for clarity

greptile-apps · 2024-10-16T07:53:47Z

packages/twenty-docker/docker-compose.yml

@@ -25,8 +25,7 @@ services:
      PG_DATABASE_URL: postgres://twenty:twenty@${PG_DATABASE_HOST}/default
      SERVER_URL: ${SERVER_URL}
      FRONT_BASE_URL: ${FRONT_BASE_URL:-$SERVER_URL}
-      REDIS_PORT: ${REDIS_PORT:-6379}
-      REDIS_HOST: ${REDIS_HOST:-redis}
+      REDIS_URL: ${REDIS_URL:-redis://localhost:6379}


style: Consider using the redis service name instead of localhost in the default URL

greptile-apps · 2024-10-16T07:55:24Z

packages/twenty-docker/k8s/terraform/deployment-server.tf

-            name  = "REDIS_PORT"
-            value = 6379
+            name  = "REDIS_URL"
+            value = "redis://${kubernetes_service.twentycrm_redis.metadata.0.name}.${kubernetes_namespace.twentycrm.metadata.0.name}.svc.cluster.local:6379"


logic: The Redis URL format doesn't include authentication. If Redis requires auth, update the URL to include username and password

greptile-apps · 2024-10-16T07:56:05Z

packages/twenty-docker/k8s/terraform/deployment-worker.tf

+            name  = "REDIS_URL"
+            value = "redis://${kubernetes_service.twentycrm_redis.metadata.0.name}.${kubernetes_namespace.twentycrm.metadata.0.name}.svc.cluster.local:6379"


style: Consider using a separate variable for the Redis URL instead of constructing it inline. This would improve maintainability and allow for easier configuration changes.

greptile-apps · 2024-10-16T07:57:45Z

packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts

        throw new Error(
-          `${cacheStorageType} cache storage requires host: ${host} and port: ${port} to be defined, check your .env file`,
+          `${cacheStorageType} cache storage requires ${connectionString} to be defined, check your .env file`,


logic: Error message uses connectionString variable, which could be undefined. Use 'REDIS_URL' string instead

greptile-apps · 2024-10-16T07:58:59Z

packages/twenty-server/src/engine/core-modules/message-queue/message-queue.module-factory.ts

@@ -3,6 +3,7 @@ import {
  MessageQueueDriverType,
  MessageQueueModuleOptions,
 } from 'src/engine/core-modules/message-queue/interfaces';
+import IORedis from 'ioredis';


style: Consider adding a comment explaining why IORedis is being imported and used

greptile-apps · 2024-10-16T07:58:59Z

packages/twenty-server/src/engine/core-modules/message-queue/message-queue.module-factory.ts

-      const port = environmentService.get('REDIS_PORT');
-      const username = environmentService.get('REDIS_USERNAME');
-      const password = environmentService.get('REDIS_PASSWORD');
+      const connectionString = environmentService.get('REDIS_URL');


logic: Validate REDIS_URL before using it to prevent potential runtime errors

charlesBochet

Clean change! I'm testing that it's working as expected locally

charlesBochet · 2024-10-16T08:15:43Z

When I don't have the environment variable REDIS_URL set in my .env, I'm getting: ERROR [ExceptionHandler] redis cache storage requires undefined to be defined, check your .env file

packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts

charlesBochet · 2024-10-16T08:17:33Z

@thomasmol could you made the fix mentioned above and we are good to go :)

charlesBochet · 2024-10-16T08:18:33Z

Actually, can you try to also leverage the @ValidateIf decorator in environment-variables.ts, it's our recommended way to ask for an env variable to be set

charlesBochet · 2024-10-16T08:23:09Z

Also, when I run npx nx run twenty-server:command cache:flush, the command seems to be hanging forever

charlesBochet · 2024-10-16T08:26:55Z

And the CI is red:

linter
a more serious issue (the server does not seem to properly boot on the CI)

charlesBochet · 2024-10-16T08:28:07Z

packages/twenty-server/src/engine/core-modules/message-queue/message-queue.module-factory.ts

-            username,
-            password,
-          },
+          connection: new IORedis(connectionString)


this seems wrong actually, we should not instanciate here

true, i changed it to just pass the connection string here. It works but there is a type error. Unsure why that is since passing a string should be OK as per: https://redis.github.io/ioredis/classes/Redis.html#constructor

thomasmol · 2024-10-16T10:18:42Z

changed:

@ValidateIf decorator in environment-variables.ts, only validate if cache or message queue is set to Redis
fixed linting
flushing cache command does not hang anymore
validation error message updated
passing of connection string updated, but results in type error:

Do you know of a fix for the type error in message-queue.module-factory.ts @charlesBochet ?

Updating the bullmq factory options interface to

export interface BullMQDriverFactoryOptions {
 type: MessageQueueDriverType.BullMQ;
 options: BullMQDriverOptions & { connection: string };
}

would work

charlesBochet · 2024-10-16T12:37:33Z

packages/twenty-server/src/engine/core-modules/message-queue/message-queue.module-factory.ts

-            username,
-            password,
-          },
+          connection: connectionString as any,


let's get rid of this any type
@Weiko is taking a look

charlesBochet

LGTM to be merged once typing issue has been solved

Weiko · 2024-10-16T13:14:56Z

@charlesBochet tried to fix typing but couldn't find a good solution, seems bullmq redis option works with a string but typing does not accept string. I've updated our custom type to avoid type assertion but this is not the perfect solution. Ideally we should open an issue on bullmq repo

Weiko · 2024-10-16T14:22:52Z

/award 300

oss-gg · 2024-10-16T14:22:56Z

Awarding thomasmol: 300 points 🕹️ Well done! Check out your new contribution on oss.gg/thomasmol

FelixMalfait · 2024-10-16T15:41:18Z

Thanks a lot @thomasmol!

thomasmol added 3 commits October 16, 2024 09:48

Refactor Redis connection to use Redis URL

158ab70

fix

4d15012

fixes

0ec9d12

greptile-apps bot reviewed Oct 16, 2024

View reviewed changes

charlesBochet added the -PR: awaiting review label Oct 16, 2024

charlesBochet self-assigned this Oct 16, 2024

charlesBochet approved these changes Oct 16, 2024

View reviewed changes

charlesBochet reviewed Oct 16, 2024

View reviewed changes

packages/twenty-server/src/engine/core-modules/cache-storage/cache-storage.module-factory.ts Outdated Show resolved Hide resolved

charlesBochet added -PR: awaiting author and removed -PR: awaiting review labels Oct 16, 2024

charlesBochet requested changes Oct 16, 2024

View reviewed changes

add fixes

655265c

charlesBochet reviewed Oct 16, 2024

View reviewed changes

charlesBochet approved these changes Oct 16, 2024

View reviewed changes

Weiko and others added 2 commits October 16, 2024 15:10

fix typing

2ed4704

Fix docker-compose CI

9112b6c

fix

68c8612

Fix

551f789

charlesBochet merged commit ba2ee0d into twentyhq:main Oct 16, 2024
8 checks passed

stnby mentioned this pull request Oct 17, 2024

When deploying the application in Docker (locally, on cloud Linux, or in Render), both the worker and server containers fail to connect to Redis, resulting in an ECONNREFUSED error. #7768

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Redis connections to use Redis URL - closes #7421 #7736

Refactor Redis connections to use Redis URL - closes #7421 #7736

thomasmol commented Oct 16, 2024

greptile-apps bot left a comment

greptile-apps bot Oct 16, 2024

greptile-apps bot Oct 16, 2024

greptile-apps bot Oct 16, 2024

greptile-apps bot Oct 16, 2024

greptile-apps bot Oct 16, 2024

greptile-apps bot Oct 16, 2024

greptile-apps bot Oct 16, 2024

charlesBochet left a comment

charlesBochet commented Oct 16, 2024 •

edited

Loading

charlesBochet commented Oct 16, 2024

charlesBochet commented Oct 16, 2024

charlesBochet commented Oct 16, 2024 •

edited

Loading

charlesBochet commented Oct 16, 2024

charlesBochet Oct 16, 2024

thomasmol Oct 16, 2024

thomasmol commented Oct 16, 2024 •

edited

Loading

charlesBochet Oct 16, 2024

charlesBochet left a comment

Weiko commented Oct 16, 2024

Weiko commented Oct 16, 2024

oss-gg bot commented Oct 16, 2024

FelixMalfait commented Oct 16, 2024

		name = "REDIS_URL"
		value = "redis://${kubernetes_service.twentycrm_redis.metadata.0.name}.${kubernetes_namespace.twentycrm.metadata.0.name}.svc.cluster.local:6379"

Refactor Redis connections to use Redis URL - closes #7421 #7736

Refactor Redis connections to use Redis URL - closes #7421 #7736

Conversation

thomasmol commented Oct 16, 2024

Environment Variable Updates:

Docker and Kubernetes Configuration Updates:

Code Module Updates:

Documentation Updates:

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 16, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 16, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 16, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 16, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 16, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 16, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 16, 2024

Choose a reason for hiding this comment

charlesBochet left a comment

Choose a reason for hiding this comment

charlesBochet commented Oct 16, 2024 • edited Loading

charlesBochet commented Oct 16, 2024

charlesBochet commented Oct 16, 2024

charlesBochet commented Oct 16, 2024 • edited Loading

charlesBochet commented Oct 16, 2024

charlesBochet Oct 16, 2024

Choose a reason for hiding this comment

thomasmol Oct 16, 2024

Choose a reason for hiding this comment

thomasmol commented Oct 16, 2024 • edited Loading

charlesBochet Oct 16, 2024

Choose a reason for hiding this comment

charlesBochet left a comment

Choose a reason for hiding this comment

Weiko commented Oct 16, 2024

Weiko commented Oct 16, 2024

oss-gg bot commented Oct 16, 2024

FelixMalfait commented Oct 16, 2024

charlesBochet commented Oct 16, 2024 •

edited

Loading

charlesBochet commented Oct 16, 2024 •

edited

Loading

thomasmol commented Oct 16, 2024 •

edited

Loading