Skip to content
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

feat(microservices): add noAssert option for RMQ connection #9798

Conversation

frankmangone
Copy link

@frankmangone frankmangone commented Jun 18, 2022

Rabbit MQ brokers may not allow for queue declaration, so a check is needed to avoid 403 errors in that scenario

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Rabbit MQ brokers may not allow for queue declaration, so a check is needed to avoid 403 errors in that scenario
@coveralls
Copy link

Pull Request Test Coverage Report for Build cf3c2081-3964-437f-bd0e-0d8a73dbd915

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 3b459293-491d-46fc-a469-e4753dfd9de9: 0.0%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

Copy link
Member

@jmcdo29 jmcdo29 left a comment

Choose a reason for hiding this comment

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

Code is clean and easy enough to follow. I'm not well enough aware of RMQ to determine the functionality other than what is in the PR description, but it seems like a good thing to me. It's got my approval, but I think @kamilmysliwiec should look over it too

@frankmangone
Copy link
Author

Just to add some more context, here's the assertQueue documentation:
https://amqp-node.github.io/amqplib/channel_api.html#channel_assertQueue

At the moment, I need to use patch-package to match the RabbitMQ permissions in the project I'm currently working on (none of the users have configuration permissions). This is the reason why I'm getting a 403 when working with @nestjs/microservices in it's current state.

It would be very much appreciated if this could be merged.
@kamilmysliwiec I summon thee 😄

@kamilmysliwiec kamilmysliwiec merged commit 5663012 into nestjs:master Jul 20, 2022
@kamilmysliwiec
Copy link
Member

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants