-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add support for vhosts in RabbitMQ Dev Services topology #43785
Conversation
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
479ee86
to
b889f39
Compare
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.
Very nice work! I added a small suggestion and I think we need @ozangunalp to have a look.
Thanks for going the extra mile by fixing the doc warnings but don't feel obliged to fix Vale warnings that are not due to your additions when providing a PR :).
.github/native-tests.json
Outdated
@@ -63,7 +63,7 @@ | |||
{ | |||
"category": "Messaging2", | |||
"timeout": 80, | |||
"test-modules": "reactive-messaging-amqp, reactive-messaging-rabbitmq, reactive-messaging-rabbitmq-dyn, reactive-messaging-pulsar", | |||
"test-modules": "reactive-messaging-amqp, reactive-messaging-rabbitmq, reactive-messaging-rabbitmq-dyn, reactive-messaging-rabbitmq-devservices, reactive-messaging-pulsar", |
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.
You don't need to add the test here as we don't really care testing native for it. The Dev Services are run in the exact same way.
Having the test run in JVM mode is enough.
(We are a bit careful about adding tests there as native compilation takes quite a lot of time)
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.
Okay, removed the native test.
This comment has been minimized.
This comment has been minimized.
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 to me and thanks for adding an IT! I also don't see the need to add it in native tests.
b889f39
to
a4b2f11
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Status for workflow
|
Status for workflow
|
RabbitMQ has a concept of a virtual host (vhost). Adding support so the RabbitMQ dev service topology builder can include virtual host information.