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

Add support for vhosts in RabbitMQ Dev Services topology #43785

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

subnova
Copy link
Contributor

@subnova subnova commented Oct 9, 2024

RabbitMQ has a concept of a virtual host (vhost). Adding support so the RabbitMQ dev service topology builder can include virtual host information.

Copy link

github-actions bot commented Oct 9, 2024

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

This comment has been minimized.

@quarkus-bot quarkus-bot bot added the area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure label Oct 10, 2024
@subnova subnova changed the title Add support for vhosts in RabbitMQ dev service topology Add support for vhosts in RabbitMQ Dev Services topology Oct 10, 2024
Copy link
Member

@gsmet gsmet left a 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 :).

@@ -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",
Copy link
Member

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)

Copy link
Contributor Author

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.

Copy link
Contributor

@ozangunalp ozangunalp left a 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.

This comment has been minimized.

This comment has been minimized.

Copy link

quarkus-bot bot commented Oct 10, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 9872e63.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Oct 10, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9872e63.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand geoand merged commit 04a486a into quarkusio:main Oct 14, 2024
53 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/reactive-messaging area/smallrye
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants