-
Notifications
You must be signed in to change notification settings - Fork 183
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
Fixing redis queue for tests by adding prefix #921
Conversation
@@ -26,6 +27,7 @@ const getQueueOptions = (timeout: number): QueueOptions => { | |||
removeOnComplete: true, | |||
removeOnFail: true | |||
}, | |||
prefix: envVars.BULL_QUEUE_PREFIX, |
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.
Looks like a very simple and elegant solution.
just want to make sure that if we don't have this env var in prod we won't add any prefixes. (Cuz I am afraid if we'll start using another "queue" in prod and loose messages from the one we were using before.)
Is there any way to test it in ddev/staging? Would be great to do before merging
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.
Well, the env var isn't set anywhere else but in .env.test
and will just return undefined
for all other environments, which will then default it.
However, that being said, I think I found a better option which is to set the key prefix for all redis keys and not just bull, so that no tests ever affects development.
Happy to deploy to staging just to be 100% sure though. One way to test this would be to start a big sync before, deploy this to staging, then make sure the sync continues afterwards.
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.
actually, nvm, using redis keyPrefix
didn't seem to work well with bull queues for some reason.
…dis keys for tests
…r all redis keys for tests" This reverts commit 32ff62d.
@KAbakumovAtl I've deployed to staging while in the middle of a sync to be 100% sure that the new setting doesn't affect anything by testing and it worked fine. |
Fixes flakiness in tests that used the Redis bull queues by creating separate queues for the tests compared to development so the running app doesn't consume the test messages.