You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "fonsdant (via GitHub)" <gi...@apache.org> on 2024/11/20 18:17:14 UTC
[PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
fonsdant opened a new pull request, #17882:
URL: https://github.com/apache/kafka/pull/17882
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax merged PR #17882:
URL: https://github.com/apache/kafka/pull/17882
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "fonsdant (via GitHub)" <gi...@apache.org>.
fonsdant commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2499405990
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2526845764
Thanks for the PR. Merged to `trunk`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2522152860
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "fonsdant (via GitHub)" <gi...@apache.org>.
fonsdant commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2503456985
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2502349548
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "fonsdant (via GitHub)" <gi...@apache.org>.
fonsdant commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2522714820
Thanks, @mjsax! Sorry for the delay this week. I will push the changes through tomorrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "fonsdant (via GitHub)" <gi...@apache.org>.
fonsdant commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2555840955
@mjsax, of course! Thanks for mentioning me! Have we already a task for this or should I create a minor PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2555521111
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #17882:
URL: https://github.com/apache/kafka/pull/17882#discussion_r1866856763
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImpl.java:
##########
@@ -1204,75 +1201,6 @@ private <VO, VR> KStream<K, VR> doStreamTableJoin(final KTable<K, VO> table,
builder);
}
- @Override
- @Deprecated
- public <VR> KStream<K, VR> flatTransformValues(final org.apache.kafka.streams.kstream.ValueTransformerSupplier<? super V, Iterable<VR>> valueTransformerSupplier,
- final String... stateStoreNames) {
- Objects.requireNonNull(valueTransformerSupplier, "valueTransformerSupplier can't be null");
- return doFlatTransformValues(
- toValueTransformerWithKeySupplier(valueTransformerSupplier),
- NamedInternal.empty(),
- stateStoreNames);
- }
-
- @Override
- @Deprecated
- public <VR> KStream<K, VR> flatTransformValues(final org.apache.kafka.streams.kstream.ValueTransformerSupplier<? super V, Iterable<VR>> valueTransformerSupplier,
- final Named named,
- final String... stateStoreNames) {
- Objects.requireNonNull(valueTransformerSupplier, "valueTransformerSupplier can't be null");
- return doFlatTransformValues(
- toValueTransformerWithKeySupplier(valueTransformerSupplier),
- named,
- stateStoreNames);
- }
-
- @Override
- @Deprecated
- public <VR> KStream<K, VR> flatTransformValues(final ValueTransformerWithKeySupplier<? super K, ? super V, Iterable<VR>> valueTransformerSupplier,
- final String... stateStoreNames) {
- Objects.requireNonNull(valueTransformerSupplier, "valueTransformerSupplier can't be null");
- return doFlatTransformValues(valueTransformerSupplier, NamedInternal.empty(), stateStoreNames);
- }
-
- @Override
- @Deprecated
- public <VR> KStream<K, VR> flatTransformValues(final ValueTransformerWithKeySupplier<? super K, ? super V, Iterable<VR>> valueTransformerSupplier,
- final Named named,
- final String... stateStoreNames) {
- Objects.requireNonNull(valueTransformerSupplier, "valueTransformerSupplier can't be null");
- return doFlatTransformValues(valueTransformerSupplier, named, stateStoreNames);
- }
-
- private <VR> KStream<K, VR> doFlatTransformValues(final ValueTransformerWithKeySupplier<? super K, ? super V, Iterable<VR>> valueTransformerWithKeySupplier,
- final Named named,
- final String... stateStoreNames) {
- Objects.requireNonNull(stateStoreNames, "stateStoreNames can't be a null array");
- for (final String stateStoreName : stateStoreNames) {
- Objects.requireNonNull(stateStoreName, "stateStoreNames can't contain `null` as store name");
- }
- ApiUtils.checkSupplier(valueTransformerWithKeySupplier);
Review Comment:
I think we can also remove this `checkSupplier(...)` overload from `ApiUtils` ?
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImpl.java:
##########
@@ -1204,75 +1201,6 @@ private <VO, VR> KStream<K, VR> doStreamTableJoin(final KTable<K, VO> table,
builder);
}
- @Override
- @Deprecated
- public <VR> KStream<K, VR> flatTransformValues(final org.apache.kafka.streams.kstream.ValueTransformerSupplier<? super V, Iterable<VR>> valueTransformerSupplier,
- final String... stateStoreNames) {
- Objects.requireNonNull(valueTransformerSupplier, "valueTransformerSupplier can't be null");
- return doFlatTransformValues(
- toValueTransformerWithKeySupplier(valueTransformerSupplier),
- NamedInternal.empty(),
- stateStoreNames);
- }
-
- @Override
- @Deprecated
- public <VR> KStream<K, VR> flatTransformValues(final org.apache.kafka.streams.kstream.ValueTransformerSupplier<? super V, Iterable<VR>> valueTransformerSupplier,
- final Named named,
- final String... stateStoreNames) {
- Objects.requireNonNull(valueTransformerSupplier, "valueTransformerSupplier can't be null");
- return doFlatTransformValues(
- toValueTransformerWithKeySupplier(valueTransformerSupplier),
Review Comment:
I think we can also remove `toValueTransformerWithKeySupplier(...)` ? Should be unused now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16339: [4/4 KStream#flatTransformValues] Remove Deprecated "transformer" methods and classes [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #17882:
URL: https://github.com/apache/kafka/pull/17882#issuecomment-2556004798
MINOR PR is fine -- if you do a dedicated PR only for `[flat]Transform[Values]`, you could also use KAFKA-16339.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org