-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
fix: Management of BullMQ queue prefixes #3220
base: minor
Are you sure you want to change the base?
fix: Management of BullMQ queue prefixes #3220
Conversation
- current implementation had 2 issues: - replacing ALL passed worker options by default values - assuming that all redis scripts should be executed vs 'bull:*' queues, which contradics with worker/queue options
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
recheck |
@michaelbromley I think this is a good first fix, but we need to check if there are any dependencies on this |
const options = injector.get<BullMQPluginOptions>(BULLMQ_PLUGIN_OPTIONS); | ||
this.options = { | ||
...options, | ||
workerOptions: { |
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.
Please spread the configured worker options in there with ...(options.workerOptions ?? {})
instead of assigning it the bottom.
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.
Great point, pls see updated version.
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.
@areg-gareginyan-im the default values of removeOnComplete
and removeOnFail
should be kept. If you are passing workerOptions
now they would be lost.
recheck |
@areg-gareginyan-im the CLA already worked. Could you please fix the things from my code review? |
Quality Gate failedFailed conditions |
const options = injector.get<BullMQPluginOptions>(BULLMQ_PLUGIN_OPTIONS); | ||
this.options = { | ||
...options, | ||
workerOptions: { |
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.
@areg-gareginyan-im the default values of removeOnComplete
and removeOnFail
should be kept. If you are passing workerOptions
now they would be lost.
@areg-gareginyan-im could you please fix the things from the last code review, then we can finally merge it. Thanks! |
Description
In my project i planned to use same Redis for multiple Vendure instances, and wanted to separate BullMQ queus using worker/queue prefixes. However, current implementation had 2 issues:
NB: I know that passing in a
db
to theconnection
options would be a viable way to achieve my goal as stated in #2645, however making worker/queue prefixes work as they should be IMO is a nice thing.Breaking changes
None
Screenshots
You can add screenshots here if applicable.
Checklist
📌 Always:
👍 Most of the time: