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.

Sign up for GitHub

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

Jump to bottom

Support to configure deserializers of KafkaConsumer #13097

Merged
merged 1 commit into from
Sep 17, 2022
Merged

Support to configure deserializers of KafkaConsumer #13097

merged 1 commit into from
Sep 17, 2022

Conversation

eshengit
Copy link
Contributor

Fixes #12960.

Description

Allow Kafka Consumer's custom deserializer to be configured after its instantiation.

Key changed/added classes in this PR
  • extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaRecordSupplier.java

This PR has:

  • [ x] been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • [ x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • [ x] been tested in a test Druid cluster.

@eshengit eshengit mentioned this pull request Sep 15, 2022
Closed
7 tasks
@FrankChen021 FrankChen021 changed the title kafka consumer: custom serializer can't be configured after it's inst… Support to configure deserializers of KafkaConsumer Sep 16, 2022
@FrankChen021
Copy link
Member

Thanks for your first contribution @eshengit

@eshengit eshengit force-pushed the kafka-custom-serilizer-configurable branch from 1fb69ba to fde95dc Compare September 16, 2022 02:16
@eshengit
Copy link
Contributor Author

@FrankChen021
Thanks for reviewing! how can I merge once all the check is completed?
should I add you as collaborator of my personal repo, or you add me as druid collaborator?

@FrankChen021
Copy link
Member

No worry, we will merge it once the CI finishes.

@FrankChen021
Copy link
Member

@eshengit Please fix the failures reported by Travis CI

kafka consumer: custom serializer can't be configured after it's inst…
a46c1d7 
…antiation (apache#12960)

  * allow kakfa custom serializer to be configured

  * add unit tests
@eshengit eshengit force-pushed the kafka-custom-serilizer-configurable branch from fde95dc to a46c1d7 Compare September 16, 2022 16:15
@eshengit
Copy link
Contributor Author

@FrankChen021 ,

  1. checkestyle error is corrected.
  2. the unit test failure: the module failed indexing-service does not seem to have anything to do with my change which depends on failed module.
    run those failed tests in my IDE didn't show any error.

@FrankChen021 FrankChen021 merged commit da30c80 into apache:master Sep 17, 2022
@kfaraz kfaraz added this to the 25.0 milestone Nov 22, 2022
This was referenced Dec 18, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a2l007 a2l007 a2l007 approved these changes

@FrankChen021 FrankChen021 FrankChen021 approved these changes

Assignees
No one assigned
Labels
Area - Streaming Ingestion
Projects
None yet
Milestone
25.0
Development

Successfully merging this pull request may close these issues.

kafka consumer: custom serializer can't be configured after it's instantiation
4 participants
@eshengit @FrankChen021 @a2l007 @kfaraz