Hi Maulin, Clement brought up a good point, which is that we should have agreement on the overall design before we start looking at PRs. In Kafka, that means starting a discussion on a KIP, and then getting that KIP voted in.
There is more information about the KIP process here: https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals Feel free to ask more detailed questions about the process on the mailing list as well :) It sounds like Clement is proposing that you create a new KIP rather than reusing KIP-383. So you should probably either repurpose KIP-486 for this, or choose a new number. Both options seem reasonable here. As far as I can see, once we implement a pluggable SslEngineBuilder, KIP-383 would no longer be needed, and so we would put that one into "rejected KIPs." In general, coming up with good APIs can be harder than it might seem at first. Once we add an API, we have to assume that we're going to support it forever. Deprecating an API can take a very long time, and typically requires a new major release of Kafka. Therefore, sometimes it's worth making users go through a little bit of copying and pasting in order to avoid creating an API that would constrain the project in the future. Hopefully, we can come up with something good here that will be useful to everyone. best, Colin On Fri, Sep 6, 2019, at 07:48, Pellerin, Clement wrote: > This is the way I see it, which is in no way authoritative. > > First you have to find a complete solution, then you need to document > it in a KIP and that KIP needs to pass a vote. Any votes before the KIP > vote starts is meaningless. > > As for the ownership and authorship of the KIPs, I don't plan to work > on this, so KIP-383 is better left the way it is. I would prefer if you > would update your KIP or maybe create a new one. We can mark KIP-383 as > obsolete after your KIP passes its vote. > > -----Original Message----- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, September 6, 2019 2:36 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-486 Support for pluggable KeyStore and TrustStore > > Hi all > > Any input on my last question about the process? > > Also, I have updated the code based on the feedback so far at: > https://github.com/maulin-vasavada/kafka/pull/2/files. Still have to figure > out how to plug keys/certs loading while using DefaultSslEngineFactory's > implementation (still need to make it final). > > Thanks > Maulin > > > On Thu, Sep 5, 2019 at 4:34 PM Maulin Vasavada <maulin.vasav...@gmail.com> > wrote: > > > Hi all > > > > It seems we are in some sort of agreement so far apart from code > > review/comments. However, I have a fundamental question - going forward how > > this work from the process standpoint? What do we do with this KIP-486 vs > > KIP-383? I feel that both needs to come together since in order to make > > Pluggable key/trust store on top of making SslEngineBuilder pluggable we > > will need changes suggested by KIP-486 with some differences to the > > original proposal. It would great if someone can help us clarify the next > > steps. > > > > Thanks > > Maulin > > > > On Wed, Sep 4, 2019 at 1:54 PM Maulin Vasavada <maulin.vasav...@gmail.com> > > wrote: > > > >> Do you guys think it would be easier if you can provide comments on > >> GitHub and we can continue there and summarize the conclusion here? > >> > >> We should not lose addressing any comments. > >> > >> On Wed, Sep 4, 2019 at 12:34 PM Pellerin, Clement < > >> clement_pelle...@ibi.com> wrote: > >> > >>> The proposed interface does not look like the Builder pattern I am used > >>> to. > >>> Should SslEngineBuilder be called SslEngineFactory instead? > >>> > >>> On Mon, Sep 2, 2019, at 03:33, Rajini Sivaram wrote: > >>> > I would expect SslEngineBuilder interface to look something like this, > >>> > perhaps with some tweaking: > >>> > > >>> > public interface SslEngineBuilder extends Configurable, Closeable { > >>> > > >>> > Set<String> reconfigurableConfigs(); > >>> > > >>> > boolean shouldBeRebuilt(Map<String, Object> nextConfigs); > >>> > > >>> > SSLEngine createSslEngine(Mode mode, String peerHost, int > >>> > peerPort, String endpointIdentification); > >>> > > >>> > } > >>> > >> >
