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

Kafka configuration normalization #13889

Closed
jtama opened this issue Dec 15, 2020 · 5 comments
Closed

Kafka configuration normalization #13889

jtama opened this issue Dec 15, 2020 · 5 comments
Assignees
Milestone

Comments

@jtama
Copy link
Contributor

jtama commented Dec 15, 2020

Describe the bug
Kafka configuration properties should be the same whatever the extension used.

Expected behavior
As of now, if you use the smallrye-reactive-messaging-kafka, the kafka host is configured through the kafka.bootstrap.servers property, but if you use the quarkus-smallrye-reactive-messaging-kafka, the you must use quarkus.kafka-streams.bootstrap-servers property.

If both extension are used, the configuration miust be duplicated which is error prone.

Actualy, none of the quarkus-smallrye-reactive-messaging-kafka properties starts with quarkuswhich seems odd to me.

Actual behavior
Configuration property keys should be normalized.

  • Quarkus version : 1.10.3
@jtama jtama added the kind/bug Something isn't working label Dec 15, 2020
@ghost ghost added the triage/needs-triage label Dec 15, 2020
@cescoffier
Copy link
Member

SmallRye Reactive Messaging properties do not start with the quarkus prefix because they are defined in the Reactive Messaging specification.

Regarding this issue, we should make the Kafka streams extension read the kafka.bootstrap.servers property as a fallback (if quarkus.kafka-streams.bootstrap-servers is not set)

@cescoffier cescoffier self-assigned this Jan 4, 2021
@jtama
Copy link
Contributor Author

jtama commented Jan 4, 2021

Yes I get that property keys are defined by SmallRye Reactive Messaging, but couldn't we provide "translation" from quarkus to SmallRye Reactive Messaging like property names ? I mean many other extension do exactly that.

If I can help on this, please let me know.

@cescoffier
Copy link
Member

No, unfortunately, it's not that simple in this case. How reactive messaging (not smallrye, the spec) is a bit particular.

We could prepend "quarkus." to all the properties, but that would only create longer keys.

cescoffier added a commit to cescoffier/quarkus that referenced this issue Jan 4, 2021
"kafka.bootstrap.servers" is the property used by the Reactive Messaging Kafka connector.

Related to quarkusio#13889.
@cescoffier
Copy link
Member

BTW, I just opened #14095 which allows Kafka Streams to be configured using kafka.bootstrap.servers.

@cescoffier
Copy link
Member

Fixed by #14095.

@cescoffier cescoffier added this to the 1.11 - master milestone Jan 5, 2021
@cescoffier cescoffier added area/kafka area/kafka-streams kind/enhancement New feature or request and removed triage/needs-triage kind/bug Something isn't working labels Jan 5, 2021
cemnura pushed a commit to cemnura/quarkus that referenced this issue Jan 20, 2021
"kafka.bootstrap.servers" is the property used by the Reactive Messaging Kafka connector.

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

No branches or pull requests

2 participants