-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Redis connections to use Redis URL - closes #7421 #7736
Changes from all commits
158ab70
4d15012
0ec9d12
655265c
2ed4704
9112b6c
68c8612
551f789
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,10 +41,8 @@ spec: | |
value: "https://crm.example.com:443" | ||
- name: "PG_DATABASE_URL" | ||
value: "postgres://twenty:[email protected]/default" | ||
- name: "REDIS_HOST" | ||
value: "twentycrm-redis.twentycrm.svc.cluster.local" | ||
- name: "REDIS_PORT" | ||
value: 6379 | ||
- name: "REDIS_URL" | ||
value: "redis://twentycrm-redis.twentycrm.svc.cluster.local:6379" | ||
- name: ENABLE_DB_MIGRATIONS | ||
value: "true" | ||
- name: SIGN_IN_PREFILLED | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,12 +61,8 @@ resource "kubernetes_deployment" "twentycrm_server" { | |
value = "postgres://twenty:${var.twentycrm_pgdb_admin_password}@${kubernetes_service.twentycrm_db.metadata.0.name}.${kubernetes_namespace.twentycrm.metadata.0.name}.svc.cluster.local/default" | ||
} | ||
env { | ||
name = "REDIS_HOST" | ||
value = "${kubernetes_service.twentycrm_redis.metadata.0.name}.${kubernetes_namespace.twentycrm.metadata.0.name}.svc.cluster.local" | ||
} | ||
env { | ||
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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: The Redis URL format doesn't include authentication. If Redis requires auth, update the URL to include username and password |
||
} | ||
env { | ||
name = "ENABLE_DB_MIGRATIONS" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,13 +59,8 @@ resource "kubernetes_deployment" "twentycrm_worker" { | |
} | ||
|
||
env { | ||
name = "REDIS_HOST" | ||
value = "${kubernetes_service.twentycrm_redis.metadata.0.name}.${kubernetes_namespace.twentycrm.metadata.0.name}.svc.cluster.local" | ||
} | ||
|
||
env { | ||
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" | ||
Comment on lines
+62
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
} | ||
|
||
env { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,10 +50,7 @@ SIGN_IN_PREFILLED=true | |
# SENTRY_FRONT_DSN=https://[email protected]/xxx | ||
# LOG_LEVELS=error,warn | ||
# MESSAGE_QUEUE_TYPE=pg-boss | ||
# REDIS_HOST=127.0.0.1 | ||
# REDIS_PORT=6379 | ||
# REDIS_USERNAME= | ||
# REDIS_PASSWORD= | ||
# REDIS_URL=redis://localhost:6379 | ||
# DEMO_WORKSPACE_IDS=REPLACE_ME_WITH_A_RANDOM_UUID | ||
# SERVER_URL=http://localhost:3000 | ||
# WORKSPACE_INACTIVE_DAYS_BEFORE_NOTIFICATION=30 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,12 @@ | ||
import { ConnectionOptions } from 'tls'; | ||
|
||
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service'; | ||
import { | ||
BullMQDriverFactoryOptions, | ||
MessageQueueDriverType, | ||
MessageQueueModuleOptions, | ||
PgBossDriverFactoryOptions, | ||
SyncDriverFactoryOptions, | ||
} from 'src/engine/core-modules/message-queue/interfaces'; | ||
|
||
/** | ||
|
@@ -19,7 +24,7 @@ export const messageQueueModuleFactory = async ( | |
return { | ||
type: MessageQueueDriverType.Sync, | ||
options: {}, | ||
}; | ||
} satisfies SyncDriverFactoryOptions; | ||
} | ||
case MessageQueueDriverType.PgBoss: { | ||
const connectionString = environmentService.get('PG_DATABASE_URL'); | ||
|
@@ -29,25 +34,23 @@ export const messageQueueModuleFactory = async ( | |
options: { | ||
connectionString, | ||
}, | ||
}; | ||
} satisfies PgBossDriverFactoryOptions; | ||
} | ||
case MessageQueueDriverType.BullMQ: { | ||
const host = environmentService.get('REDIS_HOST'); | ||
const port = environmentService.get('REDIS_PORT'); | ||
const username = environmentService.get('REDIS_USERNAME'); | ||
const password = environmentService.get('REDIS_PASSWORD'); | ||
const connectionString = environmentService.get('REDIS_URL'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Validate REDIS_URL before using it to prevent potential runtime errors |
||
|
||
if (!connectionString) { | ||
throw new Error( | ||
`${MessageQueueDriverType.BullMQ} message queue requires REDIS_URL to be defined, check your .env file`, | ||
); | ||
} | ||
|
||
return { | ||
type: MessageQueueDriverType.BullMQ, | ||
options: { | ||
connection: { | ||
host, | ||
port, | ||
username, | ||
password, | ||
}, | ||
connection: connectionString as ConnectionOptions, | ||
}, | ||
}; | ||
} satisfies BullMQDriverFactoryOptions; | ||
} | ||
default: | ||
throw new Error( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using the redis service name instead of localhost in the default URL