-
Notifications
You must be signed in to change notification settings - Fork 29
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
feature request: allow connection url #246
Comments
optionally i believe adding |
Good idea @iwpnd. I can definitely add this in. |
i have a working first draft, where whenever a private getConnection() {
const hasUrl = this.configService.config.REDIS_URL !== '';
return !hasUrl
? {
connection: {
host: this.configService.config.REDIS_HOST,
port: this.configService.config.REDIS_PORT,
password: this.configService.config.REDIS_PASSWORD,
},
}
: {
connection: new Redis(this.configService.config.REDIS_URL, {
maxRetriesPerRequest: null,
}),
};
}
// used like
create(queuePrefix: string, queueName: string, queue: Queue) {
const labels = {
[LABEL_NAMES.QUEUE_PREFIX]: queuePrefix,
[LABEL_NAMES.QUEUE_NAME]: queueName,
};
const queueEvents = new QueueEvents(queueName, {
prefix: queuePrefix,
...this.getConnection(),
});
[...] I understand that everybody has their way of working, so I understand that this is not the desired way to do it - yet, with some guidance I can draft a PR and complete this myself if you'd allow me to. wdyt @ejhayes ? |
adding my vote for this as well. as I understand it, this package won't work with redis instances that are configured to use TLS? |
This is correct @cahlan |
Hey @ejhayes 👋
any chance you could offer the option to pass a redis url instead of the host/port? that would allow to connect optionally via tls using
rediss://
orredis://
.Looks like
@nestjs/redis
is supporting it here. As for implementation one could prioritise an env such asREDIS_URL
overREDIS_HOST/REDIS_PORT/REDIS_PASSWORD
if set.wdyt?
The text was updated successfully, but these errors were encountered: