Thanks for the PR. A couple of comments: 1) For other public interfaces, we use parameters to configure instead of internal config names going into the same list of configs. Kafka has public config names and custom config names that don't conflict with public config names. These internal config names fall into neither category.
An example of how we do this in other classes is Deserializer: void configure(Map<String, ?> configs, boolean isKey); 2) I am not sure `SslFactory` as it stands today is the right class to turn into a pluggable class. I can see why we want to have a custom class to configure SSLEngines. But I don't think we want a custom class to decide the logic for dynamic reconfiguration or decide whether to validate against trust stores. It would perhaps be better to have a class to just create SSLContext and possibly configure SSLEngine with additional options is there is a good use case for that. But the non-pluggable class in Kafka should deal with all the reconfiguration and validation logic. Thoughts? On Tue, Oct 23, 2018 at 12:40 AM Pellerin, Clement <clement_pelle...@ibi.com> wrote: > If it is up to me, I'd say the KIP is now finalized. Can I call for a > [VOTE] or I need more feedback? > > -----Original Message----- > From: Harsha Chintalapani [mailto:ka...@harsha.io] > Sent: Monday, October 22, 2018 4:36 PM > To: dev@kafka.apache.org > Subject: RE: [DISCUSS] KIP-383 Pluggable interface for SSL Factory > > Thanks for the changes. KIP looks good to me. > > -Harsha > On Oct 22, 2018, 12:57 PM -0700, Pellerin, Clement < > clement_pelle...@ibi.com>, wrote: > > I updated the KIP. The proposed interface is now named SslFactory and > the default implementation is DefaultSslFactory. > > I also mention the existing non-default constructors will be removed. > > The constants for the new config keys are now declared in the interface > itself. > > Please give me your feedback on this new version. > > > > -----Original Message----- > > From: Harsha Chintalapani [mailto:ka...@harsha.io] > > Sent: Friday, October 19, 2018 6:51 PM > > To: dev@kafka.apache.org; dev@kafka.apache.org > > Subject: RE: [DISCUSS] KIP-383 Pluggable interface for SSL Factory > > > > SslFactory is not a public interface for others to use. EchoServer is > internal testing. > > We should make these as proposed in rejected alternatives to SslFactory > and DefaultSslFactory. > > I don’t see any one using a internal class as public API. > > > > -Harsha >
