-
Notifications
You must be signed in to change notification settings - Fork 25
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
Jobs RabbitMQ Action connection info should come from ConfigService #1575
Conversation
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.
This will create a new messagebroker instance for each action. I think it would be better to wrap this into a nest module so it can be shared.
AppModule also constructs a RabbitMQMessageBroker, coming from @user-office-software/duo-message-broker rather than amqplib
directly. It seems to be ESS-specific functionality for their proposal system. I was going to suggest using the same broker instance for that too, but after looking at the code I'm not sure it would work. We should discuss that code with the ESS people, but perhaps not in this PR.
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 good, thanks for the update!
}, | ||
); | ||
this.rabbitMQService.connect(this.queue, this.exchange, this.key); | ||
this.rabbitMQService.sendMessage(this.queue, JSON.stringify(job)); |
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.
We should double check that it's thread-safe. I think it should be ok since they aren't async.
Description
Previously, all information needed to set up the Jobs RabbitMQ action would come from
jobConfig
.A better approach is to split this information: connection config info should be in
.env
file, and only the queue info in thejobConfig
file.Motivation
Extract the info from
ConfigService
after the NestJS refactor.Changes
While NestJs offers a RabbitMQ microservice, unfortunately it does not cover all necessary options needed for Jobs. More specifically, we are configurating
exchange
andkey
, which is not offered by that library. So instead, we stick to using theamqplib
library as before, but only requestqueue
,exchange
andkey
to be defined injobConfig
. Which means that the.env
file should now look like this example for RabbitMQ Job Action to be set up properly:And the job action definition in the
jobConfig
file, should look like this:Note: make sure you have a running RabbitMQ instance for testing this. For example, with
scicat-ci
: despadam/scicat-ci@9699590To make sure that messages are being sent, you can connect to the RabbitMQ admin page (http://localhost:15672/ by default) with the username and password provided in the
.env
file. There, after a job creation or update, you will see that the queue has been created and messages are incoming:You can also explore the queue contents, as seen here: