Skip to content

KAFKA-20173: Propagate headers into serde 1/N#21490

Merged
frankvicky merged 5 commits intoapache:trunkfrom
UladzislauBlok:bloku/kafka-20173
Feb 18, 2026
Merged

KAFKA-20173: Propagate headers into serde 1/N#21490
frankvicky merged 5 commits intoapache:trunkfrom
UladzislauBlok:bloku/kafka-20173

Conversation

@UladzislauBlok
Copy link
Contributor

@UladzislauBlok UladzislauBlok commented Feb 16, 2026

Propagate headers into serializes / deserializers

This PR partially covers org.apache.kafka.streams.kstream package.

Code:

  • Use empty headers for backward compatibility
  • Propagate headers when it's possible

Tests:

  • Use empty headers for existing tests
  • Add new test if needed

Reviewers: Matthias J. Sax matthias@confluent.io, Alieh Saeedi
asaeedi@confluent.io, TengYao Chi frankvicky@apache.org

@github-actions github-actions bot added triage PRs from the community streams small Small PRs labels Feb 16, 2026
@github-actions github-actions bot removed the small Small PRs label Feb 16, 2026
@UladzislauBlok
Copy link
Contributor Author

@mjsax Hello.
First is ready to be review. Thanks

Copy link
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

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

Overall LGTM.

@mjsax mjsax added ci-approved and removed triage PRs from the community labels Feb 17, 2026
@mjsax
Copy link
Member

mjsax commented Feb 17, 2026

Is the PR description correct?

This PR covers org.apache.kafka.streams.kstream package (without sub-packages)

Does this PR really cover the entire package?

@mjsax
Copy link
Member

mjsax commented Feb 17, 2026

Build failed with checkstyle error:

Execution failed for task ':streams:spotlessJavaCheck'.
> The following files had format violations:
      src/test/java/org/apache/kafka/streams/kstream/SessionWindowedDeserializerTest.java
          @@ -24,8 +24,8 @@
           import·org.apache.kafka.common.serialization.Serdes;
           import·org.apache.kafka.common.serialization.StringDeserializer;
           import·org.apache.kafka.streams.StreamsConfig;
          +import·org.apache.kafka.streams.kstream.internals.TimeWindow;
           
          -import·org.apache.kafka.streams.kstream.internals.TimeWindow;
           import·org.junit.jupiter.api.Test;
           
           import·java.util.HashMap;
  Run './gradlew spotlessApply' to fix all violations.

@UladzislauBlok
Copy link
Contributor Author

UladzislauBlok commented Feb 17, 2026

Is the PR description correct?

This PR covers org.apache.kafka.streams.kstream package (without sub-packages)

Does this PR really cover the entire package?

Thanks for checking. I found two more calls to serializer in this package (previously I checked only deserializer). They are from the same classes, so I'll add them

@aliehsaeedii
Copy link
Contributor

aliehsaeedii commented Feb 17, 2026

Thanks for checking. I found two more calls to serializer in this package (previously I checked only deserializer). They are from the same classes, so I'll add them

not sure about the scope of the change. Should we consider the deserializer in FullChangeSerde.java, CombinedKeySchema.java, ... and many more calls of .deserialize(topic, data) in the package?

Oh, OK, you said (without sub-packages). A short explanation of the reason for this scope could be added maybe.

Copy link
Contributor

@frankvicky frankvicky left a comment

Choose a reason for hiding this comment

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

Overall LGTM
Please run ./gradlew spotlessApply to fix the CI.

@UladzislauBlok
Copy link
Contributor Author

UladzislauBlok commented Feb 17, 2026

@mjsax @aliehsaeedii
I found out that my initial approach to finding all locations needing fixes was incorrect (I was checking all calls to serialize/deserialize methods). There are still a few places in the org.apache.kafka.streams.kstream package to address. Since this PR has already been reviewed, I would merge it now and address the remaining places in follow-up PRs. Description was changed accordingly

@frankvicky frankvicky merged commit 448338c into apache:trunk Feb 18, 2026
25 checks passed
@frankvicky
Copy link
Contributor

@UladzislauBlok Thanks for the PR, merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants