Skip to content

KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces#18238

Merged
mjsax merged 4 commits intoapache:trunkfrom
mjsax:kafka-12829-remove-papi-helper-classes
Dec 20, 2024
Merged

KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces#18238
mjsax merged 4 commits intoapache:trunkfrom
mjsax:kafka-12829-remove-papi-helper-classes

Conversation

@mjsax
Copy link
Copy Markdown
Member

@mjsax mjsax commented Dec 17, 2024 •
edited
Loading

Must be cherry-picked to 4.0 branch.

@mjsax mjsax added the streams label Dec 17, 2024
@chia7712
Copy link
Copy Markdown
Member

chia7712 commented Dec 18, 2024

@mjsax could you please fix build error?

@chia7712
Copy link
Copy Markdown
Member

chia7712 commented Dec 18, 2024

The latest commits include a trunk rebase and build fix.

@mjsax mjsax changed the title KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces [DO NOT MERGE] KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces Dec 18, 2024
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Dec 18, 2024

@chia7712 -- This PR is stacked on another one anyway, and need rebasing before we can finalize reviewing and merge it.

I hope that you pushing into my branch, does not make rebasing more difficult now... Why did you push into my branch? There was not really a good reason for it...

@chia7712
Copy link
Copy Markdown
Member

chia7712 commented Dec 18, 2024 •
edited
Loading

I hope that you pushing into my branch, does not make rebasing more difficult now... Why did you push into my branch? There was not really a good reason for it...

Apologies for the intrusion. I tried to expedite the CI process by using a merge push instead of a rebase. I assumed the merge policy is easier for you to pull the code locally (just call git pull). Additionally, it is easy to revert it if you don't like those commits. I won't do that again, as it causes you trouble :(

bbejeck
bbejeck approved these changes Dec 18, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

LGTM - just one question on one of the Javadocs

Comment thread streams/src/main/java/org/apache/kafka/streams/processor/ProcessorContext.java
* {@link Processor#init(ProcessorContext) initialization} or
* {@link Processor#process(Object, Object) processing} to
* {@link org.apache.kafka.streams.kstream.ValueTransformer#init(ProcessorContext) initialization} or
* {@link org.apache.kafka.streams.kstream.ValueTransformer#transform(Object) processing} to
Copy link
Copy Markdown
Member

@bbejeck bbejeck Dec 18, 2024

Choose a reason for hiding this comment

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

Why mention ValueTransformer? Is this intentional?

Copy link
Copy Markdown
Member Author

@mjsax mjsax Dec 19, 2024

Choose a reason for hiding this comment

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

Yes. ValueTransformer (while deprecated itself) is the last class which is still used via KTAbel#transformValues() which we did not yet deprecate... (There is ticket for it already: https://issues.apache.org/jira/browse/KAFKA-17178)

So we cannot remove ValueTransformer yet, and it make sense to refer to it here IMHO.

@mjsax mjsax force-pushed the kafka-12829-remove-papi-helper-classes branch from b055790 to 5fd4378 Compare December 20, 2024 00:37
@mjsax mjsax changed the title [DO NOT MERGE] KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces Dec 20, 2024
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Dec 20, 2024

Rebased to trunk. Plan to merge after CI passed if I don't hear otherwise.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Dec 20, 2024

@chia7712 @bbejeck -- I just pushed small change -- To helper class was updated incorrectly, and I reverted the change. The JavaDocs for To are correct as-is, and don't need any update. Just an FYI.

@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Dec 20, 2024

Java 23 passed.

Java 17:

Found 3 test failures:
FAILED ❌ RemoteLogManagerTest > testRLMOpsWhenMetadataIsNotReady()
FAILED ❌ PlaintextAdminIntegrationTest > testConsumerGroupsDeprecatedConsumerGroupState(String, String).quorum=kraft.groupProtocol=consumer
FAILED ❌ PlaintextAdminIntegrationTest > testConsumerGroups(String, String).quorum=kraft.groupProtocol=consumer

@mjsax mjsax merged commit f7ae4e8 into apache:trunk Dec 20, 2024
@mjsax mjsax deleted the kafka-12829-remove-papi-helper-classes branch December 20, 2024 06:26
mjsax added a commit that referenced this pull request Dec 20, 2024
@mjsax
KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces (#…
8afe9b9 
…18238)

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, Bill Bejeck <bill@confluent.io>
@mjsax
Copy link
Copy Markdown
Member Author

mjsax commented Dec 20, 2024

Merged to trunk and cherry-picked to 4.0 branch.

ijuma added a commit to ijuma/kafka that referenced this pull request Dec 20, 2024
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into kip-896-remov…
31a4619 
…e-old-protocol-versions

* apache-github/trunk:
  KAFKA-18312: Added entityType: topicName to SubscribedTopicNames in ShareGroupHeartbeatRequest.json (apache#18285)
  HOTFIX: fix incompatible types: Optional<TimestampAndOffset> cannot be converted to Option<TimestampAndOffset> (apache#18284)
  MINOR Fix some test-catalog issues (apache#18272)
  KAFKA-18180: Move OffsetResultHolder to storage module (apache#18100)
  KAFKA-18301; Make coordinator records first class citizen (apache#18261)
  KAFKA-18262 Remove DefaultPartitioner and UniformStickyPartitioner (apache#18204)
  KAFKA-18296 Remove deprecated KafkaBasedLog constructor (apache#18257)
  KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces (apache#18238)
  KAFKA-18292 Remove deprecated methods of UpdateFeaturesOptions (apache#18245)
  KAFKA-12829: Remove deprecated Topology#addProcessor of old Processor API (apache#18154)
  KAFKA-18035, KAFKA-18306, KAFKA-18092: Address TransactionsTest flaky tests (apache#18264)
  MINOR: change the default linger time in the new coordinator (apache#18274)
  KAFKA-18305: validate controller.listener.names is not in inter.broker.listener.name for kcontrollers (apache#18222)
  KAFKA-18207: Serde for handling transaction records (apache#18136)
  KAFKA-13722: Refactor Kafka Streams store interfaces (apache#18243)
  KAFKA-17131: Refactor TimeDefinitions (apache#18241)
  MINOR: Fix MessageFormatters (apache#18266)
  Mark flaky tests for Dec 18, 2024 (apache#18263)
tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
@mjsax @tedyu
KAFKA-12829: Remove old Processor and ProcessorSupplier interfaces (a…
008484b 
…pache#18238)

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, Bill Bejeck <bill@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bbejeck bbejeck bbejeck approved these changes

@chia7712 chia7712 chia7712 approved these changes

Assignees

No one assigned

Labels

streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mjsax @chia7712 @bbejeck