Skip to content

KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes #17882

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
Merged
merged 16 commits into from
Dec 9, 2024
Merged

KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes #17882

merged 16 commits into from
Dec 9, 2024

Conversation

fonsdant
Copy link
Contributor

No description provided.

@fonsdant fonsdant force-pushed the kstream-flat-transform-values-remove-deprecated-transformer-methods-and-classes branch from 8088b56 to 3b42776 Compare November 26, 2024 00:29
@fonsdant fonsdant marked this pull request as ready for review November 26, 2024 01:46
@fonsdant
Copy link
Contributor Author

Hi, @mjsax! I have a question for this PR. I'm wondering if shouldNotAllowNullStoreNameOnProcessValues were not duplicated as shouldNotAllowNullStoreNamesOnProcessValues (note the little "s" in "Names"). What do you think?

@mjsax
Copy link
Member

mjsax commented Nov 27, 2024

Looking into the test code, it seem the s just goes somewhere else? There is shouldNotAllowNullStoreNameOnProcess and shouldNotAllowNullStoreName[s]OnProcess, one taking (String) null and the other taking a (String[]) null, so it seems we are fine?

@fonsdant fonsdant force-pushed the kstream-flat-transform-values-remove-deprecated-transformer-methods-and-classes branch from 5b8bbc8 to 0b18c5a Compare November 27, 2024 10:06
@fonsdant
Copy link
Contributor Author

Looking into the test code, it seem the s just goes somewhere else? There is shouldNotAllowNullStoreNameOnProcess and shouldNotAllowNullStoreName[s]OnProcess, one taking (String) null and the other taking a (String[]) null, so it seems we are fine?

Thanks, @mjsax! Alright, just a confirmation.

@fonsdant fonsdant force-pushed the kstream-flat-transform-values-remove-deprecated-transformer-methods-and-classes branch from 0b18c5a to 075fa2d Compare December 2, 2024 01:03
mjsax
mjsax reviewed Dec 3, 2024
View reviewed changes
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.

Thanks for the PR. Made a pass. Think we can remove a little more code, which is unused now.

streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImpl.java
final String... stateStoreNames) {
Objects.requireNonNull(valueTransformerSupplier, "valueTransformerSupplier can't be null");
return doFlatTransformValues(
toValueTransformerWithKeySupplier(valueTransformerSupplier),
Copy link
Member

Choose a reason for hiding this comment

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

I think we can also remove toValueTransformerWithKeySupplier(...) ? Should be unused now?

streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImpl.java
for (final String stateStoreName : stateStoreNames) {
Objects.requireNonNull(stateStoreName, "stateStoreNames can't contain `null` as store name");
}
ApiUtils.checkSupplier(valueTransformerWithKeySupplier);
Copy link
Member

Choose a reason for hiding this comment

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

I think we can also remove this checkSupplier(...) overload from ApiUtils ?

@mjsax
Copy link
Member

mjsax commented Dec 6, 2024

@fonsdant -- This PR need a rebase to resolve conflicts. Would also be great if you could also address the two minor comments from the review, so we can merge this PR.

@fonsdant
Copy link
Contributor Author

fonsdant commented Dec 6, 2024 •
edited
Loading

Thanks, @mjsax! Sorry for the delay this week. I can push the changes by tomorrow.

fonsdant added 16 commits December 7, 2024 23:04
@fonsdant
refactor(streams)!: remove flatTransformValues from KStream
bf99c68 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
refactor(streams)!: remove flatTransformValues from KStreamImpl
4c8b54a 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
test(streams): remove flatTransformValues from StreamsBuilderTest
555dd46 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
test(streams): remove flatTransformValues from KStreamImplTest
c3ed9f0 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
refactor(streams-scala)!: remove flatTransformValues from KStream
d0faf59 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
docs(streams)!: replace flatTransformValues with processValues in KSt…
08913d0 
…ream

Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
docs(streams)!: replace flatTransformValues with processValues in Con…
4d29a0b 
…nectedStoreProvider

Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
test(streams): remove KStreamFlatTransformValuesTest
da71098 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
refactor(streams)!: remove KStreamFlatTransformValues
394b463 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
test(streams): remove KStreamFlatTransformTest
bbc3db7 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
refactor(streams)!: remove KStreamFlatTransform
268e2e2 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
refactor(streams-scala): fix spotless scala imports
6a9ccc9 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
test(streams-scala): add KStreamTest#testProcessValuesCorrectlyRecords
fe1c5ed 
Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
test(streams): add StreamsBuilderTest#shouldUseSpecifiedNameForProces…
365707f 
…sValuesOperation

Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
refactor(streams)!: remove AbstractStream#toValueTransformerWithKeySu…
6c3dfb9 
…pplier

Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant
refactor(streams)!: remove ApiUtils#checkSupplier with ValueTransform…
f9d2bf8 
…erSupplier

Signed-off-by: Joao Pedro Fonseca Dantas <[email protected]>
@fonsdant fonsdant force-pushed the kstream-flat-transform-values-remove-deprecated-transformer-methods-and-classes branch from 075fa2d to f9d2bf8 Compare December 8, 2024 02:29
@fonsdant fonsdant requested a review from mjsax December 8, 2024 14:10
@mjsax mjsax merged commit d5c2029 into apache:trunk Dec 9, 2024
8 checks passed
@mjsax
Copy link
Member

mjsax commented Dec 9, 2024

Thanks for the PR. Merged to trunk.

peterxcli pushed a commit to peterxcli/kafka that referenced this pull request Dec 18, 2024
@fonsdant @peterxcli
KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "tra…
ec1146a 
…nsformer" methods and classes (apache#17882)

Reviewer: Matthias J. Sax <[email protected]>
@mjsax
Copy link
Member

mjsax commented Dec 19, 2024

@fonsdant -- Would you be interested to help us updating the docs?

Eg https://kafka.apache.org/39/documentation/streams/developer-guide/dsl-api.html#applying-processors-and-transformers-processor-api-integration -- there might be other parts, too, which we need to update.

It would also be good to maybe add a guide (ie, some simple examples) how to rewrite old code for the new process() method we have?

Besides [flat]Transform[Values] stuff, we did remove a lot of other API for 4.0. If you are interested to help updating the docs for these, too, would be great.

Don't feel obligated. Just let us know. -- If you don't have interest, we can also work through it on our side.

@fonsdant
Copy link
Contributor Author

@mjsax, of course! Thanks for mentioning me! Have we already a task for this or should I create a minor PR?

@mjsax
Copy link
Member

mjsax commented Dec 20, 2024

MINOR PR is fine -- if you do a dedicated PR only for [flat]Transform[Values], you could also use KAFKA-16339.

@fonsdant fonsdant deleted the kstream-flat-transform-values-remove-deprecated-transformer-methods-and-classes branch December 22, 2024 23:22
tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
@fonsdant @tedyu
KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "tra…
2e35f0e 
…nsformer" methods and classes (apache#17882)

Reviewer: Matthias J. Sax <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjsax mjsax Awaiting requested review from mjsax

Assignees
No one assigned
Labels
ci-approved streams
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fonsdant @mjsax