> On May 15, 2015, 8:26 p.m., Michael Herstine wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java, 
> > lines 137-139
> > <https://reviews.apache.org/r/33620/diff/5/?file=957064#file957064line137>
> >
> >     This is interesting; I don't see a corresponding 
> > `createSSLSocketFactory`-- did I miss it? Besides, I thought Kafka used NIO 
> > (in which case you wouldn't be using `SSLServerSocketFactory`, which IIRC 
> > is blocking. Who calls this?

This method is used for tests and I moved it under tests.


> On May 15, 2015, 8:26 p.m., Michael Herstine wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  lines 56-59
> > <https://reviews.apache.org/r/33620/diff/5/?file=957065#file957065line56>
> >
> >     Do you want to make your nomenclature more uniform? The network buffers 
> > are referred to in terms of "in" & "out" while the application buffers are 
> > referred to as "read" & "write".
> >     
> >     On a related note, do you need read & write for each? At any given 
> > point in time, you're doing one or the other, so why not just have a 
> > "network" buffer & an "application" buffer?

Even though we do one op at any given time we still want two buffer as we might 
not have read the entire buffer in op invocation and we need to keep this data 
around. Having two buffers for read and write makes it easier.


> On May 15, 2015, 8:26 p.m., Michael Herstine wrote:
> > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java,
> >  line 76
> > <https://reviews.apache.org/r/33620/diff/5/?file=957065#file957065line76>
> >
> >     I'm confused on the difference between `startHandshake` & `handshake` 
> > (inherited from interface `TransportLayer`).
> 
> Sriharsha Chintalapani wrote:
>     startHandshake is more of a prepare method for handshake process and its 
> only SSLTransportLayer and I'll make that as a private method. 
>     handshake method is for general TransportLayer handshake . As indicated 
> in comments for PlainTextChannel its a non-op and for SSL its implemented as 
> non-blocking ssl handshake.

I made startHandshake to private and its prepare method when the transportLayer 
gets created. If you see the naming of the method confusing please re-open.


- Sriharsha


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33620/#review83807
-----------------------------------------------------------


On May 20, 2015, 9:54 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> -----------------------------------------------------------
> 
> (Updated May 20, 2015, 9:54 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1690
>     https://issues.apache.org/jira/browse/KAFKA-1690
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. SSLFactory tests.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Added 
> PrincipalBuilder.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Addressing 
> reviews.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Addressing 
> reviews.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Addressing 
> reviews.
> 
> 
> Diffs
> -----
> 
>   build.gradle cd2aa838fd53e8124f308979b1d70efe0c5725a6 
>   checkstyle/checkstyle.xml a215ff36e9252879f1e0be5a86fef9a875bb8f38 
>   checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 
>   clients/src/main/java/org/apache/kafka/clients/ClientUtils.java 
> 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a 
>   clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java 
> cf32e4e7c40738fe6d8adc36ae0cfad459ac5b0b 
>   clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 
> bdff518b732105823058e6182f445248b45dc388 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> d301be4709f7b112e1f3a39f3c04cfa65f00fa60 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 8e336a3aa96c73f52beaeb56b931baf4b026cf21 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 
> 187d0004c8c46b6664ddaffecc6166d4b47351e5 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> c4fa058692f50abb4f47bd344119d805c60123f5 
>   clients/src/main/java/org/apache/kafka/common/config/SecurityConfigs.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/Authenticator.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/Channel.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/ChannelBuilder.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/DefaultAuthenticator.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlainTextChannelBuilder.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlainTextTransportLayer.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/SSLChannelBuilder.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/Selectable.java 
> b5f8d83e89f9026dc0853e5f92c00b2d7f043e22 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 57de0585e5e9a53eb9dcd99cac1ab3eb2086a302 
>   clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/protocol/SecurityProtocol.java 
> dab1a94dd29563688b6ecf4eeb0e180b06049d3f 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/DefaultPrincipalBuilder.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/PrincipalBuilder.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> f73eedb030987f018d8446bb1dcd98d19fa97331 
>   clients/src/test/java/org/apache/kafka/common/network/EchoServer.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SSLFactoryTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SSLSelectorTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
> d5b306b026e788b4e5479f3419805aa49ae889f3 
>   clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 
> 2ebe3c21f611dc133a2dbb8c7dfb0845f8c21498 
>   clients/src/test/java/org/apache/kafka/test/TestSSLUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33620/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>

Reply via email to