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

Units test to investigate AMQP generation #790

Closed
wants to merge 2 commits into from

Conversation

pdalfarr
Copy link
Contributor

@pdalfarr pdalfarr commented Jun 5, 2024

Units test to investigate AMQP generation
I added some comments in the PR.

Copy link

netlify bot commented Jun 5, 2024

Deploy Preview for springwolf-ui canceled.

Name Link
🔨 Latest commit 8c0950e
🔍 Latest deploy log https://app.netlify.com/sites/springwolf-ui/deploys/666054a762d49700098046e9

@@ -24,6 +24,7 @@ springwolf-bindings/springwolf-sqs-binding/build/
node_modules/

asyncapi.actual.json
asyncapi.actual.yaml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add YAML to ease analysis / investigation

import lombok.NoArgsConstructor;

@NoArgsConstructor(access = AccessLevel.PRIVATE)
public final class AmqpConstants {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add constant to ease navigation in code

@Exchange(
name = AmqpConstants.EXCHANGE_CRUD_TOPIC_EXCHANGE_1,
type = ExchangeTypes.TOPIC),
key = AmqpConstants.ROUTING_KEY_ALL_MESSAGES,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using "#" routing key

host: amqp:5672
protocol: amqp
channels:
'CRUD-topic-exchange-1_#':
Copy link
Contributor Author

@pdalfarr pdalfarr Jun 5, 2024

Choose a reason for hiding this comment

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

in generated ayncapi.yaml, this is replaced by "#" and there are 2 messages (GenericPayloadDto and ExamplePayLoadDto) which are associated both associated to CRUD-topic-exchange-1 !
And NOT to CRUD-topic-exchange-2 !
I would have expected to have 2 separated channels, as written in this file (on for exchange CRUD-topic-exchange-1, the other for exchange CRUD-topic-exchange-2)
Something is fishy here.

@timonback
Copy link
Member

Hi @pdalfarr,
I took this PR as a base to look into amqp and further understand how rabbitmq and the AsyncAPI amqp binding works. The result is #886

I agree with your point on the expected channel name.
Assuming that is fixed, how to bind the routingKey + exchange with the queue (see my comment)?

Also, it would be great if you can verify the comments on RabbitListenerUtil as I gather them by testing, but have low confidence since I haven't used RabbitMQ in production (see https://github.com/springwolf/springwolf-core/pull/886/files#r1699142615 )

@timonback
Copy link
Member

Closing this PR, as the proposed changes have been merged via other PRs.

Also, part of the issue has been addressed, while the link from exchange to queue is missing in the AsyncAPI at this point. Lets move the discussion into the original Springwolf issue #366

@timonback timonback closed this Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants