Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 19, 2015, 1:56 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/utils/Utils.java, line 520 > > > > > > second flip after `BUFFER_OVERFLOW` in `handshakeWrap` - you had > > mention

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 19, 2015, 1:56 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/utils/Utils.java, line 520 > > > > > > second flip after `BUFFER_OVERFLOW` in `handshakeWrap` - you had > > mention

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review95765 --- I haven't finished going through the latest patch, but discussed con

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated Aug. 19, 2015, 12:24 a.m.) Review request for kafka. Bugs: KAFKA-169

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review95798 --- Ship it! Looks good to me. Could you rebase? - Jun Rao On Aug. 1

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 18, 2015, 4:09 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 438 > > > > > > Hmm, do we want to break here? It seems that we want to conti

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated Aug. 18, 2015, 6:24 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 18, 2015, 4:09 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 450 > > > > > > The termination condition doesn't seem quite right. If dst ha

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 368-381 > > > > > > If handshake status is BUFFER_OVERFLOW, we will return to

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review95671 --- Ship it! Thanks for the latest patch. +1. I only have a few minor c

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated Aug. 17, 2015, 7:21 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review95619 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated Aug. 17, 2015, 4:28 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review95589 --- Ship it! Thanks for the various fixes. There is still a small issue

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated Aug. 17, 2015, 3:13 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review95582 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote: > > clients/src/main/java/org/apache/kafka/common/config/SSLConfigs.java, line > > 29 > > > > > > SSL is deprecated > > (https://ma.ttias.be/rfc-7568-ssl-3-0-is-now

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated Aug. 17, 2015, 3:41 a.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, line 867 > > > > > > Why are we using a Java Map here? Is it used in Java code? If not, then > > why not use

Re: Review Request 33620: Patch for KAFKA-1690

> On July 31, 2015, 4:28 p.m., Ismael Juma wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 319 > > > > > > This message seems a bit unclear to me. Are we saying tha

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 417 > > > > > > It's still not very clear to me how renegotiation can be suppo

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 368-381 > > > > > > If handshake status is BUFFER_OVERFLOW, we will return to

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 306-320 > > > > > > It seems that the logic here can be simpler. In handshake

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 262-269 > > > > > > Could we transition from NEED_WRAP to NOT_HANDSHAKING dir

Re: Review Request 33620: Patch for KAFKA-1690

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote: > > clients/src/main/java/org/apache/kafka/common/network/DefaultAuthenticator.java, > > line 34 > > > > > > Why not create the principal at this point instead of in

Re: Review Request 33620: Patch for KAFKA-1690

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote: > > clients/src/main/java/org/apache/kafka/common/config/SSLConfigs.java, line > > 29 > > > > > > SSL is deprecated > > (https://ma.ttias.be/rfc-7568-ssl-3-0-is-now

Re: Review Request 33620: Patch for KAFKA-1690

> On July 29, 2015, 1:15 a.m., Dong Lin wrote: > > clients/src/test/java/org/apache/kafka/common/network/SSLFactoryTest.java, > > line 52 > > > > > > I am not sure about this. It is probably a stupid question. Do yo

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 430-433 > > > > > > Is this needed? If we need to expand appReadBuffer, netRe

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/network/SocketServerTest.scala, line 207 > > > > > > Is this test needed given the tests we have on EchoServer? > > > > Also,

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/Selector.java, line > > 535 > > > > > > Not sure why we need to check hasSend. It's possible for a channel to

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/Selector.java, lines > > 250-251 > > > > > > It seems that we will need to further check whether those channels

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 417 > > > > > > It's still not very clear to me how renegotiation can be suppo

Re: Review Request 33620: Patch for KAFKA-1690

> On Aug. 3, 2015, 4:50 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 306-320 > > > > > > It seems that the logic here can be simpler. In handshake

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93939 --- clients/src/main/java/org/apache/kafka/clients/ClientUtils.java (li

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93862 --- Thanks for the patch. A few more comments below. build.gradle (lin

Re: Review Request 33620: Patch for KAFKA-1690

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote: > > clients/src/main/java/org/apache/kafka/common/config/SSLConfigs.java, line > > 29 > > > > > > SSL is deprecated > > (https://ma.ttias.be/rfc-7568-ssl-3-0-is-now

Re: Review Request 33620: Patch for KAFKA-1690

> On July 31, 2015, 4:32 p.m., Sriharsha Chintalapani wrote: > > core/src/test/scala/integration/kafka/api/SSLConsumerTest.scala, line 218 > > > > > > If we want to enforce this coding convention.Lets open up a new

Re: Review Request 33620: Patch for KAFKA-1690

> On July 31, 2015, 4:29 p.m., Sriharsha Chintalapani wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 162 > > > > > > hasRemaining doesn't work here. Hence the reas

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93736 --- core/src/test/scala/integration/kafka/api/SSLConsumerTest.scala (li

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93735 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93734 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93729 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93684 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

> On July 30, 2015, 10:37 p.m., Dong Lin wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 416 > > > > > > Do you mean unwrapResult.getHandshakeStatus() == > > Hands

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93652 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93639 --- clients/src/main/java/org/apache/kafka/common/config/AbstractConfig

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93332 --- clients/src/main/java/org/apache/kafka/common/network/PlainTextTran

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93177 --- clients/src/main/java/org/apache/kafka/common/config/AbstractConfig

Re: Review Request 33620: Patch for KAFKA-1690

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote: > > core/src/main/scala/kafka/api/FetchResponse.scala, line 82 > > > > > > Casts are to be avoided in Scala, pattern matching is a better way to > > do this: > >

Re: Review Request 33620: Patch for KAFKA-1690

> On July 27, 2015, 1:32 p.m., Ismael Juma wrote: > > core/src/main/scala/kafka/api/FetchResponse.scala, line 82 > > > > > > Casts are to be avoided in Scala, pattern matching is a better way to > > do this: > >

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review93110 --- I did an initial pass over the code (excluding tests) and left some

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated July 25, 2015, 7:11 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

> On July 22, 2015, 3 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 411 > > > > > > Is renegotiation actually supported? It seems that renegotiation

Re: Review Request 33620: Patch for KAFKA-1690

> On July 22, 2015, 3 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 231-234 > > > > > > Still some questions on this. > > > > 1. When hand

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review92711 --- clients/src/main/java/org/apache/kafka/common/network/ByteBufferSen

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review92599 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review92405 --- clients/src/main/java/org/apache/kafka/common/network/ByteBufferSen

Re: Review Request 33620: Patch for KAFKA-1690

> On July 22, 2015, 3 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 231-234 > > > > > > Still some questions on this. > > > > 1. When hand

Re: Review Request 33620: Patch for KAFKA-1690

> On July 22, 2015, 3 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 411 > > > > > > Is renegotiation actually supported? It seems that renegotiation

Re: Review Request 33620: Patch for KAFKA-1690

> On July 22, 2015, 3 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 361-374 > > > > > > Hmm, if status is ok and handshakeStatus is NEED_UNWRAP, co

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review92314 --- Thanks for the patch. Looks good overall. A few comments below. The

Re: Review Request 33620: Patch for KAFKA-1690

> On May 22, 2015, 12:33 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 294 > > > > > > Not sure why we are looping back here. If the HandshakeStatus is

Re: Review Request 33620: Patch for KAFKA-1690

> On June 24, 2015, 4:39 p.m., Jiangjie Qin wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 75-78 > > > > > > Can they be replaced by clear() method? These are set o

Re: Review Request 33620: Patch for KAFKA-1690

> On June 30, 2015, 5:20 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/network/Channel.java, lines > > 148-150 > > > > > > This should be caught by checkstyle - can you double-check? If we

Re: Review Request 33620: Patch for KAFKA-1690

> On June 30, 2015, 3:03 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, line 393 > > > > > > This parameter doesn't seem to be used. We can remove it. I am using for metrics > On Jun

Re: Review Request 33620: Patch for KAFKA-1690

> On June 30, 2015, 3:03 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 67 > > > > > > Shouldn't we only start the ssl handshake after the raw socket >

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated July 20, 2015, 7 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated July 20, 2015, 1:10 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

> On June 30, 2015, 3:03 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 179 > > > > > > Is this needed? It seems that the assumption is that if there i

Re: Review Request 33620: Patch for KAFKA-1690

> On June 30, 2015, 5:27 p.m., Michael Herstine wrote: > > clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java, > > line 31 > > > > > > This may be my lack of Kafak-specific knowledge, but I'm ge

Re: Review Request 33620: Patch for KAFKA-1690

> On June 30, 2015, 3:03 p.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 602 > > > > > > Not sure if we should do the flip here. The caller may not be a

Re: Review Request 33620: Patch for KAFKA-1690

> On May 15, 2015, 10:54 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 321 > > > > > > Actually, can you describe how this would be done (say, for d

Re: Review Request 33620: Patch for KAFKA-1690

> On June 30, 2015, 5:27 p.m., Michael Herstine wrote: > > clients/src/main/java/org/apache/kafka/common/network/Channel.java, lines > > 109-114 > > > > > > So you want class `Channel` to have at most one "send" pend

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review89910 --- clients/src/main/java/org/apache/kafka/clients/ClientUtils.java (li

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review89922 --- I have a few other comments which I can add later or discuss offline

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review89677 --- Thanks for the patch. A few comments below. Also, we need to handle

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review89063 --- Thanks for the patch, Sriharsha. A few comments below. clients/src

Re: Review Request 33620: Patch for KAFKA-1690

> On May 22, 2015, 12:33 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 294 > > > > > > Not sure why we are looping back here. If the HandshakeStatus is

Re: Review Request 33620: Patch for KAFKA-1690

> On May 22, 2015, 12:33 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 231 > > > > > > Do we need config.values? Could we just pass in config.originals()

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated June 23, 2015, 8:18 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

> On May 22, 2015, 12:33 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java, > > line 231 > > > > > > Do we need config.values? Could we just pass in config.originals()

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review87646 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated June 4, 2015, 1:52 a.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

> On May 22, 2015, 12:33 a.m., Jun Rao wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 240-242 > > > > > > Would it be better to just throw the exception in the NOT_H

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review86250 --- clients/src/main/java/org/apache/kafka/common/network/SSLTransportL

Re: Review Request 33620: Patch for KAFKA-1690

- Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review83993 --- On May 21, 2015, 5:37 p.m., Sriharsha Chintalapani wrot

Re: Review Request 33620: Patch for KAFKA-1690

> On May 22, 2015, 12:14 a.m., Michael Herstine wrote: > > clients/src/main/java/org/apache/kafka/common/security/auth/PrincipalBuilder.java, > > line 44 > > > > > > I'm trying to imagine implementing `buildPrincipal`

Re: Review Request 33620: Patch for KAFKA-1690

> On May 15, 2015, 10:54 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 153 > > > > > > I think Michael meant the following which I think is valid ri

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review84845 --- clients/src/main/java/org/apache/kafka/common/network/SSLFactory.ja

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review84232 --- Thanks for the patch. A few more comments below. clients/src/main/

Re: Review Request 33620: Patch for KAFKA-1690

> On May 22, 2015, 12:14 a.m., Michael Herstine wrote: > > clients/src/main/java/org/apache/kafka/common/security/auth/PrincipalBuilder.java, > > line 44 > > > > > > I'm trying to imagine implementing `buildPrincipal`

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review84235 --- clients/src/main/java/org/apache/kafka/common/network/SSLFactory.ja

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/ --- (Updated May 21, 2015, 5:37 p.m.) Review request for kafka. Bugs: KAFKA-1690

Re: Review Request 33620: Patch for KAFKA-1690

> 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 > > > > > > This is interesting; I don't see a corresponding > > `cre

Re: Review Request 33620: Patch for KAFKA-1690

--- 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

Re: Review Request 33620: Patch for KAFKA-1690

> On May 15, 2015, 8:26 p.m., Michael Herstine wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 418 > > > > > > So you're transferring bytes from `appReadBuffer` to `ds

Re: Review Request 33620: Patch for KAFKA-1690

> On May 15, 2015, 8:26 p.m., Michael Herstine wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > lines 112-121 > > > > > > Sorry, but I'm a little confused here. Why would th

Re: Review Request 33620: Patch for KAFKA-1690

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review84144 --- clients/src/main/java/org/apache/kafka/common/config/SecurityConfig

Re: Review Request 33620: Patch for KAFKA-1690

> On May 16, 2015, 12:07 a.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLChannelBuilder.java, > > line 35 > > > > > > Can we just use a fixed thread pool? > > > > A more ge

Re: Review Request 33620: Patch for KAFKA-1690

> On May 15, 2015, 10:54 p.m., Joel Koshy wrote: > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java, > > line 153 > > > > > > I think Michael meant the following which I think is valid ri

  1   2   >