-
Notifications
You must be signed in to change notification settings - Fork 69
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
Implement/rabbitmq channel #1204
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1204 +/- ##
==========================================
- Coverage 86.81% 86.53% -0.28%
==========================================
Files 87 87
Lines 5906 5926 +20
==========================================
+ Hits 5127 5128 +1
- Misses 779 798 +19
|
Thanks for the contribution! To merge this we would need some tests to be added and some instruction on how to test this. Is there also a PR for the openhim-console to add this functionality to the UI? |
route.type === 'kafka' | ||
? sendKafkaRequest | ||
: route.type === 'rabbitmq' | ||
? sendRabbitMQRequest | ||
: sendHttpRequest |
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 chained logic is a bit confusing, perhaps rather split this out to a switch statement or something rather than chaining ternaries.
Do you also have a particular use case where you are using RabbitMQ with OpenHIM. I'd love to hear about it. |
Thanks for the response,
I am adding the tests for all the additional codes as required. Also there
exist a PR for the console too which includes the implementation for the
RabbitMQ in the UI.
…On Thu, 14 Sept 2023 at 15:46, Ryan Crichton ***@***.***> wrote:
Thanks for the contribution! To merge this we would need some tests to be
added and some instruction on how to test this. Is there also a PR for the
openhim-console to add this functionality to the UI?
—
Reply to this email directly, view it on GitHub
<#1204 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGMCI5MMPYWDHJSTNCW6CQDX2L4DHANCNFSM6AAAAAA35IBMRI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
*Sincerely,*
*Lawrance Massanja*
*Director at *MatrixHub Company Limited <https://www.matrixhub.co.tz/>
*Software Developer at Ministry of Health <https://www.moh.go.tz/>**Dodoma
- Tanzania*
Mobile: +255 754 690 060
ALT: +255 717 455 043
ALT: ***@***.*** | ***@***.***
|
Thanks, i am working on it.
…On Thu, 14 Sept 2023 at 15:49, Ryan Crichton ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In src/middleware/router.js
<#1204 (comment)>
:
> + route.type === 'kafka'
+ ? sendKafkaRequest
+ : route.type === 'rabbitmq'
+ ? sendRabbitMQRequest
+ : sendHttpRequest
This chained logic is a bit confusing, perhaps rather split this out to a
switch statement or something rather than chaining ternaries.
—
Reply to this email directly, view it on GitHub
<#1204 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGMCI5PNADL47NJZHTSU7NDX2L4NTANCNFSM6AAAAAA35IBMRI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
*Sincerely,*
*Lawrance Massanja*
*Director at *MatrixHub Company Limited <https://www.matrixhub.co.tz/>
*Software Developer at Ministry of Health <https://www.moh.go.tz/>**Dodoma
- Tanzania*
Mobile: +255 754 690 060
ALT: +255 717 455 043
ALT: ***@***.*** | ***@***.***
|
Yes we have a particular use case for it.
Use case 1.
We currently have a Microservices implementation in the Ministry of Health
which RabbitMQ stands in between communication of two or more services. In
the beginning we integrated services with RabbitMQ only but we came
across data verification on either ends. So, we developed a mediator which
communicates with RabbitMQ for tracking purposes of data flow. The Mediator
misbehaves sometime with its ups and downs because we currently host all
the mediators in their own specific servers. We began an intervention of
adding the functionality in openhim which will solve our issue and it is
solved successfully.
Use case 2.
We have some integrations (data exchange) with some systems which are not
from our premises and they are the one coming for the data. So far, they
are accessing RabbitMQ for the collection of data which was manipulated by
our source system and dumped in RabbitMQ. So, they have their own listeners
which when data is posted in RabbitMQ Channel, they trigger a consuming
activity.
Through these two cases, we opted in integrating RabbitMQ into openhim
which saves a lot of purposes.
We thought that maybe someone somewhere using openhim may need such an
implementation thats why we initiated a pull request for others to use.
Thanks a lot for your consideration,
Lawrance M.
…On Thu, 14 Sept 2023 at 15:50, Ryan Crichton ***@***.***> wrote:
Do you also have a particular use case where you are using RabbitMQ with
OpenHIM. I'd love to hear about it.
—
Reply to this email directly, view it on GitHub
<#1204 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGMCI5NBNAEKES3JEDVJAH3X2L4PVANCNFSM6AAAAAA35IBMRI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
*Sincerely,*
*Lawrance Massanja*
*Director at *MatrixHub Company Limited <https://www.matrixhub.co.tz/>
*Software Developer at Ministry of Health <https://www.moh.go.tz/>**Dodoma
- Tanzania*
Mobile: +255 754 690 060
ALT: +255 717 455 043
ALT: ***@***.*** | ***@***.***
|
Implementation of rabbitmq enabling openhim to publish message in rabbitmq and returning response to the source in less response time.