Skip to content

Commit

Permalink
fix: disable redis if not configured
Browse files Browse the repository at this point in the history
Fixes #1516
  • Loading branch information
jef committed Dec 26, 2020
1 parent 14998d8 commit 6bc7737
Showing 1 changed file with 13 additions and 7 deletions.
20 changes: 13 additions & 7 deletions src/notification/redis.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
import {Link, Store} from '../store/model';
import redis, {RedisClient} from 'redis';
import {config} from '../config';
import {logger} from '../logger';
import redis from 'redis';

const {url} = config.notifications.redis;

const client = redis.createClient({
url
});
function initRedis(): RedisClient | null {
if (url) {
return redis.createClient({url});
}

return null;
}

const updateRedis = (link: Link, store: Store) => {
function updateRedis(link: Link, store: Store) {
try {
if (url) {
const client = initRedis();

if (client) {
const key = `${store.name}:${link.brand}:${link.model}`
.split(' ')
.join('-');
Expand All @@ -35,6 +41,6 @@ const updateRedis = (link: Link, store: Store) => {
} catch (error: unknown) {
logger.error("✖ couldn't update redis", error);
}
};
}

export default updateRedis;

5 comments on commit 6bc7737

@gianmarcotoso
Copy link

Choose a reason for hiding this comment

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

Doesn't this create a new client every time a notification is sent? You might want to add a client.quit() call after the write has finished, or scope the client outside of the initRedis function and keep just one open (even though it might time out if no notification is sent for too long, depending on the Redis server configuration).

@jef
Copy link
Owner Author

@jef jef commented on 6bc7737 Dec 27, 2020

Choose a reason for hiding this comment

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

Yes, very true. Sorry, I followed up with another commit (a908ce4) before your comment. This should hopefully take care of what you've mentioned!

@jef
Copy link
Owner Author

@jef jef commented on 6bc7737 Dec 27, 2020

Choose a reason for hiding this comment

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

Thanks for your helpful comment @gianmarcotoso 😀

@gianmarcotoso
Copy link

Choose a reason for hiding this comment

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

You're very welcome, and thank you for this tool! 😀

@jef
Copy link
Owner Author

@jef jef commented on 6bc7737 Jan 5, 2021

Choose a reason for hiding this comment

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

You bet! Enjoy 🚀

Please sign in to comment.