You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "raymcdermott (via GitHub)" <gi...@apache.org> on 2024/07/07 09:42:27 UTC
[PR] KAFKA-17091: Add @FunctionalInterface to Streams interfaces [kafka]
raymcdermott opened a new pull request, #16544:
URL: https://github.com/apache/kafka/pull/16544
### Goal
Add @FunctionalInterface to Streams interfaces to enable Clojure 1.12 (and any other JVM language) to have the correct hints for these SAM interfaces.
The Clojure issue is [here ](https://ask.clojure.org/index.php/13908/expand-fi-adapting-to-sam-types-not-marked-as-fi)
### Testing
Unit tests and system tests all pass locally.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2635629948
This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a review.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2336439767
@raymcdermott -- what is the status? Seems the KIP is not making much progress lately?
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2235215843
> I attempted to have a maximal set but some of the tests broke so I pared it back to the minimum set.
Curious to learn about this. I don't understand how adding an annotation could break a test?
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2847788380
Fixed via https://github.com/apache/kafka/pull/19234
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "raymcdermott (via GitHub)" <gi...@apache.org>.
raymcdermott commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2236187306
> > Many of the mentioned PAPI interfaces do not qualify as @FunctionalInterface as they have > 1 method. Ref the [Java API docs](https://docs.oracle.com/javase/8/docs/api/java/util/function/package-summary.html#package.description)
>
> But the JavaDocs says: `Each functional interface has a single abstract method, called the functional method for that functional interface, to which the lambda expression's parameter and return types are matched or adapted.` -- My interpretation is that "single abstract method" means, **only one** abstract method, but it does not mean **exactly one** abstract method (ie, there can also be non-abstract methods)? It's only not a functional interface if there is zero, or more than one abstract methods? So an interface with three method, and two having a default imply, would still qualify as functional interface? -- Or is my interpretation incorrect?
TIL that `default` methods would not count so I will check again. Thanks for the education.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "bbejeck (via GitHub)" <gi...@apache.org>.
bbejeck commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2214141652
Hi @raymcdermott thanks for the PR! Since this is a change to the public interface code of Kafka Streams, this PR will require a [KIP](https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals). Ping me here once you create a wiki ID (step 2 from the page referenced above) and I'll go in and give you edit permissions to create a KIP.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "raymcdermott (via GitHub)" <gi...@apache.org>.
raymcdermott commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2236189673
> > I attempted to have a maximal set but some of the tests broke so I pared it back to the minimum set.
>
> Curious to learn about this. I don't understand how adding an annotation could break a test?
Will write it up if it happens again as I add more `@FunctionalInterface` annotations.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2571488966
This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.
<p>
If you are having difficulty finding a reviewer, please reach out on the
[mailing list](https://kafka.apache.org/contact).
<p>
If this PR is no longer valid or desired, please feel free to close it. If no activity
occurs in the next 30 days, it will be automatically closed.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "raymcdermott (via GitHub)" <gi...@apache.org>.
raymcdermott commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2233226348
> It seems `ForeachAction` is missing?
>
> Also wondering about `TransformerSupplier` (and siblings) -- are they not effectively deprecated via https://cwiki.apache.org/confluence/display/KAFKA/KIP-820%3A+Extend+KStream+process+with+new+Processor+API (even if the KIP does not say so expliclity -- what might be a miss on our end to add to the KIP... Thoughts)
>
> What about other PAPI interfaces like `FixedKeyProcessor`, `api.Processor` (not sure if both quality or not), `Punctuator`, `StateRestoreCallback` (maybe?), `StreamPartitioner`, `TimestampExtractor`, `TopicNameExtractor`, and IQ interface `Query`?
>
> Not sure if there might be other I have missed.
I attempted to have a maximal set but some of the tests broke so I pared it back to the minimum set.
I will however check those other interfaces to see if the annotation will break anything.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "raymcdermott (via GitHub)" <gi...@apache.org>.
raymcdermott commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2236167653
> > Also wondering about TransformerSupplier (and siblings) -- are they not effectively deprecated via https://cwiki.apache.org/confluence/display/KAFKA/KIP-820%3A+Extend+KStream+process+with+new+Processor+API (even if the KIP does not say so expliclity -- what might be a miss on our end to add to the KIP... Thoughts)
> > Libraries use those deprecated functions so I would like to include them.
>
> We do plan to remove many of these things in 4.0 (I should have linked both ticket: https://issues.apache.org/jira/browse/KAFKA-12829 and https://issues.apache.org/jira/browse/KAFKA-16339) -- Thus, it might not be worth to cover them with the KIP -- but it might be good to drop a sentence in the KIP why we don't consider them (just for documentation purpose, and to have a "complete" KIP)
I'm a bit confused about the guidance here. Are you saying that we should exclude the currently deprecated interfaces and target this KIP against 4.0?
That's a bit worrying to me because those things have not yet been removed and there may be reasons for not removing them.
I was hoping that this KIP would be simple but trapping it in the timelines of releases adds a lot of complexity IMHO
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2214996644
It seems `ForeachAction` is missing?
Also wondering about `TransformerSupplier` (and siblings) -- are they not effectively deprecated via https://cwiki.apache.org/confluence/display/KAFKA/KIP-820%3A+Extend+KStream+process+with+new+Processor+API (even if the KIP does not say so expliclity -- what might be a miss on our end to add to the KIP... Thoughts)
What about other PAPI interfaces like `FixedKeyProcessor`, `api.Processor` (not sure if both quality or not), `Punctuator`, `StateRestoreCallback` (maybe?), `StreamPartitioner`, `TimestampExtractor`, `TopicNameExtractor`, and IQ interface `Query`?
Not sure if there might be other I have missed.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "raymcdermott (via GitHub)" <gi...@apache.org>.
raymcdermott commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2233542645
> Also wondering about `TransformerSupplier` (and siblings) -- are they not effectively deprecated via https://cwiki.apache.org/confluence/display/KAFKA/KIP-820%3A+Extend+KStream+process+with+new+Processor+API (even if the KIP does not say so expliclity -- what might be a miss on our end to add to the KIP... Thoughts)
Libraries use those deprecated functions so I would like to include them.
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #16544: KAFKA-17091: Add @FunctionalInterface to Streams interfaces
URL: https://github.com/apache/kafka/pull/16544
--
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-17091: Add @FunctionalInterface to Streams interfaces [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #16544:
URL: https://github.com/apache/kafka/pull/16544#issuecomment-2237345525
> I'm a bit confused about the guidance here. Are you saying that we should exclude the currently deprecated interfaces and target this KIP against 4.0?
>
> That's a bit worrying to me because those things have not yet been removed and there may be reasons for not removing them.
>
> I was hoping that this KIP would be simple but trapping it in the timelines of releases adds a lot of complexity IMHO
That's a fair statement. Just wanted to point it out. If your KIP gets approved and we really remove them in 4.0, it was just busy work. (And I am 99% certain that we will remove it in 4.0). -- Just wanted to avoid any surprises. -- The other aspect is: even if the methods are not removed in 4.0, the would not be any damage if we exclude them in your KIP IMHO. The methors are deprecated and should not be use any longer, so there is no need to improve them. Not adding the annotation to them could actually be a forcing function for users to use the new methods instead of the deprecated ones.
If you want to include it in the KIP, fine with me, too. Leave it up to you. (With a slight preference to exclude them.)
> TIL that default methods would not count so I will check again. Thanks for the education.
That's my understanding -- but I could be wrong. Would be good to verify and double check this.
--
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