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

Update document for testing kafka without broker #35051

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

cuichenli
Copy link
Contributor

@cuichenli cuichenli commented Jul 27, 2023

two main changes:

  1. add the @Connector("smallrye-in-memory") annotation. otherwise it will report error
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.arc.deployment.ArcProcessor#validate threw an exception: jakarta.enterprise.inject.spi.DeploymentException: jakarta.enterprise.inject.UnsatisfiedResolutionException: Unsatisfied dependency for type io.smallrye.reactive.messaging.memory.InMemoryConnector and qualifiers [@Default]
	- java member: me.cuichenli.BaristaTest#connector
	- declared on CLASS bean [types=[me.cuichenli.BaristaTest, java.lang.Object], qualifiers=[@Default, @Any], target=me.cuichenli.BaristaTest]
	The following beans match by type, but none have matching qualifiers:
		- Bean [class=io.smallrye.reactive.messaging.memory.InMemoryConnector, qualifiers=[@Any, @Connector("smallrye-in-memory")]]

The reproducible repository: https://github.com/cuichenli/quarkus-35051-kafka-test-without-broker
2. add test example for testing batch consumer with inmemoryconnector

one minor change:

  1. specified the await is imported from awaitability

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 27, 2023

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@cuichenli cuichenli marked this pull request as ready for review July 27, 2023 06:35
@github-actions
Copy link

github-actions bot commented Jul 27, 2023

🙈 The PR is closed and the preview is expired.

@cuichenli cuichenli changed the title update document for testing kafka without broker Update document for testing kafka without broker Jul 27, 2023
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.

Thanks for taking the time to contribute!

I added a few small comments about the form.

I will let @ozangunalp review the content.

<5> Use the `received` method on `beverages` channel to check the messages produced by the application.
<5> Use the `received` method on `beverages` channel to check the messages produced by the application. The `await` is imported from https://github.com/awaitility/awaitility[awaitility].

If your Kafka consumer is batch based, you will need to send a batch of messages to the channel as per discussed in this https://github.com/smallrye/smallrye-reactive-messaging/discussions/1691#discussioncomment-2471033[Github Discussion].
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should link to a discussion. Our doc should be authoritative.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -2052,7 +2053,75 @@ class BaristaTest {
<3> Retrieve the outgoing channel (`beverages`) - the channel must have been switched to in-memory in the test resource.
<4> Use the `send` method to send a message to the `orders` channel.
The application will process this message and send a message to `beverages` channel.
<5> Use the `received` method on `beverages` channel to check the messages produced by the application.
<5> Use the `received` method on `beverages` channel to check the messages produced by the application. The `await` is imported from https://github.com/awaitility/awaitility[awaitility].
Copy link
Member

@gsmet gsmet Jul 27, 2023

Choose a reason for hiding this comment

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

I think we should add the imports to the examples instead. That's what we try to do elsewhere as it can be hard to get it right if you don't have the imports.

Note that this is orthogonal to your PR but maybe you could take care of it in a separate commit, instead of adding this sentence?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@cuichenli
Copy link
Contributor Author

thanks @gsmet for the review. i will update it accordingly but later - i am currently on something else.

@cuichenli
Copy link
Contributor Author

Hi @gsmet and @ozangunalp, it has been a while since the last time the pr got reviewed, any chance you can have a look on it when you got a chance? Thanks!

@cuichenli cuichenli requested a review from gsmet August 21, 2023 04:07
@gsmet gsmet force-pushed the update-kafka-documents branch from 6a845a3 to 12690df Compare September 22, 2023 15:41
@gsmet
Copy link
Member

gsmet commented Sep 22, 2023

I rebased and squashed. @ozangunalp could you have a look, please? It looks like a worthy addition.

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.

Good addition indeed! The test case needs some tweaking but it'll be nice to add it in the Kafka docs.

public class BeverageProcessor {

@Incoming("orders")
@Outgoing("beverages")
Copy link
Contributor

Choose a reason for hiding this comment

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

The processing method doesn't return any outgoing data. You need to remove this Outgoing annotation and the beverages sink in the test code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

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.

LGTM! Thanks

@ozangunalp
Copy link
Contributor

@cuichenli could you squash commits to only have one?

@cuichenli cuichenli force-pushed the update-kafka-documents branch from 6401a2a to 2ba044d Compare September 25, 2023 07:30
@cuichenli
Copy link
Contributor Author

@cuichenli could you squash commits to only have one?

sure, updated!

@ozangunalp ozangunalp merged commit 1e6b053 into quarkusio:main Sep 25, 2023
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.5 - main milestone Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants