Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Robert Yokota]

Hi Colin,

> With regard to delayMs, can’t we just restart the
> Connector when the keys are actually changed?

Currently the VaultConfigProvider does not find out when values for keys
have changed.  You could do this with a poll model (with a
background thread in the ConfigProvider), but since for each key-value
pair, Vault provides a lease duration stating exactly when a value for a
key will change in the future, this is an alternative model of just passing
the lease duration to the client (in this case the Connector), to allow it
to determine what to do (such as schedule a restart).   This may allow one
to avoid the complexity of figuring out a proper poll interval (with lease
durations of varying periods), or worrying about putting too much load on
the secrets manager by polling too often.  In other words, by adding this
one additional parameter, a ConfigProvider can provide both push and pull
models to clients, perhaps with an additional configuration parameter to
the ConfigProvider to determine which model (push or poll) to use.

Thanks,
Robert

On Wed, May 16, 2018 at 9:56 PM, Colin McCabe  wrote:

> Thanks, Robert.  With regard to delayMs, can’t we just restart the
> Connector when the keys are actually changed?  Or is the concern that
> this would lengthen the effective key rotation time?  Can’t the user
> just configure a slightly shorter key rotation time to counteract
> this concern?
> Regards,
> Colin
>
> On Wed, May 16, 2018, at 19:13, Robert Yokota wrote:
> > Hi Colin,
> >
> > Good questions.
> >
> >
> > > As a clarification about the indirections, what if I have the
> > > connect> configuration key foo set up as ${vault:bar}, and in Vault,
> > have the bar> key set to ${file:baz}?
> > > Does connect get foo as the contents of the baz file?  I would
> > > argue that> it should not (and in general, we shouldn't allow
> ConfigProviders to
> > indirect to other
> > > ConfigProviders) but I don't think it's spelled out right now.
> >
> > I've added a clarification to the KIP that further indirections are
> > not> performed even if the values returned from ConfigProviders have the
> > variable syntax.
> >
> >
> > > What's the behavior when a config key is not found in Vault
> > > (or other> ConfigProvider)?  Does the variable get replaced with the
> empty
> > string, or> with the literal ${vault:whatever} string?
> >
> > It would remain unresolved and still be of the form
> > ${provider:key}.  I've> added a clarification to the KIP.
> >
> >
> > > Do we really need "${provider:[path:]key}", or can it just be
> > ${provider:key}?
> >
> > The path is a separate parameter in the APIs, so I think it's
> > important to> explicitly delineate it in the variable syntax.  For
> example, I
> > currently> have a working VaultConfigProvider prototype and the syntax
> for a
> > Vault key> reference looks like
> >
> > db_password=${vault:secret/staging:mysql_password}
> >
> > I think it's important to standardize how to separate the path
> > from the key> rather than leave it to each ConfigProvider to determine a
> possibly
> > different way.  This will also make it easier to move secrets from one>
> ConfigProvider to another should one choose to do so.
> >
> >
> > > Do we really need delayMs?
> >
> > One of the goals of this KIP is to allow for secrets rotation without>
> having to modify existing connectors.  In the case of the
> > VaultConfigProvider, it knows the lease durations and will be able to>
> schedule a restart of the Connector using an API in the Herder.  The
> > delayMs will simply be passed to the Herder.restartConnector(long
> > delayMs,> String connName, Callback cb) method here:
> >
> > https://github.com/rayokota/kafka/blob/secrets-in-connect-
> configs/connect/runtime/src/main/java/org/apache/kafka/
> connect/runtime/Herder.java#L170>
> >
> > Best,
> > Robert
> >
> >
> >
> > On Wed, May 16, 2018 at 6:16 PM, Colin McCabe
> >  wrote:>
> > > Thanks, Robert.  Looks good overall.
> > >
> > > As a clarification about the indirections, what if I have the
> > > connect> > configuration key foo set up as ${vault:bar}, and in Vault,
> have
> > > the bar> > key set to ${file:baz}?  Does connect get foo as the
> contents of
> > > the baz> > file?  I would argue that it should not (and in general, we
> > > shouldn't allow> > ConfigProviders to indirect to other
> ConfigProviders) but I
> > > don't think> > it's spelled out right now.
> > >
> > > What's the behavior when a config key is not found in Vault
> > > (or other> > ConfigProvider)?  Does the variable get replaced with the
> empty
> > > string, or> > with the literal ${vault:whatever} string?
> > >
> > > Do we really need "${provider:[path:]key}", or can it just be
> > > ${provider:key}?  It seems like the path can be rolled up into the
> > > key.  So> > if you want to put your connect keys under
> my.connect.path, you
> > > ask for> > ${vault:my.connect.path.jdbc.config}, etc.
> > >
> > > >// A 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Colin McCabe]

Hi Viktor,

The shell command isn’t that easy to integrate into applications.
AdminClient will get integrated  into a lot more stuff, which
increases the potential for conflicts.  I would argue that we should
fix this soon.
If we do want to reduce the scope in this KIP, we could do the merge in
the ConfigCommand  tool for now, and leave AC unchanged.
Best,
Colin


On Wed, May 16, 2018, at 04:57, Viktor Somogyi wrote:
> Hi Colin,
>
> > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> > set at the same time as you, you might overwrite that person's
> > changes, or vice versa.  So I really don't think we should try to do
> > this.  Also, having both an incremental and a full API is useful,
> > and it's just a single boolean at the protocol and API level.>
> Overwriting somebody's change is currently possible with the
> ConfigCommand, as it will do this get-merge-set behavior on the client> side, 
> in the command. From this perspective I think it's not much
> different to do this with the admin client. Also I think admins don't> modify 
> the quotas/configs of a client/user/topic/broker often (and
> multiple admins would do it even more rarely), so I don't think it is> a big 
> issue. What I think would be useful here but may be out of scope> is to 
> version the changes similarly to leader epochs. So when an admin> updates the 
> configs, it will increment a version number and won't let> other admins to 
> push changes in with lower than that. Instead it would> return an error.
>
> I would be also interested what others think about this?
>
> Cheers,
> Viktor
>
>
> On Sat, May 12, 2018 at 2:29 AM, Colin McCabe
>  wrote:> > On Wed, May 9, 2018, at 05:41, Viktor Somogyi 
> wrote:
> >> Hi Colin,
> >>
> >> > We are going to need to create a new version of
> >> > AlterConfigsRequest to add the "incremental" boolean.  So while
> >> > we're doing that, maybe we can change the type to
> >> > NULLABLE_STRING.> >>
> >> I was just talking to a colleague yesterday and we came to the
> >> conclusion that we should keep the boolean flag only on the client> >> 
> >> side (as you may have suggested earlier?) and not make part of the> >> 
> >> protocol as it might lead to a very complicated API on the long
> >> term.> >> Also we would keep the server side API simpler. Instead of the
> >> protocol change we could just simply have the boolean flag in
> >> AlterConfigOptions and the AdminClient should do the get-merge-set> >> 
> >> logic which corresponds to the behavior of the current
> >> ConfigCommand.> >> That way we won't need to change the protocol for now 
> >> but
> >> still have> >> both functionality. What do you think?
> >
> >  Hi Viktor,
> >
> > Doing get-merge-set is buggy, though.  If someone else does get-merge-
> > set at the same time as you, you might overwrite that person's
> > changes, or vice versa.  So I really don't think we should try to do
> > this.  Also, having both an incremental and a full API is useful,
> > and it's just a single boolean at the protocol and API level.> >
> >>
> >> > Hmm.  Not sure I follow.  KIP-133 doesn't use the empty string or
> >> > "" to indicate defaults, does it?> >>
> >> No it doesn't. It was just my early idea to indicate "delete"
> >> on the> >> protocol level. (We are using  for denoting the default
> >> client id or user in zookeeper.) Rajini was referring that we
> >> shouldn't expose this to the protocol level but instead denote
> >> delete> >> with an empty string.
> >>
> >> > This comes from DescribeConfigsResponse.
> >> > Unless I'm missing something, I think your suggestion to not
> >> > expose "" is already implemented?> >>
> >> In some way, yes. Although this one is used in describe and not in> >> 
> >> alter. For alter I don't think we'd need my early "" idea.> >
> > OK.  Thanks for the explanation.  Using an empty string to indicate
> > delete, as Rajini suggested, seems pretty reasonable to me.  null
> > would work as well.> >
> >>
> >> >> And we use STRING rather than NULLABLE_STRING in describe
> >> >> configs etc. So we> >> >> should probably do the same for quotas."
> >> >
> >> > I think nearly all responses treat ERROR_MESSAGE as a nullable
> >> > string.  CommonFields#ERROR_MESSAGE, which is used by most of
> >> > them, is a nullable string.  It's DescribeConfigsResponse that is
> >> > the black sheep here.> >> >
> >> >  > public static final Field.NullableStr ERROR_MESSAGE = new
> >> >  > Field.NullableStr("error_message", "Response error
> >> >  > message");> >>
> >> Looking at DescribeConfigsResponse (and AlterConfigsResponse)
> >> they use> >> nullable_string in the code. KIP-133 states otherwise though. 
> >> So in> >> this case it's not a problem luckily.
> >
> > Thanks for finding this inconsistency.  I'll change the KIP to
> > reflect what was actually implemented (nullable string for error).> >
> > cheers,
> > Colin
> >
> >>
> >> > What about writing a small script that just handles 

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Colin McCabe]

Thanks, Robert.  With regard to delayMs, can’t we just restart the
Connector when the keys are actually changed?  Or is the concern that
this would lengthen the effective key rotation time?  Can’t the user
just configure a slightly shorter key rotation time to counteract
this concern?
Regards,
Colin

On Wed, May 16, 2018, at 19:13, Robert Yokota wrote:
> Hi Colin,
>
> Good questions.
>
>
> > As a clarification about the indirections, what if I have the
> > connect> configuration key foo set up as ${vault:bar}, and in Vault,
> have the bar> key set to ${file:baz}?
> > Does connect get foo as the contents of the baz file?  I would
> > argue that> it should not (and in general, we shouldn't allow 
> > ConfigProviders to
> indirect to other
> > ConfigProviders) but I don't think it's spelled out right now.
>
> I've added a clarification to the KIP that further indirections are
> not> performed even if the values returned from ConfigProviders have the
> variable syntax.
>
>
> > What's the behavior when a config key is not found in Vault
> > (or other> ConfigProvider)?  Does the variable get replaced with the empty
> string, or> with the literal ${vault:whatever} string?
>
> It would remain unresolved and still be of the form
> ${provider:key}.  I've> added a clarification to the KIP.
>
>
> > Do we really need "${provider:[path:]key}", or can it just be
> ${provider:key}?
>
> The path is a separate parameter in the APIs, so I think it's
> important to> explicitly delineate it in the variable syntax.  For example, I
> currently> have a working VaultConfigProvider prototype and the syntax for a
> Vault key> reference looks like
>
> db_password=${vault:secret/staging:mysql_password}
>
> I think it's important to standardize how to separate the path
> from the key> rather than leave it to each ConfigProvider to determine a 
> possibly
> different way.  This will also make it easier to move secrets from one> 
> ConfigProvider to another should one choose to do so.
>
>
> > Do we really need delayMs?
>
> One of the goals of this KIP is to allow for secrets rotation without> having 
> to modify existing connectors.  In the case of the
> VaultConfigProvider, it knows the lease durations and will be able to> 
> schedule a restart of the Connector using an API in the Herder.  The
> delayMs will simply be passed to the Herder.restartConnector(long
> delayMs,> String connName, Callback cb) method here:
>
> https://github.com/rayokota/kafka/blob/secrets-in-connect-configs/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java#L170>
>
> Best,
> Robert
>
>
>
> On Wed, May 16, 2018 at 6:16 PM, Colin McCabe
>  wrote:>
> > Thanks, Robert.  Looks good overall.
> >
> > As a clarification about the indirections, what if I have the
> > connect> > configuration key foo set up as ${vault:bar}, and in Vault, have
> > the bar> > key set to ${file:baz}?  Does connect get foo as the contents of
> > the baz> > file?  I would argue that it should not (and in general, we
> > shouldn't allow> > ConfigProviders to indirect to other ConfigProviders) 
> > but I
> > don't think> > it's spelled out right now.
> >
> > What's the behavior when a config key is not found in Vault
> > (or other> > ConfigProvider)?  Does the variable get replaced with the empty
> > string, or> > with the literal ${vault:whatever} string?
> >
> > Do we really need "${provider:[path:]key}", or can it just be
> > ${provider:key}?  It seems like the path can be rolled up into the
> > key.  So> > if you want to put your connect keys under my.connect.path, you
> > ask for> > ${vault:my.connect.path.jdbc.config}, etc.
> >
> > >// A delayMs of 0 indicates an immediate change; a positive
> > >delayMs> > indicates
> > >// that a future change is anticipated (such as a lease
> > >duration)> > >void onChange(String path, Map 
> > > values, int
> > >delayMs);> >
> > Do we really need delayMs?  It seems like if you get a callback with> > 
> > delayMs set, you don't know what the new values will be, only
> > that an> > update is coming, but not yet here.
> >
> > best,
> > Colin
> >
> >
> > On Wed, May 16, 2018, at 17:05, Robert Yokota wrote:
> > > Hello everyone,
> > >
> > > After a good round of discussions with excellent feedback and no
> > > major> > > objections, I would like to start a vote on KIP-297 to 
> > > externalize> > secrets
> > > from Kafka Connect configurations.  My thanks in advance!
> > >
> > > KIP: <
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 297%3A+Externalizing+Secrets+for+Connect+Configurations
> > > >
> > >
> > > JIRA: 
> > >
> > > Discussion thread: <
> > > https://www.mail-archive.com/dev@kafka.apache.org/msg87638.html>
> > >
> > > Best,
> > > Robert
> >

[VOTE] KIP-285: Connect Rest Extension Plugin

[Magesh Nandakumar]

Hello everyone,

After a good round of discussions with excellent feedback and no major
objections, I would like to start a vote on KIP-285: Connect Rest Extension
Plugin.

KIP: <
https://cwiki.apache.org/confluence/display/KAFKA/KIP-285%3A+Connect+Rest+Extension+Plugin
>


JIRA: <*https://issues.apache.org/jira/browse/KAFKA-6776
*>

Discussion thread: <
https://www.mail-archive.com/dev@kafka.apache.org/msg86910.html>

Thanks,
Magesh

Re: [VOTE] KIP-277 - Fine Grained ACL for CreateTopics API

[Matt Farmer]

+1 (non-binding)

On Tue, May 15, 2018 at 4:26 AM, Edoardo Comar  wrote:

> Hi,
> bumping the thread as the current vote count for this KIP is
> 2 binding +1
> 5 non-binding +1
>
> thanks, Edo
>
> On 8 May 2018 at 16:14, Edoardo Comar  wrote:
> > Hi,
> > bumping the thread as the current vote count for this KIP is
> > 2 binding +1
> > 5 non-binding +1
> >
> > so still missing a binding vote please
> >
> > thanks,
> > Edo
> >
> >
> > On 30 April 2018 at 12:49, Manikumar  wrote:
> >>
> >> +1 (non-binding)
> >>
> >> Thanks
> >>
> >> On Thu, Apr 26, 2018 at 9:59 PM, Colin McCabe 
> wrote:
> >>
> >> > +1 (non-binding)
> >> >
> >> > best,
> >> > Colin
> >> >
> >> >
> >> > On Wed, Apr 25, 2018, at 02:45, Edoardo Comar wrote:
> >> > > Hi,
> >> > >
> >> > > The discuss thread on KIP-277 (
> >> > > https://www.mail-archive.com/dev@kafka.apache.org/msg86540.html )
> >> > > seems to have been fruitful and concerns have been addressed, please
> >> > allow
> >> > > me start a vote on it:
> >> > >
> >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> > 277+-+Fine+Grained+ACL+for+CreateTopics+API
> >> > >
> >> > > I will update the small PR to the latest KIP semantics if the vote
> >> > passes
> >> > > (as I hope :-).
> >> > >
> >> > > cheers
> >> > > Edo
> >> > > --
> >> > >
> >> > > Edoardo Comar
> >> > >
> >> > > IBM Message Hub
> >> > >
> >> > > IBM UK Ltd, Hursley Park, SO21 2JN
> >> > > Unless stated otherwise above:
> >> > > IBM United Kingdom Limited - Registered in England and Wales with
> >> > > number
> >> > > 741598.
> >> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire
> PO6
> >> > 3AU
> >> >
> >
> >
> >
> >
> > --
> > "When the people fear their government, there is tyranny; when the
> > government fears the people, there is liberty." [Thomas Jefferson]
> >
>
>
>
> --
> "When the people fear their government, there is tyranny; when the
> government fears the people, there is liberty." [Thomas Jefferson]
>

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Matt Farmer]

Hey Arjun,

I like deadletterqueue all lower case, so I'm +1 on that.

Yes, in the case we were seeing there were external system failures.
We had issues connecting to S3. While the connector does include
some retry functionality, however setting these values sufficiently
high seemed to cause us to hit timeouts and cause the entire
task to fail anyway. (I think I was using something like 100 retries
during the brief test of this behavior?)

Yeah, totally understand that there could be unintended concequences
from this. I guess the use case I'm trying to optimize for is to give
folks some bubblegum to keep a high volume system limping
along until the software engineers get time to address it. So I'm
imagining the situation that I'm paged on a Saturday night because of
an intermittent network issue. With a config flag like this I could push
a config change to cause Connect to treat that as retriable and allow
me to wait until the following Monday to make changes to the code.
That may not be a sensible concern for Kafka writ large, but Connect
is a bit weird when compared with Streams or the Clients. It's almost
more of a piece of infrastructure than a library, and I generally like
infrastructure to have escape hatches like that. Just my 0.02 though. :)

Thanks,
Matt

On Tue, May 15, 2018 at 8:46 PM, Arjun Satish 
wrote:

> Matt,
>
> Thanks so much for your comments. Really appreciate it!
>
> 1. Good point about the acronym. I can use deadletterqueue instead of dlq
> (using all lowercase to be consistent with the other configs in Kafka).
> What do you think?
>
> 2. Could you please tell us what errors caused these tasks to fail? Were
> they because of external system failures? And if so, could they be
> implemented in the Connector itself? Or using retries with backoffs?
>
> 3. I like this idea. But did not include it here since it might be a
> stretch. One thing to note is that ConnectExceptions can be thrown from a
> variety of places in a connector. I think it should be OK for the Connector
> to throw RetriableException or something that extends it for the operation
> to be retried. By changing this behavior, a lot of existing connectors
> would have to be updated so that they don't rewrite messages into this
> sink. For example, a sink connector might write some data into the external
> system partially, and then fail with a ConnectException. Since the
> framework has no way of knowing what was written and what was not, a retry
> here might cause the same data to written again into the sink.
>
> Best,
>
>
> On Mon, May 14, 2018 at 12:46 PM, Matt Farmer  wrote:
>
> > Hi Arjun,
> >
> > I'm following this very closely as better error handling in Connect is a
> > high priority
> > for MailChimp's Data Systems team.
> >
> > A few thoughts (in no particular order):
> >
> > For the dead letter queue configuration, could we use deadLetterQueue
> > instead of
> > dlq? Acronyms are notoriously hard to keep straight in everyone's head
> and
> > unless
> > there's a compelling reason it would be nice to use the characters and be
> > explicit.
> >
> > Have you considered any behavior that would periodically attempt to
> restart
> > failed
> > tasks after a certain amount of time? To get around our issues internally
> > we've
> > deployed a tool that monitors for failed tasks and restarts the task by
> > hitting the
> > REST API after the failure. Such a config would allow us to get rid of
> this
> > tool.
> >
> > Have you considered a config setting to allow-list additional classes as
> > retryable? In the situation we ran into, we were getting
> ConnectExceptions
> > that
> > were intermittent due to an unrelated service. With such a setting we
> could
> > have
> > deployed a config that temporarily whitelisted that Exception as
> > retry-worthy
> > and continued attempting to make progress while the other team worked
> > on mitigating the problem.
> >
> > Thanks for the KIP!
> >
> > On Wed, May 9, 2018 at 2:59 AM, Arjun Satish 
> > wrote:
> >
> > > All,
> > >
> > > I'd like to start a discussion on adding ways to handle and report
> record
> > > processing errors in Connect. Please find a KIP here:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 298%3A+Error+Handling+in+Connect
> > >
> > > Any feedback will be highly appreciated.
> > >
> > > Thanks very much,
> > > Arjun
> > >
> >
>

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Robert Yokota]

Hi Colin,

Good questions.


> As a clarification about the indirections, what if I have the connect
configuration key foo set up as ${vault:bar}, and in Vault, have the bar
key set to ${file:baz}?
> Does connect get foo as the contents of the baz file?  I would argue that
it should not (and in general, we shouldn't allow ConfigProviders to
indirect to other
> ConfigProviders) but I don't think it's spelled out right now.

I've added a clarification to the KIP that further indirections are not
performed even if the values returned from ConfigProviders have the
variable syntax.


> What's the behavior when a config key is not found in Vault (or other
ConfigProvider)?  Does the variable get replaced with the empty string, or
with the literal ${vault:whatever} string?

It would remain unresolved and still be of the form ${provider:key}.  I've
added a clarification to the KIP.


> Do we really need "${provider:[path:]key}", or can it just be
${provider:key}?

The path is a separate parameter in the APIs, so I think it's important to
explicitly delineate it in the variable syntax.  For example, I currently
have a working VaultConfigProvider prototype and the syntax for a Vault key
reference looks like

db_password=${vault:secret/staging:mysql_password}

I think it's important to standardize how to separate the path from the key
rather than leave it to each ConfigProvider to determine a possibly
different way.  This will also make it easier to move secrets from one
ConfigProvider to another should one choose to do so.


> Do we really need delayMs?

One of the goals of this KIP is to allow for secrets rotation without
having to modify existing connectors.  In the case of the
VaultConfigProvider, it knows the lease durations and will be able to
schedule a restart of the Connector using an API in the Herder.  The
delayMs will simply be passed to the Herder.restartConnector(long delayMs,
String connName, Callback cb) method here:

https://github.com/rayokota/kafka/blob/secrets-in-connect-configs/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java#L170


Best,
Robert



On Wed, May 16, 2018 at 6:16 PM, Colin McCabe  wrote:

> Thanks, Robert.  Looks good overall.
>
> As a clarification about the indirections, what if I have the connect
> configuration key foo set up as ${vault:bar}, and in Vault, have the bar
> key set to ${file:baz}?  Does connect get foo as the contents of the baz
> file?  I would argue that it should not (and in general, we shouldn't allow
> ConfigProviders to indirect to other ConfigProviders) but I don't think
> it's spelled out right now.
>
> What's the behavior when a config key is not found in Vault (or other
> ConfigProvider)?  Does the variable get replaced with the empty string, or
> with the literal ${vault:whatever} string?
>
> Do we really need "${provider:[path:]key}", or can it just be
> ${provider:key}?  It seems like the path can be rolled up into the key.  So
> if you want to put your connect keys under my.connect.path, you ask for
> ${vault:my.connect.path.jdbc.config}, etc.
>
> >// A delayMs of 0 indicates an immediate change; a positive delayMs
> indicates
> >// that a future change is anticipated (such as a lease duration)
> >void onChange(String path, Map values, int delayMs);
>
> Do we really need delayMs?  It seems like if you get a callback with
> delayMs set, you don't know what the new values will be, only that an
> update is coming, but not yet here.
>
> best,
> Colin
>
>
> On Wed, May 16, 2018, at 17:05, Robert Yokota wrote:
> > Hello everyone,
> >
> > After a good round of discussions with excellent feedback and no major
> > objections, I would like to start a vote on KIP-297 to externalize
> secrets
> > from Kafka Connect configurations.  My thanks in advance!
> >
> > KIP: <
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 297%3A+Externalizing+Secrets+for+Connect+Configurations
> > >
> >
> > JIRA: 
> >
> > Discussion thread: <
> > https://www.mail-archive.com/dev@kafka.apache.org/msg87638.html>
> >
> > Best,
> > Robert
>

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Magesh Nandakumar]

Arjun,

Thanks for all the changes. Technically, the message format used for the
DLQ should be part of the public interface since users could consume it and
take actions.

Thanks,
Magesh

On Wed, May 16, 2018 at 6:56 PM, Arjun Satish 
wrote:

> Hi Konstantine,
>
> Thanks a lot for your feedback. I have made the necessary changes to the
> KIP.
>
> Best,
>
> On Wed, May 16, 2018 at 11:38 AM, Konstantine Karantasis <
> konstant...@confluent.io> wrote:
>
> > Arjun, it's exciting to see a KIP around better handling of bad-data and
> > errors in Kafka Connect.
> >
> > I have only a few comments below, which I hope you'll find helpful.
> >
> > 1. I think it'd be useful to describe a bit more in detail how someone
> can
> > extract the raw data of a Kafka record that failed to get converted (on
> the
> > sink side in this example). How's the JSON schema looks like for an entry
> > that is added to the dead-letter-queue and what someone should do to get
> > the raw bytes?
> >
> > 2. Similarly, it'd be nice to describe a bit more what is placed or
> > attempted to be placed in the dead-letter-queue in the case of source
> > records that fail to get imported to Kafka. Currently the only sentence I
> > read related to that is: "Similarly, for source connectors, the developer
> > can write the corrected records back to the original source".
> >
> > 3. I think the plural for 'retries' in config options:
> > 'errors.retries.limit' and 'errors.retries.delay.max.ms' doesn't read
> very
> > well. Should 'retry' be used same as 'tolerance' (or 'log') is used right
> > below? For example:
> > errors.retry.limit
> > and
> > errors.retry.delay.max.ms
> >
> > 4. Should the metric names be 'total-record-failures' and
> > 'total-records-skipped' to match their metric description and also be
> > similar to 'total-retries'?
> >
> > And a few minor comments:
> >
> > - The domain of 'errors.retries.limit' does not include 0 in the allowed
> > values (even though it's the default value).
> >
> > - For someone unfamiliar with the term SMT, the acronym is not explained
> in
> > the text. Also the term transformations is better IMO.
> >
> > - typo: 'the task is to killed'
> >
> > - If you intend to add a link to a PR additionally to the jira ticket,
> it'd
> > be handy to add it to the KIP header (along with state, thread, jira,
> etc).
> > Now it's a bit hidden in the text and it's not clear that the KIP
> includes
> > a link to a PR.
> >
> > Thanks for working on this missing but important functionality.
> >
> > - Konstantine
> >
> >
> > On Tue, May 15, 2018 at 10:41 PM, Arjun Satish 
> > wrote:
> >
> > > Magesh,
> > >
> > > Just to add to your point about retriable exceptions: the producer can
> > > throw retriable exceptions which we are handling it here:
> > >
> > > https://github.com/apache/kafka/blob/trunk/connect/
> > > runtime/src/main/java/org/apache/kafka/connect/runtime/
> > > WorkerSourceTask.java#L275
> > >
> > > BTW, exceptions like TimeoutExceptions (which extend
> RetriableExceptions)
> > > are bubbled back to the application, and need to be handled as per
> > > application requirements.
> > >
> > > Best,
> > >
> > > On Tue, May 15, 2018 at 8:30 PM, Arjun Satish 
> > > wrote:
> > >
> > > > Magesh,
> > > >
> > > > Thanks for the feedback! Really appreciate your comments.
> > > >
> > > > 1. I updated the KIP to state that only the configs of the failed
> > > > operation will be emitted. Thank you!
> > > >
> > > > The purpose of bundling the configs of the failed operation along
> with
> > > the
> > > > error context is to have a single place to find everything relevant
> to
> > > the
> > > > failure. This way, we can only look at the error logs to find the
> most
> > > > common pieces to "failure" puzzles: the operation, the config and the
> > > input
> > > > record. Ideally, a programmer should be able to take these pieces and
> > > > reproduce the error locally.
> > > >
> > > > 2. Added a table to describe this in the KIP.
> > > >
> > > > 3. Raw bytes will be base64 encoded before being logged. Updated the
> > KIP
> > > > to state this. Thank you!
> > > >
> > > > 4. I'll add an example log4j config to show we can take logs from a
> > class
> > > > and redirect it to a different location. Made a note in the PR for
> > this.
> > > >
> > > > 5. When we talk about logging messages, this could mean instances of
> > > > SinkRecords or SourceRecords. When we disable logging of messages,
> > these
> > > > records would be replaced by a "null". If you think it makes sense,
> > > instead
> > > > of completely dropping the object, we could drop only the key and
> value
> > > > objects from ConnectRecord? That way some context will still be
> > retained.
> > > >
> > > > 6. Yes, for now I think it is good to have explicit config in
> > Connectors
> > > > which dictates the error handling behavior. If this becomes an
> > > > inconvenience, we can think of having a 

Re: [DISCUSS] KIP-285: Connect Rest Extension Plugin

[Randall Hauch]

Looks good to me. Thanks for quickly making the changes! Great work!
 
Best regards,

Randall

> On May 16, 2018, at 8:07 PM, Magesh Nandakumar  wrote:
> 
> Randall,
> 
> I have adjusted the package names per Ewen's suggestions and also made some
> minor edits per your suggestions. Since there are no major outstanding
> issues, i'm moving this to vote.
> 
> Thanks
> Magesh
> 
>> On Wed, May 16, 2018 at 4:38 PM, Randall Hauch  wrote:
>> 
>> A few very minor suggestions:
>> 
>> 
>>   1. There are a few formatting issues with paragraphs that use a
>>   monospace font. Minor, but it would be nice to fix.
>>   2. Would be nice to link to the PR
>>   3. Do we need the org.apache.kafka.connect.rest.extension.entities
>>   package? Could we just move the two classes into the parent
>>   org.apache.kafka.connect.rest.extension package?
>>   4. This sentence "The above approach helps alleviate any issues that
>>   could arise if Extension accidentally reregister the" is cut off.
>>   5. The "ConnectRestExtensionContext.configure(...)" method's JavaDoc
>>   should describe the behaviors that are mentioned in the "Rest Extension
>>   Integration with Connect" section; e.g., behavior when an extension
>> adds a
>>   resource that is already registered, whether unregistering works, etc.
>>   Also, ideally the "close()" method would have JavaDoc that explained
>> when
>>   it is called (e.g., no other methods will be called on the extension
>> after
>>   this, etc.).
>>   6. Packaging requirements are different for this component vs
>>   connectors, transformations, and converters, since this now mandates the
>>   Service Loader manifest file. This should be called out more explicitly.
>>   7. It'd be nice if the example included how extension-specific config
>>   properties are to be defined in the worker configuration file.
>> 
>> As I said, these are all minor suggestions that only affect the KIP
>> document. Once these are fixed, I think this is ready to move to voting.
>> 
>> Best regards,
>> 
>> Randall
>> 
>> On Tue, May 15, 2018 at 11:30 AM, Magesh Nandakumar 
>> wrote:
>> 
>>> Randall- I think I have addressed all the comments. Let me know if we can
>>> take this to Vote.
>>> 
>>> Thanks
>>> Magesh
>>> 
>>> On Tue, May 8, 2018 at 10:12 PM, Magesh Nandakumar >> 
>>> wrote:
>>> 
 Hi All,
 
 I have updated the KIP to reflect changes based on the PR
 https://github.com/apache/kafka/pull/4931. Its mostly has minor
>> changes
 to the interfaces and includes details on packages for the interfaces
>> and
 the classes. Let me know your thoughts.
 
 Thanks
 Magesh
 
 On Fri, Apr 27, 2018 at 12:03 PM, Randall Hauch 
>>> wrote:
 
> Great work, Magesh. I like the overall approach a lot, so I left some
> pretty nuanced comments about specific details.
> 
> Best regards,
> 
> Randall
> 
> On Wed, Apr 25, 2018 at 3:03 PM, Magesh Nandakumar <
>>> mage...@confluent.io>
> wrote:
> 
>> Thanks Randall for your thoughts. I have created a replica of the
> required
>> entities in the draft implementation. If you can take a look at the
>> PR
> and
>> let me know your thoughts, I will update the KIP to reflect the same
>> 
>> https://github.com/apache/kafka/pull/4931
>> 
>> On Tue, Apr 24, 2018 at 11:44 AM, Randall Hauch 
> wrote:
>> 
>>> Magesh, I think our last emails cross in mid-stream.
>>> 
>>> We definitely want to put the new public interfaces/classes in the
>>> API
>>> module, and implementation in the runtime module. Yes, this will
> affect
>> the
>>> design, since for example we don't want to expose runtime types to
>>> the
>> API,
>>> and we want to prevent breaking changes. We don't really want to
>>> move
> the
>>> REST entities if we don't have to, since that may break projects
>>> that
> are
>>> extending the runtime module -- even though the runtime module is
>>> not
> a
>>> public API we still want to _try_ to change things.
>>> 
>>> Do you want to try to create a prototype to see what kind of
>> impact
> and
>>> choices we'll have to make?
>>> 
>>> Best regards,
>>> 
>>> Randall
>>> 
>>> On Tue, Apr 24, 2018 at 12:48 PM, Randall Hauch >> 
>> wrote:
>>> 
 Thanks for updating the KIP, Magesh. You've resolved all of my
>> concerns,
 though I have one more: we should specify the package names for
>>> all
> new
 interfaces/classes.
 
 I'm looking forward to more feedback from others.
 
 Best regards,
 
 Randall
 
 On Fri, Apr 20, 2018 at 12:17 AM, Magesh Nandakumar <
>>> mage...@confluent.io>
 wrote:
 
> Hi All,
> 

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Arjun Satish]

Hi Konstantine,

Thanks a lot for your feedback. I have made the necessary changes to the
KIP.

Best,

On Wed, May 16, 2018 at 11:38 AM, Konstantine Karantasis <
konstant...@confluent.io> wrote:

> Arjun, it's exciting to see a KIP around better handling of bad-data and
> errors in Kafka Connect.
>
> I have only a few comments below, which I hope you'll find helpful.
>
> 1. I think it'd be useful to describe a bit more in detail how someone can
> extract the raw data of a Kafka record that failed to get converted (on the
> sink side in this example). How's the JSON schema looks like for an entry
> that is added to the dead-letter-queue and what someone should do to get
> the raw bytes?
>
> 2. Similarly, it'd be nice to describe a bit more what is placed or
> attempted to be placed in the dead-letter-queue in the case of source
> records that fail to get imported to Kafka. Currently the only sentence I
> read related to that is: "Similarly, for source connectors, the developer
> can write the corrected records back to the original source".
>
> 3. I think the plural for 'retries' in config options:
> 'errors.retries.limit' and 'errors.retries.delay.max.ms' doesn't read very
> well. Should 'retry' be used same as 'tolerance' (or 'log') is used right
> below? For example:
> errors.retry.limit
> and
> errors.retry.delay.max.ms
>
> 4. Should the metric names be 'total-record-failures' and
> 'total-records-skipped' to match their metric description and also be
> similar to 'total-retries'?
>
> And a few minor comments:
>
> - The domain of 'errors.retries.limit' does not include 0 in the allowed
> values (even though it's the default value).
>
> - For someone unfamiliar with the term SMT, the acronym is not explained in
> the text. Also the term transformations is better IMO.
>
> - typo: 'the task is to killed'
>
> - If you intend to add a link to a PR additionally to the jira ticket, it'd
> be handy to add it to the KIP header (along with state, thread, jira, etc).
> Now it's a bit hidden in the text and it's not clear that the KIP includes
> a link to a PR.
>
> Thanks for working on this missing but important functionality.
>
> - Konstantine
>
>
> On Tue, May 15, 2018 at 10:41 PM, Arjun Satish 
> wrote:
>
> > Magesh,
> >
> > Just to add to your point about retriable exceptions: the producer can
> > throw retriable exceptions which we are handling it here:
> >
> > https://github.com/apache/kafka/blob/trunk/connect/
> > runtime/src/main/java/org/apache/kafka/connect/runtime/
> > WorkerSourceTask.java#L275
> >
> > BTW, exceptions like TimeoutExceptions (which extend RetriableExceptions)
> > are bubbled back to the application, and need to be handled as per
> > application requirements.
> >
> > Best,
> >
> > On Tue, May 15, 2018 at 8:30 PM, Arjun Satish 
> > wrote:
> >
> > > Magesh,
> > >
> > > Thanks for the feedback! Really appreciate your comments.
> > >
> > > 1. I updated the KIP to state that only the configs of the failed
> > > operation will be emitted. Thank you!
> > >
> > > The purpose of bundling the configs of the failed operation along with
> > the
> > > error context is to have a single place to find everything relevant to
> > the
> > > failure. This way, we can only look at the error logs to find the most
> > > common pieces to "failure" puzzles: the operation, the config and the
> > input
> > > record. Ideally, a programmer should be able to take these pieces and
> > > reproduce the error locally.
> > >
> > > 2. Added a table to describe this in the KIP.
> > >
> > > 3. Raw bytes will be base64 encoded before being logged. Updated the
> KIP
> > > to state this. Thank you!
> > >
> > > 4. I'll add an example log4j config to show we can take logs from a
> class
> > > and redirect it to a different location. Made a note in the PR for
> this.
> > >
> > > 5. When we talk about logging messages, this could mean instances of
> > > SinkRecords or SourceRecords. When we disable logging of messages,
> these
> > > records would be replaced by a "null". If you think it makes sense,
> > instead
> > > of completely dropping the object, we could drop only the key and value
> > > objects from ConnectRecord? That way some context will still be
> retained.
> > >
> > > 6. Yes, for now I think it is good to have explicit config in
> Connectors
> > > which dictates the error handling behavior. If this becomes an
> > > inconvenience, we can think of having a cluster global default, or
> better
> > > defaults in the configs.
> > >
> > > Best,
> > >
> > >
> > > On Tue, May 15, 2018 at 1:07 PM, Magesh Nandakumar <
> mage...@confluent.io
> > >
> > > wrote:
> > >
> > >> Hi Arjun,
> > >>
> > >> I think this a great KIP and would be a great addition to have in
> > connect.
> > >> Had a couple of minor questions:
> > >>
> > >> 1. What would be the value in logging the connector config using
> > >> errors.log.include.configs
> > >> for every message?
> > >> 2. Not being picky on format 

Re: [VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Colin McCabe]

Thanks, Robert.  Looks good overall.

As a clarification about the indirections, what if I have the connect 
configuration key foo set up as ${vault:bar}, and in Vault, have the bar key 
set to ${file:baz}?  Does connect get foo as the contents of the baz file?  I 
would argue that it should not (and in general, we shouldn't allow 
ConfigProviders to indirect to other ConfigProviders) but I don't think it's 
spelled out right now.

What's the behavior when a config key is not found in Vault (or other 
ConfigProvider)?  Does the variable get replaced with the empty string, or with 
the literal ${vault:whatever} string?

Do we really need "${provider:[path:]key}", or can it just be ${provider:key}?  
It seems like the path can be rolled up into the key.  So if you want to put 
your connect keys under my.connect.path, you ask for 
${vault:my.connect.path.jdbc.config}, etc.

>// A delayMs of 0 indicates an immediate change; a positive delayMs 
> indicates 
>// that a future change is anticipated (such as a lease duration)
>void onChange(String path, Map values, int delayMs);

Do we really need delayMs?  It seems like if you get a callback with delayMs 
set, you don't know what the new values will be, only that an update is coming, 
but not yet here.

best,
Colin


On Wed, May 16, 2018, at 17:05, Robert Yokota wrote:
> Hello everyone,
> 
> After a good round of discussions with excellent feedback and no major
> objections, I would like to start a vote on KIP-297 to externalize secrets
> from Kafka Connect configurations.  My thanks in advance!
> 
> KIP: <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-297%3A+Externalizing+Secrets+for+Connect+Configurations
> >
> 
> JIRA: 
> 
> Discussion thread: <
> https://www.mail-archive.com/dev@kafka.apache.org/msg87638.html>
> 
> Best,
> Robert

Re: [DISCUSS] KIP-285: Connect Rest Extension Plugin

[Magesh Nandakumar]

Randall,

I have adjusted the package names per Ewen's suggestions and also made some
minor edits per your suggestions. Since there are no major outstanding
issues, i'm moving this to vote.

Thanks
Magesh

On Wed, May 16, 2018 at 4:38 PM, Randall Hauch  wrote:

> A few very minor suggestions:
>
>
>1. There are a few formatting issues with paragraphs that use a
>monospace font. Minor, but it would be nice to fix.
>2. Would be nice to link to the PR
>3. Do we need the org.apache.kafka.connect.rest.extension.entities
>package? Could we just move the two classes into the parent
>org.apache.kafka.connect.rest.extension package?
>4. This sentence "The above approach helps alleviate any issues that
>could arise if Extension accidentally reregister the" is cut off.
>5. The "ConnectRestExtensionContext.configure(...)" method's JavaDoc
>should describe the behaviors that are mentioned in the "Rest Extension
>Integration with Connect" section; e.g., behavior when an extension
> adds a
>resource that is already registered, whether unregistering works, etc.
>Also, ideally the "close()" method would have JavaDoc that explained
> when
>it is called (e.g., no other methods will be called on the extension
> after
>this, etc.).
>6. Packaging requirements are different for this component vs
>connectors, transformations, and converters, since this now mandates the
>Service Loader manifest file. This should be called out more explicitly.
>7. It'd be nice if the example included how extension-specific config
>properties are to be defined in the worker configuration file.
>
> As I said, these are all minor suggestions that only affect the KIP
> document. Once these are fixed, I think this is ready to move to voting.
>
> Best regards,
>
> Randall
>
> On Tue, May 15, 2018 at 11:30 AM, Magesh Nandakumar 
> wrote:
>
> > Randall- I think I have addressed all the comments. Let me know if we can
> > take this to Vote.
> >
> > Thanks
> > Magesh
> >
> > On Tue, May 8, 2018 at 10:12 PM, Magesh Nandakumar  >
> > wrote:
> >
> > > Hi All,
> > >
> > > I have updated the KIP to reflect changes based on the PR
> > > https://github.com/apache/kafka/pull/4931. Its mostly has minor
> changes
> > > to the interfaces and includes details on packages for the interfaces
> and
> > > the classes. Let me know your thoughts.
> > >
> > > Thanks
> > > Magesh
> > >
> > > On Fri, Apr 27, 2018 at 12:03 PM, Randall Hauch 
> > wrote:
> > >
> > >> Great work, Magesh. I like the overall approach a lot, so I left some
> > >> pretty nuanced comments about specific details.
> > >>
> > >> Best regards,
> > >>
> > >> Randall
> > >>
> > >> On Wed, Apr 25, 2018 at 3:03 PM, Magesh Nandakumar <
> > mage...@confluent.io>
> > >> wrote:
> > >>
> > >> > Thanks Randall for your thoughts. I have created a replica of the
> > >> required
> > >> > entities in the draft implementation. If you can take a look at the
> PR
> > >> and
> > >> > let me know your thoughts, I will update the KIP to reflect the same
> > >> >
> > >> > https://github.com/apache/kafka/pull/4931
> > >> >
> > >> > On Tue, Apr 24, 2018 at 11:44 AM, Randall Hauch 
> > >> wrote:
> > >> >
> > >> > > Magesh, I think our last emails cross in mid-stream.
> > >> > >
> > >> > > We definitely want to put the new public interfaces/classes in the
> > API
> > >> > > module, and implementation in the runtime module. Yes, this will
> > >> affect
> > >> > the
> > >> > > design, since for example we don't want to expose runtime types to
> > the
> > >> > API,
> > >> > > and we want to prevent breaking changes. We don't really want to
> > move
> > >> the
> > >> > > REST entities if we don't have to, since that may break projects
> > that
> > >> are
> > >> > > extending the runtime module -- even though the runtime module is
> > not
> > >> a
> > >> > > public API we still want to _try_ to change things.
> > >> > >
> > >> > > Do you want to try to create a prototype to see what kind of
> impact
> > >> and
> > >> > > choices we'll have to make?
> > >> > >
> > >> > > Best regards,
> > >> > >
> > >> > > Randall
> > >> > >
> > >> > > On Tue, Apr 24, 2018 at 12:48 PM, Randall Hauch  >
> > >> > wrote:
> > >> > >
> > >> > > > Thanks for updating the KIP, Magesh. You've resolved all of my
> > >> > concerns,
> > >> > > > though I have one more: we should specify the package names for
> > all
> > >> new
> > >> > > > interfaces/classes.
> > >> > > >
> > >> > > > I'm looking forward to more feedback from others.
> > >> > > >
> > >> > > > Best regards,
> > >> > > >
> > >> > > > Randall
> > >> > > >
> > >> > > > On Fri, Apr 20, 2018 at 12:17 AM, Magesh Nandakumar <
> > >> > > mage...@confluent.io>
> > >> > > > wrote:
> > >> > > >
> > >> > > >> Hi All,
> > >> > > >>
> > >> > > >> I have updated the KIP with following changes
> > >> > > >>
> > >> > > >>  

Build failed in Jenkins: kafka-trunk-jdk8 #2649

[Apache Jenkins Server]

See 


Changes:

[github] MINOR: fix broken links in streams doc (#5025)

[github] MINOR: add missing parameter `processing.guaratees` to Streams docs

--
[...truncated 3.57 MB...]
kafka.zookeeper.ZooKeeperClientTest > testPipelinedGetData STARTED

kafka.zookeeper.ZooKeeperClientTest > testPipelinedGetData PASSED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChildChangeHandlerForChildChange 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChildChangeHandlerForChildChange 
PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenExistingZNodeWithChildren 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetChildrenExistingZNodeWithChildren 
PASSED

kafka.zookeeper.ZooKeeperClientTest > testSetDataExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testSetDataExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testMixedPipeline STARTED

kafka.zookeeper.ZooKeeperClientTest > testMixedPipeline PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetDataExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetDataExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testDeleteExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testDeleteExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testSessionExpiry STARTED

kafka.zookeeper.ZooKeeperClientTest > testSessionExpiry PASSED

kafka.zookeeper.ZooKeeperClientTest > testSetDataNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testSetDataNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testDeleteNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testDeleteNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testExistsExistingZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testExistsExistingZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testZooKeeperStateChangeRateMetrics 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testZooKeeperStateChangeRateMetrics PASSED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChangeHandlerForDeletion STARTED

kafka.zookeeper.ZooKeeperClientTest > testZNodeChangeHandlerForDeletion PASSED

kafka.zookeeper.ZooKeeperClientTest > testGetAclNonExistentZNode STARTED

kafka.zookeeper.ZooKeeperClientTest > testGetAclNonExistentZNode PASSED

kafka.zookeeper.ZooKeeperClientTest > testStateChangeHandlerForAuthFailure 
STARTED

kafka.zookeeper.ZooKeeperClientTest > testStateChangeHandlerForAuthFailure 
PASSED

kafka.network.SocketServerTest > testGracefulClose STARTED

kafka.network.SocketServerTest > testGracefulClose PASSED

kafka.network.SocketServerTest > controlThrowable STARTED

kafka.network.SocketServerTest > controlThrowable PASSED

kafka.network.SocketServerTest > testRequestMetricsAfterStop STARTED

kafka.network.SocketServerTest > testRequestMetricsAfterStop PASSED

kafka.network.SocketServerTest > testConnectionIdReuse STARTED

kafka.network.SocketServerTest > testConnectionIdReuse PASSED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
STARTED

kafka.network.SocketServerTest > testClientDisconnectionUpdatesRequestMetrics 
PASSED

kafka.network.SocketServerTest > testProcessorMetricsTags STARTED

kafka.network.SocketServerTest > testProcessorMetricsTags PASSED

kafka.network.SocketServerTest > testMaxConnectionsPerIp STARTED

kafka.network.SocketServerTest > testMaxConnectionsPerIp PASSED

kafka.network.SocketServerTest > testConnectionId STARTED

kafka.network.SocketServerTest > testConnectionId PASSED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics STARTED

kafka.network.SocketServerTest > 
testBrokerSendAfterChannelClosedUpdatesRequestMetrics PASSED

kafka.network.SocketServerTest > testNoOpAction STARTED

kafka.network.SocketServerTest > testNoOpAction PASSED

kafka.network.SocketServerTest > simpleRequest STARTED

kafka.network.SocketServerTest > simpleRequest PASSED

kafka.network.SocketServerTest > closingChannelException STARTED

kafka.network.SocketServerTest > closingChannelException PASSED

kafka.network.SocketServerTest > testIdleConnection STARTED

kafka.network.SocketServerTest > testIdleConnection PASSED

kafka.network.SocketServerTest > 
testClientDisconnectionWithStagedReceivesFullyProcessed STARTED

kafka.network.SocketServerTest > 
testClientDisconnectionWithStagedReceivesFullyProcessed PASSED

kafka.network.SocketServerTest > testZeroMaxConnectionsPerIp STARTED

kafka.network.SocketServerTest > testZeroMaxConnectionsPerIp PASSED

kafka.network.SocketServerTest > testMetricCollectionAfterShutdown STARTED

kafka.network.SocketServerTest > testMetricCollectionAfterShutdown PASSED

kafka.network.SocketServerTest > testSessionPrincipal STARTED

kafka.network.SocketServerTest > testSessionPrincipal PASSED

kafka.network.SocketServerTest > configureNewConnectionException STARTED

Re: [VOTE] KIP-255: OAuth Authentication via SASL/OAUTHBEARER

[Jun Rao]

Hi, Ron,

Thanks. I understand now. It may be useful to add a reference to JWT in the
KIP.

Jun

On Tue, May 15, 2018 at 6:51 PM, Ron Dagostino  wrote:

> Hi Jun.  I think you are getting at the fact that OAuth 2 is a flexible
> framework that allows different installations to do things differently.  It
> is true that the principal name in Kafka could come from any claim in the
> token.  Most of the time it would come from the 'sub' claim, but it could
> certainly come from another claim, or it could be only indirectly based on
> a claim value (maybe certain text would be trimmed or prefixed/suffixed).
> The point, which I think you are getting at, is that because the framework
> is flexible we need to accommodate that flexibility.  The callback handler
> class defined by the listener.name.sasl_ssl.oauthbearer.sasl.server.
> callback.handler.class configuration value gives us the required
> flexibility.  As an example, I have an implementation that leverages a
> popular open source JOSE library to parse the compact serialization,
> retrieve the public key if it has not yet been retrieved, verify the
> digital signature, and map the 'sub' claim to the OAuthBearerToken's
> principal name (which becomes the SASL authorization ID, which becomes the
> Kafka principal name).  I could just as easily have mapped a different
> claim to the OAuthBearerToken's principal name, manipulated the 'sub' claim
> value in some way, etc.  I write the callback handler code, so I complete
> flexibility to do whatever my OAuth 2 installation requires me to do.
>
> Ron
>
> On Tue, May 15, 2018 at 1:39 PM, Jun Rao  wrote:
>
> > Hi, Ron,
> >
> > Thanks for the reply. I understood your answers to #2 and #3.
> >
> > For #1, will the server map all clients' principal name to the value
> > associated with "sub" claim? How do we support mapping different clients
> to
> > different principal names?
> >
> > Jun
> >
> > On Mon, May 14, 2018 at 7:02 PM, Ron Dagostino 
> wrote:
> >
> > > Hi Jun.  Thanks for the +1 vote.
> > >
> > > Regarding the first question about token claims, yes, you have it
> correct
> > > about translating the OAuth token to a principle name via a JAAS module
> > > option in the default unsecured case.  Specifically, the OAuth SASL
> > Server
> > > implementation is responsible for setting the authorization ID, and it
> > gets
> > > the authorization ID from the OAuthBearerToken's principalName()
> method.
> > > The listener.name.sasl_ssl.oauthbearer.sasl.server.
> > callback.handler.class
> > > is responsible for handling an instance of OAuthBearerValidatorCallback
> > to
> > > accept a token compact serialization from the client and return an
> > instance
> > > of OAuthBearerToken (assuming the compact serialization validates), and
> > in
> > > the default unsecured case the builtin unsecured validator callback
> > handler
> > > defines the OAuthBearerToken.principalName() method to return the
> 'sub'
> > > claim value by default (with the actual claim it uses being
> configurable
> > > via the unsecuredValidatorPrincipalClaimName JAAS module option).  So
> > that
> > > is how we translate from a token to a principal name in the default
> > > unsecured (out-of-the-box) case.
> > >
> > > For production use cases, the implementation associated with
> > > listener.name.sasl_ssl.oauthbearer.sasl.server.callback.handler.class
> > can
> > > do whatever it wants.  As an example, I have written a class that
> wraps a
> > > com.nimbusds.jwt.SignedJWT instance (see
> > > https://connect2id.com/products/nimbus-jose-jwt/) and presents it as
> an
> > > OAuthBearerToken:
> > >
> > > public class NimbusSignedJwtOAuthBearerToken implements
> > OAuthBearerToken {
> > > private final SignedJWT signedJwt;
> > > private final String principalName;
> > > private final Set scope;
> > > private final Long startTimeMs;
> > > private final long lifetimeMs;
> > >
> > > /**
> > >  * Constructor
> > >  *
> > >  * @param signedJwt
> > >  *the mandatory signed JWT
> > >  * @param principalClaimName
> > >  *the mandatory claim name identifying the claim from
> > which
> > > the
> > >  *principal name will be extracted. The claim must
> exist
> > > and must be
> > >  *a String.
> > >  * @param optionalScopeClaimName
> > >  *the optional claim name identifying the claim from
> > which
> > > any scope
> > >  *will be extracted. If specified and the claim exists
> > then
> > > the
> > >  *value must be either a String or a String List.
> > >  * @throws ParseException
> > >  * if the principal claim does not exist or is not a
> > > String; the
> > >  * scope claim is neither a String nor a String List;
> the
> > > 'exp'
> > >  * claim does not exist or is not a number; the 'iat'
> > claim
> 

[VOTE] KIP-297: Externalizing Secrets for Connect Configurations

[Robert Yokota]

Hello everyone,

After a good round of discussions with excellent feedback and no major
objections, I would like to start a vote on KIP-297 to externalize secrets
from Kafka Connect configurations.  My thanks in advance!

KIP: <
https://cwiki.apache.org/confluence/display/KAFKA/KIP-297%3A+Externalizing+Secrets+for+Connect+Configurations
>

JIRA: 

Discussion thread: <
https://www.mail-archive.com/dev@kafka.apache.org/msg87638.html>

Best,
Robert

Re: [DISCUSS] KIP-285: Connect Rest Extension Plugin

[Magesh Nandakumar]

Ewen,

Thanks for your comments. I have made the changes to the package names and
also moved the nested class up in the package.

Public API would include

*org.apache.kafka.connect.rest*


-ConnectClusterState
-ConnectRestExtension
-ConnectRestExtensionContext

*org.apache.kafka.connect.health*

AbstractState
ConnectClusterState
ConnectorHealth
ConnectorState
ConnectorType
TaskState

Runtime would include the following implementations

*org.apache.kafka.connect.runtime.rest*

ConnectRestExtensionContextImpl
ConnectRestConfigurable

*org.apache.kafka.connect.runtime.health*

ConnectClusterStateImpl

Let me know your thoughts.

Thanks
Magesh

On Wed, May 16, 2018 at 3:50 PM, Ewen Cheslack-Postava 
wrote:

> Hey,
>
> Sorry for the late follow up. I just had a couple of minor questions about
> details:
>
> * Some of the public API being added is under a runtime package. But that
> would be new for public API -- currently only things under the runtime
> package use that package name. I think changing the package name to just be
> under o.a.k.connect.rest or something like that would better keep this
> distinction clear and would also help shorten it a bit -- the packages are
> getting quite deeply nested with some of the new naming.
> * The cluster state classes probably shouldn't be under a rest package.
> That's where we're exposing them for public APIs currently, but it's not
> really specific to REST stuff in any way. I think we should house those
> somewhere more generic so they won't be awkward to reuse if we decided to
> (e.g. you could imagine extensions that provide this directly for metrics.
> * Currently we have the State classes nested inside ConnectorHealth class.
> I think this makes those classes more annoying to use. Is there a reason
> for them to be nested or can we just pull them out to the same level as
> ConnectorHealth?
>
> -Ewen
>
> On Tue, May 15, 2018 at 9:30 AM Magesh Nandakumar 
> wrote:
>
> > Randall- I think I have addressed all the comments. Let me know if we can
> > take this to Vote.
> >
> > Thanks
> > Magesh
> >
> > On Tue, May 8, 2018 at 10:12 PM, Magesh Nandakumar  >
> > wrote:
> >
> > > Hi All,
> > >
> > > I have updated the KIP to reflect changes based on the PR
> > > https://github.com/apache/kafka/pull/4931. Its mostly has minor
> changes
> > > to the interfaces and includes details on packages for the interfaces
> and
> > > the classes. Let me know your thoughts.
> > >
> > > Thanks
> > > Magesh
> > >
> > > On Fri, Apr 27, 2018 at 12:03 PM, Randall Hauch 
> > wrote:
> > >
> > >> Great work, Magesh. I like the overall approach a lot, so I left some
> > >> pretty nuanced comments about specific details.
> > >>
> > >> Best regards,
> > >>
> > >> Randall
> > >>
> > >> On Wed, Apr 25, 2018 at 3:03 PM, Magesh Nandakumar <
> > mage...@confluent.io>
> > >> wrote:
> > >>
> > >> > Thanks Randall for your thoughts. I have created a replica of the
> > >> required
> > >> > entities in the draft implementation. If you can take a look at the
> PR
> > >> and
> > >> > let me know your thoughts, I will update the KIP to reflect the same
> > >> >
> > >> > https://github.com/apache/kafka/pull/4931
> > >> >
> > >> > On Tue, Apr 24, 2018 at 11:44 AM, Randall Hauch 
> > >> wrote:
> > >> >
> > >> > > Magesh, I think our last emails cross in mid-stream.
> > >> > >
> > >> > > We definitely want to put the new public interfaces/classes in the
> > API
> > >> > > module, and implementation in the runtime module. Yes, this will
> > >> affect
> > >> > the
> > >> > > design, since for example we don't want to expose runtime types to
> > the
> > >> > API,
> > >> > > and we want to prevent breaking changes. We don't really want to
> > move
> > >> the
> > >> > > REST entities if we don't have to, since that may break projects
> > that
> > >> are
> > >> > > extending the runtime module -- even though the runtime module is
> > not
> > >> a
> > >> > > public API we still want to _try_ to change things.
> > >> > >
> > >> > > Do you want to try to create a prototype to see what kind of
> impact
> > >> and
> > >> > > choices we'll have to make?
> > >> > >
> > >> > > Best regards,
> > >> > >
> > >> > > Randall
> > >> > >
> > >> > > On Tue, Apr 24, 2018 at 12:48 PM, Randall Hauch  >
> > >> > wrote:
> > >> > >
> > >> > > > Thanks for updating the KIP, Magesh. You've resolved all of my
> > >> > concerns,
> > >> > > > though I have one more: we should specify the package names for
> > all
> > >> new
> > >> > > > interfaces/classes.
> > >> > > >
> > >> > > > I'm looking forward to more feedback from others.
> > >> > > >
> > >> > > > Best regards,
> > >> > > >
> > >> > > > Randall
> > >> > > >
> > >> > > > On Fri, Apr 20, 2018 at 12:17 AM, Magesh Nandakumar <
> > >> > > mage...@confluent.io>
> > >> > > > wrote:
> > >> > > >
> > >> > > >> Hi All,
> > >> > > >>
> > >> > > >> I have updated the KIP with 

Re: [DISCUSS] KIP-285: Connect Rest Extension Plugin

[Randall Hauch]

A few very minor suggestions:


   1. There are a few formatting issues with paragraphs that use a
   monospace font. Minor, but it would be nice to fix.
   2. Would be nice to link to the PR
   3. Do we need the org.apache.kafka.connect.rest.extension.entities
   package? Could we just move the two classes into the parent
   org.apache.kafka.connect.rest.extension package?
   4. This sentence "The above approach helps alleviate any issues that
   could arise if Extension accidentally reregister the" is cut off.
   5. The "ConnectRestExtensionContext.configure(...)" method's JavaDoc
   should describe the behaviors that are mentioned in the "Rest Extension
   Integration with Connect" section; e.g., behavior when an extension adds a
   resource that is already registered, whether unregistering works, etc.
   Also, ideally the "close()" method would have JavaDoc that explained when
   it is called (e.g., no other methods will be called on the extension after
   this, etc.).
   6. Packaging requirements are different for this component vs
   connectors, transformations, and converters, since this now mandates the
   Service Loader manifest file. This should be called out more explicitly.
   7. It'd be nice if the example included how extension-specific config
   properties are to be defined in the worker configuration file.

As I said, these are all minor suggestions that only affect the KIP
document. Once these are fixed, I think this is ready to move to voting.

Best regards,

Randall

On Tue, May 15, 2018 at 11:30 AM, Magesh Nandakumar 
wrote:

> Randall- I think I have addressed all the comments. Let me know if we can
> take this to Vote.
>
> Thanks
> Magesh
>
> On Tue, May 8, 2018 at 10:12 PM, Magesh Nandakumar 
> wrote:
>
> > Hi All,
> >
> > I have updated the KIP to reflect changes based on the PR
> > https://github.com/apache/kafka/pull/4931. Its mostly has minor changes
> > to the interfaces and includes details on packages for the interfaces and
> > the classes. Let me know your thoughts.
> >
> > Thanks
> > Magesh
> >
> > On Fri, Apr 27, 2018 at 12:03 PM, Randall Hauch 
> wrote:
> >
> >> Great work, Magesh. I like the overall approach a lot, so I left some
> >> pretty nuanced comments about specific details.
> >>
> >> Best regards,
> >>
> >> Randall
> >>
> >> On Wed, Apr 25, 2018 at 3:03 PM, Magesh Nandakumar <
> mage...@confluent.io>
> >> wrote:
> >>
> >> > Thanks Randall for your thoughts. I have created a replica of the
> >> required
> >> > entities in the draft implementation. If you can take a look at the PR
> >> and
> >> > let me know your thoughts, I will update the KIP to reflect the same
> >> >
> >> > https://github.com/apache/kafka/pull/4931
> >> >
> >> > On Tue, Apr 24, 2018 at 11:44 AM, Randall Hauch 
> >> wrote:
> >> >
> >> > > Magesh, I think our last emails cross in mid-stream.
> >> > >
> >> > > We definitely want to put the new public interfaces/classes in the
> API
> >> > > module, and implementation in the runtime module. Yes, this will
> >> affect
> >> > the
> >> > > design, since for example we don't want to expose runtime types to
> the
> >> > API,
> >> > > and we want to prevent breaking changes. We don't really want to
> move
> >> the
> >> > > REST entities if we don't have to, since that may break projects
> that
> >> are
> >> > > extending the runtime module -- even though the runtime module is
> not
> >> a
> >> > > public API we still want to _try_ to change things.
> >> > >
> >> > > Do you want to try to create a prototype to see what kind of impact
> >> and
> >> > > choices we'll have to make?
> >> > >
> >> > > Best regards,
> >> > >
> >> > > Randall
> >> > >
> >> > > On Tue, Apr 24, 2018 at 12:48 PM, Randall Hauch 
> >> > wrote:
> >> > >
> >> > > > Thanks for updating the KIP, Magesh. You've resolved all of my
> >> > concerns,
> >> > > > though I have one more: we should specify the package names for
> all
> >> new
> >> > > > interfaces/classes.
> >> > > >
> >> > > > I'm looking forward to more feedback from others.
> >> > > >
> >> > > > Best regards,
> >> > > >
> >> > > > Randall
> >> > > >
> >> > > > On Fri, Apr 20, 2018 at 12:17 AM, Magesh Nandakumar <
> >> > > mage...@confluent.io>
> >> > > > wrote:
> >> > > >
> >> > > >> Hi All,
> >> > > >>
> >> > > >> I have updated the KIP with following changes
> >> > > >>
> >> > > >>1. Expanded the Motivation section
> >> > > >>2. Included details about the interface in the public
> interface
> >> > > section
> >> > > >>3. Modified the config name to rest.extension.classes
> >> > > >>4. Modified the ConnectRestExtension to include Configurable
> >> > instead
> >> > > of
> >> > > >>ResourceConfig
> >> > > >>5. Modified the "Rest Extension Integration with Connect" in
> >> > > "Proposed
> >> > > >>Approach" to include a new Custom implementation for
> >> Configurable
> >> > > >>6. Provided examples 

Build failed in Jenkins: kafka-trunk-jdk10 #109

[Apache Jenkins Server]

See 


Changes:

[wangguoz] KAFKA-6905: Document that Processors may be re-used by Streams 
(#5022)

[github] MINOR: fix broken links in streams doc (#5025)

[github] MINOR: add missing parameter `processing.guaratees` to Streams docs

--
[...truncated 1.50 MB...]

kafka.zk.KafkaZkClientTest > testTopicAssignmentMethods PASSED

kafka.zk.KafkaZkClientTest > testPropagateIsrChanges STARTED

kafka.zk.KafkaZkClientTest > testPropagateIsrChanges PASSED

kafka.zk.KafkaZkClientTest > testControllerEpochMethods STARTED

kafka.zk.KafkaZkClientTest > testControllerEpochMethods PASSED

kafka.zk.KafkaZkClientTest > testDeleteRecursive STARTED

kafka.zk.KafkaZkClientTest > testDeleteRecursive PASSED

kafka.zk.KafkaZkClientTest > testGetTopicPartitionStates STARTED

kafka.zk.KafkaZkClientTest > testGetTopicPartitionStates PASSED

kafka.zk.KafkaZkClientTest > testCreateConfigChangeNotification STARTED

kafka.zk.KafkaZkClientTest > testCreateConfigChangeNotification PASSED

kafka.zk.KafkaZkClientTest > testDelegationTokenMethods STARTED

kafka.zk.KafkaZkClientTest > testDelegationTokenMethods PASSED

kafka.javaapi.message.ByteBufferMessageSetTest > testSizeInBytesWithCompression 
STARTED

kafka.javaapi.message.ByteBufferMessageSetTest > testSizeInBytesWithCompression 
PASSED

kafka.javaapi.message.ByteBufferMessageSetTest > 
testIteratorIsConsistentWithCompression STARTED

kafka.javaapi.message.ByteBufferMessageSetTest > 
testIteratorIsConsistentWithCompression PASSED

kafka.javaapi.message.ByteBufferMessageSetTest > testIteratorIsConsistent 
STARTED

kafka.javaapi.message.ByteBufferMessageSetTest > testIteratorIsConsistent PASSED

kafka.javaapi.message.ByteBufferMessageSetTest > testEqualsWithCompression 
STARTED

kafka.javaapi.message.ByteBufferMessageSetTest > testEqualsWithCompression 
PASSED

kafka.javaapi.message.ByteBufferMessageSetTest > testWrittenEqualsRead STARTED

kafka.javaapi.message.ByteBufferMessageSetTest > testWrittenEqualsRead PASSED

kafka.javaapi.message.ByteBufferMessageSetTest > testEquals STARTED

kafka.javaapi.message.ByteBufferMessageSetTest > testEquals PASSED

kafka.javaapi.message.ByteBufferMessageSetTest > testSizeInBytes STARTED

kafka.javaapi.message.ByteBufferMessageSetTest > testSizeInBytes PASSED

kafka.javaapi.consumer.ZookeeperConsumerConnectorTest > testBasic STARTED

kafka.javaapi.consumer.ZookeeperConsumerConnectorTest > testBasic PASSED

kafka.metrics.MetricsTest > testMetricsReporterAfterDeletingTopic STARTED

kafka.metrics.MetricsTest > testMetricsReporterAfterDeletingTopic PASSED

kafka.metrics.MetricsTest > testSessionExpireListenerMetrics STARTED

kafka.metrics.MetricsTest > testSessionExpireListenerMetrics PASSED

kafka.metrics.MetricsTest > 
testBrokerTopicMetricsUnregisteredAfterDeletingTopic STARTED

kafka.metrics.MetricsTest > 
testBrokerTopicMetricsUnregisteredAfterDeletingTopic PASSED

kafka.metrics.MetricsTest > testClusterIdMetric STARTED

kafka.metrics.MetricsTest > testClusterIdMetric PASSED

kafka.metrics.MetricsTest > testControllerMetrics STARTED

kafka.metrics.MetricsTest > testControllerMetrics PASSED

kafka.metrics.MetricsTest > testWindowsStyleTagNames STARTED

kafka.metrics.MetricsTest > testWindowsStyleTagNames PASSED

kafka.metrics.MetricsTest > testMetricsLeak STARTED

kafka.metrics.MetricsTest > testMetricsLeak PASSED

kafka.metrics.MetricsTest > testBrokerTopicMetricsBytesInOut STARTED

kafka.metrics.MetricsTest > testBrokerTopicMetricsBytesInOut PASSED

kafka.metrics.KafkaTimerTest > testKafkaTimer STARTED

kafka.metrics.KafkaTimerTest > testKafkaTimer PASSED

kafka.security.auth.ResourceTypeTest > testJavaConversions STARTED

kafka.security.auth.ResourceTypeTest > testJavaConversions PASSED

kafka.security.auth.ResourceTypeTest > testFromString STARTED

kafka.security.auth.ResourceTypeTest > testFromString PASSED

kafka.security.auth.OperationTest > testJavaConversions STARTED

kafka.security.auth.OperationTest > testJavaConversions PASSED

kafka.security.auth.ZkAuthorizationTest > testIsZkSecurityEnabled STARTED

kafka.security.auth.ZkAuthorizationTest > testIsZkSecurityEnabled PASSED

kafka.security.auth.ZkAuthorizationTest > testZkUtils STARTED

kafka.security.auth.ZkAuthorizationTest > testZkUtils PASSED

kafka.security.auth.ZkAuthorizationTest > testZkAntiMigration STARTED

kafka.security.auth.ZkAuthorizationTest > testZkAntiMigration PASSED

kafka.security.auth.ZkAuthorizationTest > testZkMigration STARTED

kafka.security.auth.ZkAuthorizationTest > testZkMigration PASSED

kafka.security.auth.ZkAuthorizationTest > testChroot STARTED

kafka.security.auth.ZkAuthorizationTest > testChroot PASSED

kafka.security.auth.ZkAuthorizationTest > testDelete STARTED

kafka.security.auth.ZkAuthorizationTest > testDelete PASSED

kafka.security.auth.ZkAuthorizationTest > testDeleteRecursive STARTED

kafka.security.auth.ZkAuthorizationTest > 

Build failed in Jenkins: kafka-trunk-jdk7 #3432

[Apache Jenkins Server]

See 


Changes:

[github] MINOR: add missing parameter `processing.guaratees` to Streams docs

--
[...truncated 420.14 KB...]

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithBrokers STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithBrokers PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithTopicsOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithTopicsOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumVerifyOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumVerifyOptions PASSED

Re: [DISCUSS] KIP-285: Connect Rest Extension Plugin

[Ewen Cheslack-Postava]

Hey,

Sorry for the late follow up. I just had a couple of minor questions about
details:

* Some of the public API being added is under a runtime package. But that
would be new for public API -- currently only things under the runtime
package use that package name. I think changing the package name to just be
under o.a.k.connect.rest or something like that would better keep this
distinction clear and would also help shorten it a bit -- the packages are
getting quite deeply nested with some of the new naming.
* The cluster state classes probably shouldn't be under a rest package.
That's where we're exposing them for public APIs currently, but it's not
really specific to REST stuff in any way. I think we should house those
somewhere more generic so they won't be awkward to reuse if we decided to
(e.g. you could imagine extensions that provide this directly for metrics.
* Currently we have the State classes nested inside ConnectorHealth class.
I think this makes those classes more annoying to use. Is there a reason
for them to be nested or can we just pull them out to the same level as
ConnectorHealth?

-Ewen

On Tue, May 15, 2018 at 9:30 AM Magesh Nandakumar 
wrote:

> Randall- I think I have addressed all the comments. Let me know if we can
> take this to Vote.
>
> Thanks
> Magesh
>
> On Tue, May 8, 2018 at 10:12 PM, Magesh Nandakumar 
> wrote:
>
> > Hi All,
> >
> > I have updated the KIP to reflect changes based on the PR
> > https://github.com/apache/kafka/pull/4931. Its mostly has minor changes
> > to the interfaces and includes details on packages for the interfaces and
> > the classes. Let me know your thoughts.
> >
> > Thanks
> > Magesh
> >
> > On Fri, Apr 27, 2018 at 12:03 PM, Randall Hauch 
> wrote:
> >
> >> Great work, Magesh. I like the overall approach a lot, so I left some
> >> pretty nuanced comments about specific details.
> >>
> >> Best regards,
> >>
> >> Randall
> >>
> >> On Wed, Apr 25, 2018 at 3:03 PM, Magesh Nandakumar <
> mage...@confluent.io>
> >> wrote:
> >>
> >> > Thanks Randall for your thoughts. I have created a replica of the
> >> required
> >> > entities in the draft implementation. If you can take a look at the PR
> >> and
> >> > let me know your thoughts, I will update the KIP to reflect the same
> >> >
> >> > https://github.com/apache/kafka/pull/4931
> >> >
> >> > On Tue, Apr 24, 2018 at 11:44 AM, Randall Hauch 
> >> wrote:
> >> >
> >> > > Magesh, I think our last emails cross in mid-stream.
> >> > >
> >> > > We definitely want to put the new public interfaces/classes in the
> API
> >> > > module, and implementation in the runtime module. Yes, this will
> >> affect
> >> > the
> >> > > design, since for example we don't want to expose runtime types to
> the
> >> > API,
> >> > > and we want to prevent breaking changes. We don't really want to
> move
> >> the
> >> > > REST entities if we don't have to, since that may break projects
> that
> >> are
> >> > > extending the runtime module -- even though the runtime module is
> not
> >> a
> >> > > public API we still want to _try_ to change things.
> >> > >
> >> > > Do you want to try to create a prototype to see what kind of impact
> >> and
> >> > > choices we'll have to make?
> >> > >
> >> > > Best regards,
> >> > >
> >> > > Randall
> >> > >
> >> > > On Tue, Apr 24, 2018 at 12:48 PM, Randall Hauch 
> >> > wrote:
> >> > >
> >> > > > Thanks for updating the KIP, Magesh. You've resolved all of my
> >> > concerns,
> >> > > > though I have one more: we should specify the package names for
> all
> >> new
> >> > > > interfaces/classes.
> >> > > >
> >> > > > I'm looking forward to more feedback from others.
> >> > > >
> >> > > > Best regards,
> >> > > >
> >> > > > Randall
> >> > > >
> >> > > > On Fri, Apr 20, 2018 at 12:17 AM, Magesh Nandakumar <
> >> > > mage...@confluent.io>
> >> > > > wrote:
> >> > > >
> >> > > >> Hi All,
> >> > > >>
> >> > > >> I have updated the KIP with following changes
> >> > > >>
> >> > > >>1. Expanded the Motivation section
> >> > > >>2. Included details about the interface in the public
> interface
> >> > > section
> >> > > >>3. Modified the config name to rest.extension.classes
> >> > > >>4. Modified the ConnectRestExtension to include Configurable
> >> > instead
> >> > > of
> >> > > >>ResourceConfig
> >> > > >>5. Modified the "Rest Extension Integration with Connect" in
> >> > > "Proposed
> >> > > >>Approach" to include a new Custom implementation for
> >> Configurable
> >> > > >>6. Provided examples for the Java Service provider mechanism
> >> > > >>7. Included a reference implementation in scope
> >> > > >>
> >> > > >> Kindly let me know your thoughts on the updates.
> >> > > >>
> >> > > >> Thanks
> >> > > >> Magesh
> >> > > >>
> >> > > >> On Thu, Apr 19, 2018 at 10:39 AM, Magesh Nandakumar <
> >> > > mage...@confluent.io
> >> > > >> >
> >> > > >> wrote:
> >> > > >>
> >> > > 

Re: [DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Jakub Scholz]

Hi,

What do you plan to use the read-only REST interface for? Is there
something what you cannot get through metrics interface? Otherwise it might
be easier to just disable the REST interface (either in the code, or just
on the platform level - e.g. in Kubernetes).

Also, I do not know what is the usual approach in Kafka ... but do we
really have to rename the offset.storage.* options? The current names do
not seem to have any collision with what you are adding and they would get
"out of sync" with the other options used in connect (status.storage.* and
config.storage.*). So it seems a bit unnecessary change to me.

Jakub



On Wed, May 16, 2018 at 10:10 PM Saulius Valatka 
wrote:

> Hi,
>
> I'd like to start a discussion on the following KIP:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-304%3A+Connect+runtime+mode+improvements+for+container+platforms
>
> Basically the idea is to make it easier to run separate instances of Kafka
> Connect hosting isolated connectors on container platforms such as Mesos or
> Kubernetes.
>
> In particular it would be interesting to hear opinions about the proposed
> read-only REST API mode, more specifically I'm concerned about the
> possibility to implement it in distributed mode as it appears the framework
> is using it internally (
>
> https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java#L1019
> ),
> however this particular API method appears to be undocumented(?).
>
> Looking forward for your feedback.
>

Re: [DISCUSS] KIP-303: Add Dynamic Routing in Streams Sink

[Damian Guy]

Overall i'm a +1 on this, but i'm not a big fan of using the KeyValueMapper
to choose the topic. It is a bit counter-intuitve to me. I'd prefer to add
a class specifically for it and possibly pass in the RecordContext

On Wed, 16 May 2018 at 13:22 Guozhang Wang  wrote:

> Hello folks,
>
> Please let me know if you have further feedbacks; if there is no more
> feedbacks I'm going to start the voting thread soon.
>
>
> Guozhang
>
>
> On Wed, May 16, 2018 at 8:31 AM, Guozhang Wang  wrote:
>
> > I have thought about exposing record context as well, and in the end I
> > decided to piggy-back it with KIP-159. And if we want to indeed reuse the
> > class it will be:
> >
> > ```
> > public interface RichKeyValueMapper {
> > VR apply(final K key, final V value, final RecordContext
> > recordContext);
> > }
> > ```
> >
> >
> >
> > Guozhang
> >
> >
> > On Tue, May 15, 2018 at 10:04 PM, Matthias J. Sax  >
> > wrote:
> >
> >> Just my 2 cents:
> >>
> >> I am fine with `KeyValueMapper` (+1 for code reusage) -- the JavaDocs
> >> will explain what the `KeyValueMapper` is supposed to do, ie, extract
> >> and return the sink topic name from the key-value pair.
> >>
> >> A side remark though: do we think that accessing key/value is
> >> sufficient? Or should we provide access to the full metadata? We could
> >> also do this with KIP-159 of course -- but this would come earliest in
> >> 2.1. As an alternative we could add a `TopicNameExtractor` to expose the
> >> whole record context. The advantage would be, that we don't need to
> >> change it via KIP-159 later again. WDYT?
> >>
> >> -Matthias
> >>
> >> On 5/15/18 5:57 PM, Bill Bejeck wrote:
> >> > Thanks for the KIP Guozhang, it's a +1 for me.
> >> >
> >> > As for re-using the KeyValueMapper for choosing the topic, I am on the
> >> > fence, a more explicitly named class would be more clear, but I'm not
> >> sure
> >> > it's worth a new class that will primarily perform the same actions as
> >> the
> >> > KeyValueMapper.
> >> >
> >> > Thanks,
> >> > Bill
> >> >
> >> > On Tue, May 15, 2018 at 5:52 PM, Guozhang Wang 
> >> wrote:
> >> >
> >> >> Hello John:
> >> >>
> >> >> * As for the type superclass, it is mainly for allowing super class
> >> serdes.
> >> >> More details can be found here:
> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> >> 100+-+Relax+Type+constraints+in+Kafka+Streams+API
> >> >>
> >> >> * I may have slight preference on reusing existing classes but I
> think
> >> most
> >> >> of my rationales are quite subjective. Personally I do not find `self
> >> >> documenting` worth a new class but I can be convinced if people have
> >> other
> >> >> motivations doing it :)
> >> >>
> >> >>
> >> >> Guozhang
> >> >>
> >> >>
> >> >> On Tue, May 15, 2018 at 11:19 AM, John Roesler 
> >> wrote:
> >> >>
> >> >>> Thanks for the KIP, Guozhang.
> >> >>>
> >> >>> It looks good overall to me; I just have one question:
> >> >>> * Why do we bound the generics of KVMapper in KStream to be
> >> >> superclass-of-K
> >> >>> and superclass-of-V instead of exactly K and V, as in Topology? I
> >> might
> >> >> be
> >> >>> thinking about it wrong, but that seems backwards to me. If
> anything,
> >> >>> bounding to be a subclass-of-K or subclass-of-V would seem right to
> >> me.
> >> >>>
> >> >>> One extra thought: I agree that KVMapper
> >> is an
> >> >>> applicable type for extracting the topic name, but I wonder what the
> >> >> value
> >> >>> of reusing the KVMapper for this purpose is. Would defining a new
> >> class,
> >> >>> say TopicNameExtractor, provide the same functionality while
> >> being a
> >> >>> bit more self-documenting?
> >> >>>
> >> >>> Thanks,
> >> >>> -John
> >> >>>
> >> >>> On Tue, May 15, 2018 at 12:32 AM, Guozhang Wang  >
> >> >>> wrote:
> >> >>>
> >>  Hello folks,
> >> 
> >>  I'd like to start a discussion on adding dynamic routing
> >> functionality
> >> >> in
> >>  Streams sink node. I.e. users do not need to specify the topic name
> >> at
> >>  compilation time but can dynamically determine which topic to send
> to
> >> >>> based
> >>  on each record's key value pairs. Please find a KIP here:
> >> 
> >>  https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>  303%3A+Add+Dynamic+Routing+in+Streams+Sink
> >> 
> >>  Any feedbacks are highly appreciated.
> >> 
> >>  Thanks!
> >> 
> >>  -- Guozhang
> >> 
> >> >>>
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> -- Guozhang
> >> >>
> >> >
> >>
> >>
> >
> >
> > --
> > -- Guozhang
> >
>
>
>
> --
> -- Guozhang
>

Re: Kafka Elasticsearch Connector

[Rahul Singh]

Which method did you use the most. What exactly are you having a problem with.

--
Rahul Singh
rahul.si...@anant.us

Anant Corporation

On May 15, 2018, 7:44 AM -0500, Raj, Gokul (External) 
, wrote:
> Hi Team,
> Am new to this tech. I need to connect Kafka to Elastic search using 
> Windows(To consume the data from Kafka with the help of Elastic search). I 
> have tried so many methods but I can't finish this one. Is there any way to 
> do this in windows or direct me to the respective place. Thanks in advance 
> and glad to be a part of this forum.
>
> Thanks & Regards
> Gokul Raj S
>

Build failed in Jenkins: kafka-trunk-jdk7 #3431

[Apache Jenkins Server]

See 


Changes:

[wangguoz] KAFKA-6905: Document that Processors may be re-used by Streams 
(#5022)

[github] MINOR: fix broken links in streams doc (#5025)

--
[...truncated 420.11 KB...]

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithBrokers STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithBrokers PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithTopicsOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithTopicsOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumVerifyOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Colin McCabe]

> On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram 
> wrote:
> 
> > Hi Piyush,
> >
> > It is possible to configure PrincipalBuilder for SASL (
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 189%3A+Improve+principal+builder+interface+and+add+support+for+SASL). If
> > that satisfies your requirements, perhaps we can move wildcarded principals
> > out of this KIP and focus on wildcarded resources?

+1.

We also need to determine which characters will be reserved for the future.  I 
think previously we thought about @, #, $, %, ^, &, *.

> > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay 
> > wrote:
> >
> >> Hi Colin,
> >>
> >> Escaping at this level is making sense to me but let me think more and get
> >> back to you.

Thanks, Piyush.  What questions do you think are still open regarding escape 
characters?
As Rajini mentioned, we have to get this in soon in order to make the KIP 
freeze.

> >>
> >> But should we not just get rid of one of AclBinding or AclBindingFilter
> >> then? Is there a reason to keep both given that AclBindingFilter and
> >> AclBinding look exact copy of each other after this change? This will be a
> >> one-time breaking change in APIs marked as "Evolving", but makes sense in
> >> the long term? Am I missing something here?

AclBinding represents an ACL.  AclBindingFilter is a filter which can be used 
to locate AclBinding objects.  Similarly with Resource and ResourceFilter.  
There is no reason to combine them because they represent different things.  
Although they contain many of the same fields, they are not exact copies.  Many 
fields can be null in AclBindingFilter-- fields can never be null in AclBinding.

For example, you can have an AclBindingFilter that matches every AclBinding.  
There is more discussion of this on the original KIP that added ACL support to 
AdminClient.

best,
Colin

> >>
> >>
> >>
> >> Piyush Vijay
> >>
> >> On Tue, May 15, 2018 at 9:01 AM, Colin McCabe  wrote:
> >>
> >> > Hi Piyush,
> >> >
> >> > I think AclBinding should operate the same way as AclBindingFilter.
> >> >
> >> > So you should be able to do something like this:
> >> > > AclBindingFilter filter = new AclBindingFiler(new
> >> > ResourceFilter(ResourceType.GROUP, "foo*"))
> >> > > AclBinding binding = new AclBinding(new Resource(ResourceType.GROUP,
> >> > "foo*"))
> >> > > assertTrue(filter.matches(binding));
> >> >
> >> > Thinking about this more, it's starting to feel really messy to create
> >> new
> >> > "pattern" constructors for Resource and ResourceFilter.  I don't think
> >> > people will be able to figure this out.  Maybe we should just have a
> >> > limited compatibility break here, where it is now required to escape
> >> weird
> >> > consumer group names when creating ACLs for them.
> >> >
> >> > To future-proof this, we should reserve a bunch of characters at once,
> >> > like *, @, $, %, ^, &, +, [, ], etc.  If these characters appear in a
> >> > resource name, it should be an error, unless they are escaped with a
> >> > backslash.  That way, we can use them in the future.  We should create a
> >> > Resource.escapeName function which adds the correct escape characters to
> >> > resource names (so it would translate foo* into foo\*, foo+bar into
> >> > foo\+bar, etc. etc.
> >> >
> >> > best,
> >> > Colin
> >> >
> >> >
> >> > On Mon, May 14, 2018, at 17:08, Piyush Vijay wrote:
> >> > > Colin,
> >> > >
> >> > > createAcls take a AclBinding, however, instead of AclBindingFilter.
> >> What
> >> > > are your thoughts here?
> >> > >
> >> > > public abstract DescribeAclsResult describeAcls(AclBindingFilter
> >> > > filter, DescribeAclsOptions options);
> >> > >
> >> > > public abstract CreateAclsResult createAcls(Collection
> >> > > acls, CreateAclsOptions options);
> >> > >
> >> > > public abstract DeleteAclsResult
> >> > > deleteAcls(Collection filters, DeleteAclsOptions
> >> > > options);
> >> > >
> >> > >
> >> > > Thanks
> >> > >
> >> > > Piyush Vijay
> >> > >
> >> > > On Mon, May 14, 2018 at 9:26 AM, Andy Coates 
> >> wrote:
> >> > >
> >> > > > +1
> >> > > >
> >> > > > On 11 May 2018 at 17:14, Colin McCabe  wrote:
> >> > > >
> >> > > > > Hi Andy,
> >> > > > >
> >> > > > > I see what you mean.  I guess my thought here is that if the
> >> fields
> >> > are
> >> > > > > private, we can change it later if we need to.
> >> > > > >
> >> > > > > I definitely agree that we should use the scheme you describe for
> >> > sending
> >> > > > > ACLs over the wire (just the string + version number)
> >> > > > >
> >> > > > > cheers,
> >> > > > > Colin
> >> > > > >
> >> > > > >
> >> > > > > On Fri, May 11, 2018, at 09:39, Andy Coates wrote:
> >> > > > > > i think I'm agreeing with you. I was merely suggesting that
> >> having
> >> > an
> >> > > > > > additional field that controls how the current field is
> >> > interpreted is
> >> > > > > more
> >> > > > > > flexible / extensible 

Build failed in Jenkins: kafka-trunk-jdk8 #2648

[Apache Jenkins Server]

See 


Changes:

[wangguoz] KAFKA-6905: Document that Processors may be re-used by Streams 
(#5022)

--
[...truncated 424.29 KB...]
kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsShiftPlus STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsShiftPlus PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLatest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsNewConsumerExistingTopic PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsShiftByLowerThanEarliest PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsByDuration PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToLocalDateTime 
PASSED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions STARTED

kafka.admin.ResetConsumerGroupOffsetTest > 
testResetOffsetsToEarliestOnTopicsAndPartitions PASSED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
STARTED

kafka.admin.ResetConsumerGroupOffsetTest > testResetOffsetsToEarliestOnTopics 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfBlankArg PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowVerifyWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowTopicsOptionWithVerify PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithThrottleOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldFailIfNoArgs PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithoutReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowBrokersListWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumExecuteOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithReassignmentOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldCorrectlyParseValidMinimumGenerateOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowGenerateWithoutBrokersAndTopicsOptions PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowThrottleWithVerifyOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > shouldUseDefaultsIfEnabled 
PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldAllowThrottleOptionOnExecute PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithBrokers STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithBrokers PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithTopicsOption STARTED

kafka.admin.ReassignPartitionsCommandArgsTest > 
shouldNotAllowExecuteWithTopicsOption PASSED

kafka.admin.ReassignPartitionsCommandArgsTest > 

Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved IP addresses

[Edoardo Comar]

Hi Jonathan,
I am afraid that may not work for everybody.

It would not work for us.
With our current DNS, my Kafka clients are perfectly happy to use any IPs -
DNS has multiple A records for the 'myhostname.mydomain' used for
bootstrap and advertised listeners.
The hosts all serve TLS certificates that include
'myhostname.mydomain'  and the clients are happy.

However, applying getCanonicalHostName to those IPs would return
hostnames that would not match the TLS certificates.

So once again I believe your solution and ours solve different use cases.

cheers
Edo

On 16 May 2018 at 18:29, Skrzypek, Jonathan  wrote:
> I think there are combinations that will break SASL and SSL auth.
> Could the trick be to have a single parameter that triggers dns resolve both 
> for bootstrap and advertised listeners, both using getCanonicalHostName() ?
>
> Jonathan Skrzypek
>
> -Original Message-
> From: Edoardo Comar [mailto:edoco...@gmail.com]
> Sent: 16 May 2018 17:03
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved 
> IP addresses
>
> Hi Rajini,
>
> In your example KIP-302 would attempt to connect to the first address
> returned, let's say
>
> www.apache.org/195.154.151.36
>
> then, only if that fails, will in turn try the remaining:
>
> www.apache.org/40.79.78.1
> www.apache.org/140.211.11.105
> www.apache.org/2001:bc8:2142:300:0:0:0:0
>
> You're right to say that we expect certificates served by those
> endpoints to be valid for "www.apache.org"
>
> Without KIP-302, only one would be attempted.
> Which is the first one, that can change every time
> (typically changes on every Java process restart,
> but may change also any time InetAddress.getAllByName it's invoked
> depending on the caching).
>
> The behavioral change that KIP-302 may introduce is that in the example above,
> also an IPv6 connection may be attempted after some IPv4 ones.
>
> InetAddress.getAllByName() implementation uses a system property
> "java.net.preferIPv6Addresses"
> to decide which type of address to return first (default is still IPv4
> in java 10)
>
> We will amend the KIP and PR so that the loop only uses IPs of the
> same type as the first one returned.
>
> A part from the above, KIP 302 does not seem to change any existing
> client behaviour, as any one of multiple IP addresses (of a given
> v4/v6 type) can currently be picked.
> We're happy however to keep the looping behavior optional with the
> discussed config property, disabled by default.
>
> As for KIP-235 that may introduce new hostnames in the bootstrap list
> (the current PR rewrites the bootstrap list)
> and we fail to see the conflict with KIP-302, whatever the set of
> configs chosen.
>
> We'd be happy to try understand what we are missing in a KIP call :-)
>
> cheers
> Edo
>
> On 15 May 2018 at 16:58, Rajini Sivaram  wrote:
>> Hi Edo,
>>
>> I agree that KIP-235 and KIP-302 address different scenarios. And I agree
>> that each one is not sufficient in itself to address both the scenarios.
>> But I also think that they conflict and hence they need to be looked at
>> together and perhaps use a single config.
>>
>> As an example:
>>
>> If I run:
>>
>> for (InetAddress address : InetAddress.getAllByName("www.apache.org")) {
>> System.out.printf("HostName %s canonicalHostName %s IP %s\n",
>> address.getHostName(), address.getCanonicalHostName(),
>> address.getHostAddress());
>> }
>>
>> I get:
>>
>> HostName www.apache.org canonicalHostName tlp-eu-west.apache.org IP
>> 195.154.151.36
>> HostName www.apache.org canonicalHostName 40.79.78.1 IP 40.79.78.1
>> HostName www.apache.org canonicalHostName themis.apache.org IP
>> 140.211.11.105
>> HostName www.apache.org canonicalHostName 2001:bc8:2142:300:0:0:0:0 IP
>> 2001:bc8:2142:300:0:0:0:0
>>
>>
>> If www.apache.org is used as a bootstrap address, KIP-302 would connect to (
>>  www.apache.org/195.154.151.36 and www.apache.org/140.211.11.105) while
>> KIP-235 would connect to (tlp-eu-west.apache.org/195.154.151.3. and
>> themis.apache.org/140.211.11.105). This is a significant difference not
>> just for Kerberos, but for any secure environment where hostname is
>> verified to prevent man-in-the-middle attacks. In your case, I presume you
>> would have SSL certificates with the equivalent of www.apache.org on both
>> the load balancers. In Jonathan's case, I presume he has Kerberos
>> principals for the equivalent of tlp-eu-west.apache.org and
>> themis.apache.org. We would want to support both scenarios regardless of
>> the security protocol, just need to come up with configuration options that
>> don't conflict.
>>
>>
>> On Mon, May 14, 2018 at 5:24 PM, Edoardo Comar  wrote:
>>
>>> Thanks Rajini
>>>
>>> I still don't see the overlap between the two KIPS
>>>
>>> KIP-235 allows an expansion of hostnames on the bootstrap list.
>>>
>>> KIP-302 allows alternative IPs to be used to attempt 

[jira] [Resolved] (KAFKA-6838) Transaction timeout after punctuation results in ProducerFencedException

[Matthias J. Sax (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-6838?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Matthias J. Sax resolved KAFKA-6838.

Resolution: Duplicate

Thanks for confirmation [~feli6]! Closing as duplicate.

> Transaction timeout after punctuation results in ProducerFencedException
> 
>
> Key: KAFKA-6838
> URL: https://issues.apache.org/jira/browse/KAFKA-6838
> Project: Kafka
>  Issue Type: Bug
>  Components: streams
>Affects Versions: 1.1.0
>Reporter: Felix Dsouza
>Priority: Major
> Attachments: punctuat-producer-fencing-borker.log
>
>
> It looks like the EOS/transaction handling for stream punctuation does not 
> work correctly.
> We have encountered ProducerFencedException in a specific case where new 
> events do not arrive before the transaction started by punctuation times out.
> Following squence of events trigger this scenario.
>  1. Punctuation is triggered, we delete some records from the State store 
> (logging enabled) . A transaction is registered on the broker. 
> txnState=Ongoing
> 2. There are no new events to be consumed (period of inactivity) and no new 
> punctuation happening.
> 3. Broker aborts the transaction after about 6 ms  (default transaction 
> timeout duration for the stream) , txnState=Ongoing -> txnState=PrepareAbort  
> ->  txnState=CompleteAbort
> 4. A new event comes in and hits producer fenced exception
>  
> Attached are the logs from the broker where a successful processing, along 
> with transaction timeout and producer fencing is shown.
>  
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: [DISCUSS] KIP-303: Add Dynamic Routing in Streams Sink

[Guozhang Wang]

Hello folks,

Please let me know if you have further feedbacks; if there is no more
feedbacks I'm going to start the voting thread soon.


Guozhang


On Wed, May 16, 2018 at 8:31 AM, Guozhang Wang  wrote:

> I have thought about exposing record context as well, and in the end I
> decided to piggy-back it with KIP-159. And if we want to indeed reuse the
> class it will be:
>
> ```
> public interface RichKeyValueMapper {
> VR apply(final K key, final V value, final RecordContext
> recordContext);
> }
> ```
>
>
>
> Guozhang
>
>
> On Tue, May 15, 2018 at 10:04 PM, Matthias J. Sax 
> wrote:
>
>> Just my 2 cents:
>>
>> I am fine with `KeyValueMapper` (+1 for code reusage) -- the JavaDocs
>> will explain what the `KeyValueMapper` is supposed to do, ie, extract
>> and return the sink topic name from the key-value pair.
>>
>> A side remark though: do we think that accessing key/value is
>> sufficient? Or should we provide access to the full metadata? We could
>> also do this with KIP-159 of course -- but this would come earliest in
>> 2.1. As an alternative we could add a `TopicNameExtractor` to expose the
>> whole record context. The advantage would be, that we don't need to
>> change it via KIP-159 later again. WDYT?
>>
>> -Matthias
>>
>> On 5/15/18 5:57 PM, Bill Bejeck wrote:
>> > Thanks for the KIP Guozhang, it's a +1 for me.
>> >
>> > As for re-using the KeyValueMapper for choosing the topic, I am on the
>> > fence, a more explicitly named class would be more clear, but I'm not
>> sure
>> > it's worth a new class that will primarily perform the same actions as
>> the
>> > KeyValueMapper.
>> >
>> > Thanks,
>> > Bill
>> >
>> > On Tue, May 15, 2018 at 5:52 PM, Guozhang Wang 
>> wrote:
>> >
>> >> Hello John:
>> >>
>> >> * As for the type superclass, it is mainly for allowing super class
>> serdes.
>> >> More details can be found here:
>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> >> 100+-+Relax+Type+constraints+in+Kafka+Streams+API
>> >>
>> >> * I may have slight preference on reusing existing classes but I think
>> most
>> >> of my rationales are quite subjective. Personally I do not find `self
>> >> documenting` worth a new class but I can be convinced if people have
>> other
>> >> motivations doing it :)
>> >>
>> >>
>> >> Guozhang
>> >>
>> >>
>> >> On Tue, May 15, 2018 at 11:19 AM, John Roesler 
>> wrote:
>> >>
>> >>> Thanks for the KIP, Guozhang.
>> >>>
>> >>> It looks good overall to me; I just have one question:
>> >>> * Why do we bound the generics of KVMapper in KStream to be
>> >> superclass-of-K
>> >>> and superclass-of-V instead of exactly K and V, as in Topology? I
>> might
>> >> be
>> >>> thinking about it wrong, but that seems backwards to me. If anything,
>> >>> bounding to be a subclass-of-K or subclass-of-V would seem right to
>> me.
>> >>>
>> >>> One extra thought: I agree that KVMapper
>> is an
>> >>> applicable type for extracting the topic name, but I wonder what the
>> >> value
>> >>> of reusing the KVMapper for this purpose is. Would defining a new
>> class,
>> >>> say TopicNameExtractor, provide the same functionality while
>> being a
>> >>> bit more self-documenting?
>> >>>
>> >>> Thanks,
>> >>> -John
>> >>>
>> >>> On Tue, May 15, 2018 at 12:32 AM, Guozhang Wang 
>> >>> wrote:
>> >>>
>>  Hello folks,
>> 
>>  I'd like to start a discussion on adding dynamic routing
>> functionality
>> >> in
>>  Streams sink node. I.e. users do not need to specify the topic name
>> at
>>  compilation time but can dynamically determine which topic to send to
>> >>> based
>>  on each record's key value pairs. Please find a KIP here:
>> 
>>  https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>  303%3A+Add+Dynamic+Routing+in+Streams+Sink
>> 
>>  Any feedbacks are highly appreciated.
>> 
>>  Thanks!
>> 
>>  -- Guozhang
>> 
>> >>>
>> >>
>> >>
>> >>
>> >> --
>> >> -- Guozhang
>> >>
>> >
>>
>>
>
>
> --
> -- Guozhang
>



-- 
-- Guozhang

[jira] [Resolved] (KAFKA-6905) Document that Processor objects can be reused

[Guozhang Wang (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-6905?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Guozhang Wang resolved KAFKA-6905.
--
   Resolution: Fixed
Fix Version/s: 2.0.0

> Document that Processor objects can be reused
> -
>
> Key: KAFKA-6905
> URL: https://issues.apache.org/jira/browse/KAFKA-6905
> Project: Kafka
>  Issue Type: Improvement
>  Components: documentation, streams
>Reporter: David Glasser
>Priority: Major
> Fix For: 2.0.0
>
>
> We learned the hard way that Kafka Streams will reuse Processor objects by 
> calling init() on them after they've been close()d.  This caused a bug in our 
> application as we assumed we didn't have to reset all of our Processor's 
> state to a proper starting state on init().
> As far as I can tell, this is completely undocumented. The fact that we 
> provide Processors to Kafka Streams via a ProcessorSupplier factory rather 
> than just by passing in a Processor object made it seem likely that in fact 
> Streams was creating Processors from scratch each time it needed a new one.
> The developer guide 
> ([https://docs.confluent.io/current/streams/developer-guide/processor-api.html)]
>  doesn't even allude to the existence of the close() method, let alone the 
> idea that init() may be called after close().
> The Javadocs for Processor.init says: "The framework ensures this is called 
> once per processor when the topology that contains it is initialized."  I 
> personally interpreted that as meaning that it only is ever called once!  I 
> can see that you could interpret it otherwise, but it's definitely unclear.
> I can send a PR but first want to confirm that this is a doc problem and not 
> a bug!



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[DISCUSS] KIP-304: Connect runtime mode improvements for container platforms

[Saulius Valatka]

Hi,

I'd like to start a discussion on the following KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-304%3A+Connect+runtime+mode+improvements+for+container+platforms

Basically the idea is to make it easier to run separate instances of Kafka
Connect hosting isolated connectors on container platforms such as Mesos or
Kubernetes.

In particular it would be interesting to hear opinions about the proposed
read-only REST API mode, more specifically I'm concerned about the
possibility to implement it in distributed mode as it appears the framework
is using it internally (
https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java#L1019),
however this particular API method appears to be undocumented(?).

Looking forward for your feedback.

Re: [VOTE] KIP-283: Efficient Memory Usage for Down-Conversion

[Dhruvil Shah]

Thanks, Ismael. I added the "Testing Strategy" section to the KIP outlining
the findings.

I am also closing this vote with 3 binding and 1 non-binding +1s and no
objections.

Thanks everyone for your review and feedback.

- Dhruvil

On Tue, May 15, 2018 at 11:04 AM, Ismael Juma  wrote:

> Thanks for the KIP Dhruvil, this is a welcome improvement! My understanding
> is that you have done some work to validate that the change has the desired
> effect, it would be good to include that information in the "Testing
> Strategy" section.
>
> +1 (binding)
>
> Ismael
>
> On Wed, May 2, 2018 at 9:27 AM Dhruvil Shah  wrote:
>
> > Hi all,
> >
> > I would like to start the vote on KIP-238: Efficient Memory Usage for
> > Down-Conversion.
> >
> > For reference, the link to the KIP is here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 283%3A+Efficient+Memory+Usage+for+Down-Conversion
> >
> > and the discussion thread is here:
> > https://www.mail-archive.com/dev@kafka.apache.org/msg86799.html
> >
> > Thanks,
> > Dhruvil
> >
>

Re: [DISCUSS] KIP-298: Error Handling in Connect

[Konstantine Karantasis]

Arjun, it's exciting to see a KIP around better handling of bad-data and
errors in Kafka Connect.

I have only a few comments below, which I hope you'll find helpful.

1. I think it'd be useful to describe a bit more in detail how someone can
extract the raw data of a Kafka record that failed to get converted (on the
sink side in this example). How's the JSON schema looks like for an entry
that is added to the dead-letter-queue and what someone should do to get
the raw bytes?

2. Similarly, it'd be nice to describe a bit more what is placed or
attempted to be placed in the dead-letter-queue in the case of source
records that fail to get imported to Kafka. Currently the only sentence I
read related to that is: "Similarly, for source connectors, the developer
can write the corrected records back to the original source".

3. I think the plural for 'retries' in config options:
'errors.retries.limit' and 'errors.retries.delay.max.ms' doesn't read very
well. Should 'retry' be used same as 'tolerance' (or 'log') is used right
below? For example:
errors.retry.limit
and
errors.retry.delay.max.ms

4. Should the metric names be 'total-record-failures' and
'total-records-skipped' to match their metric description and also be
similar to 'total-retries'?

And a few minor comments:

- The domain of 'errors.retries.limit' does not include 0 in the allowed
values (even though it's the default value).

- For someone unfamiliar with the term SMT, the acronym is not explained in
the text. Also the term transformations is better IMO.

- typo: 'the task is to killed'

- If you intend to add a link to a PR additionally to the jira ticket, it'd
be handy to add it to the KIP header (along with state, thread, jira, etc).
Now it's a bit hidden in the text and it's not clear that the KIP includes
a link to a PR.

Thanks for working on this missing but important functionality.

- Konstantine


On Tue, May 15, 2018 at 10:41 PM, Arjun Satish 
wrote:

> Magesh,
>
> Just to add to your point about retriable exceptions: the producer can
> throw retriable exceptions which we are handling it here:
>
> https://github.com/apache/kafka/blob/trunk/connect/
> runtime/src/main/java/org/apache/kafka/connect/runtime/
> WorkerSourceTask.java#L275
>
> BTW, exceptions like TimeoutExceptions (which extend RetriableExceptions)
> are bubbled back to the application, and need to be handled as per
> application requirements.
>
> Best,
>
> On Tue, May 15, 2018 at 8:30 PM, Arjun Satish 
> wrote:
>
> > Magesh,
> >
> > Thanks for the feedback! Really appreciate your comments.
> >
> > 1. I updated the KIP to state that only the configs of the failed
> > operation will be emitted. Thank you!
> >
> > The purpose of bundling the configs of the failed operation along with
> the
> > error context is to have a single place to find everything relevant to
> the
> > failure. This way, we can only look at the error logs to find the most
> > common pieces to "failure" puzzles: the operation, the config and the
> input
> > record. Ideally, a programmer should be able to take these pieces and
> > reproduce the error locally.
> >
> > 2. Added a table to describe this in the KIP.
> >
> > 3. Raw bytes will be base64 encoded before being logged. Updated the KIP
> > to state this. Thank you!
> >
> > 4. I'll add an example log4j config to show we can take logs from a class
> > and redirect it to a different location. Made a note in the PR for this.
> >
> > 5. When we talk about logging messages, this could mean instances of
> > SinkRecords or SourceRecords. When we disable logging of messages, these
> > records would be replaced by a "null". If you think it makes sense,
> instead
> > of completely dropping the object, we could drop only the key and value
> > objects from ConnectRecord? That way some context will still be retained.
> >
> > 6. Yes, for now I think it is good to have explicit config in Connectors
> > which dictates the error handling behavior. If this becomes an
> > inconvenience, we can think of having a cluster global default, or better
> > defaults in the configs.
> >
> > Best,
> >
> >
> > On Tue, May 15, 2018 at 1:07 PM, Magesh Nandakumar  >
> > wrote:
> >
> >> Hi Arjun,
> >>
> >> I think this a great KIP and would be a great addition to have in
> connect.
> >> Had a couple of minor questions:
> >>
> >> 1. What would be the value in logging the connector config using
> >> errors.log.include.configs
> >> for every message?
> >> 2. Not being picky on format here but it might be clearer if the
> behavior
> >> is called out for each stage separately and what the connector
> developers
> >> need to do ( may be a tabular format). Also, I think all retriable
> >> exception when talking to Broker are never propagated to the Connect
> >> Framework since the producer is configured to try indefinitely
> >> 3. If a message fails in serialization, would the raw bytes be available
> >> to
> >> the dlq or 

Re: [DISCUSS] KIP-297: Externalizing Secrets for Connect Configurations

[Rajini Sivaram]

Thanks for the update, Robert. Looks good to me.

Regards,

Rajini

On Wed, May 16, 2018 at 4:43 PM, Robert Yokota  wrote:

> Hi Rajini,
>
> Thanks for the excellent feedback!
>
> I've made the API changes that you've requested in the KIP.
>
>
> > 1. Are we expecting one provider instance with different contexts
> > provided to `ConfigProvider.get()`? If we created a different provider
> > instance for each context, we could deal with scheduling reloads in the
> > provider implementation?
>
> Yes, there would be one provider instance.  I've collapsed the
> ConfigContext and the ConfigChangeCallback by adding a parameter delayMs to
> indicate when the change will happen.  When a particular ConfigProvider
> retrieves a lease duration along with a key, it can either 1)  schedule a
> background thread to push out the change when it happens (at which time the
> delayMs will be 0), or invoke the callback immediately with the lease
> duration set as delayMs (of course, in this case the values for the keys
> will be the old values).  A ConfProvider could be parameterized to do one
> or the other.
>
>
> > 2. Couldn't ConfigData  be an interface that just returns a map of
> > key-value pairs. Providers that return metadata could extend it to
> provide
> > metadata in a meaningful format instead of Map.
>
> I've replaced ConfigData with Map as you suggested.
>
>
> > 3. For ZK, we would use ConfigProvider.get() without `keys` to get all
> > keys in the path. Do we have two get() methods since some providers need
> > keys to be specified and some don't? How do we decide which one to use?
>
> The ConfigProvider should be thought of like a Map interface and does not
> require that one signature of get() be preferred over the other.  KIP-226
> can use get(String path) while Connect will use get(String path,
> Set) since it knows which keys it is interested in.
>
>
> A few more updates to the KIP:
>
> - I've elided the ConfigTransformer implementation as Colin suggested.
> - The variable reference now looks like ${provider:[path:]key} where the
> path is optional.
>
>
> Thanks!
> Robert
>
>
>
>
> On Wed, May 16, 2018 at 4:30 AM, Rajini Sivaram 
> wrote:
>
> > Hi Robert,
> >
> > Thanks for the KIP updates.
> >
> > The interfaces look suitable for brokers, with some small changes. If we
> > can adapt the interface to implement the existing DynamicBrokerConfig,
> then
> > we are good.
> >
> > With broker configs:
> >
> >1. We don't know what configs are in ZK since we allow custom configs.
> >So we would use `ConfigProvider.get()` without specifying keys.
> >2. We want to see all changes (i.e. changes under a path). We can deal
> >with this internally by ignoring `keys` and subscribing to everything
> >3. We have two paths (one for per-broker config and another for
> default
> >config shared by all brokers). All methods should ideally provide
> path -
> >see changes suggested below.
> >4. Keys are not independent. We update in batches (e.g keystore +
> >password). We want to see batches of changes, not individual changes.
> We
> >retrieve all values from a path when a change is detected. We can do
> > this
> >by ignoring values from the callback, but it would be better if the
> >callback interface could be changed - see below.
> >
> >
> > public interface ConfigProvider extends Configurable, Closeable {
> >
> > *//** KIP-226 will use this*
> > ConfigData get(ConfigContext ctx, String path);
> >
> > *// **KIP-226 will never use this, we don't know what keys are in ZK
> > since we allow custom configs*
> > ConfigData get(ConfigContext ctx, String path, Set keys);
> >
> > *// KIP-226 will ignore `key` and subscribe to all changes.*
> > *// But based on the above method, this should perhaps be:*
> > *//  subscribe(String path, Set keys,
> > ConfigurationChangeCallback callback)?*
> > void subscribe(String key, ConfigurationChangeCallback callback);
> >
> >  *<== As above, un**subscribe(String path, Set keys)**?*
> > void unsubscribe(String key);
> > }
> >
> > public interface ConfigurationChangeCallback {
> > *// **For brokers, we want to process all updated keys in a single
> > callback. P**erhaps this could be: *
> >
> > *//   onChange(String path, Map values)?*
> >
> > void onChange(String key, String value);
> > }
> >
> > A few other questions (I read your response to Colin, but still didn't
> get
> > it. Could be because I am not familiar with the interfaces required for
> > vaults, sorry):
> >
> >1. Are we expecting one provider instance with different contexts
> >provided to `ConfigProvider.get()`? If we created a different provider
> >instance for each context, we could deal with scheduling reloads in
> the
> >provider implementation?
> >2. Couldn't ConfigData  be an interface that just returns a map of
> >

RE: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved IP addresses

[Skrzypek, Jonathan]

I think there are combinations that will break SASL and SSL auth.
Could the trick be to have a single parameter that triggers dns resolve both 
for bootstrap and advertised listeners, both using getCanonicalHostName() ?

Jonathan Skrzypek 

-Original Message-
From: Edoardo Comar [mailto:edoco...@gmail.com] 
Sent: 16 May 2018 17:03
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved 
IP addresses

Hi Rajini,

In your example KIP-302 would attempt to connect to the first address
returned, let's say

www.apache.org/195.154.151.36

then, only if that fails, will in turn try the remaining:

www.apache.org/40.79.78.1
www.apache.org/140.211.11.105
www.apache.org/2001:bc8:2142:300:0:0:0:0

You're right to say that we expect certificates served by those
endpoints to be valid for "www.apache.org"

Without KIP-302, only one would be attempted.
Which is the first one, that can change every time
(typically changes on every Java process restart,
but may change also any time InetAddress.getAllByName it's invoked
depending on the caching).

The behavioral change that KIP-302 may introduce is that in the example above,
also an IPv6 connection may be attempted after some IPv4 ones.

InetAddress.getAllByName() implementation uses a system property
"java.net.preferIPv6Addresses"
to decide which type of address to return first (default is still IPv4
in java 10)

We will amend the KIP and PR so that the loop only uses IPs of the
same type as the first one returned.

A part from the above, KIP 302 does not seem to change any existing
client behaviour, as any one of multiple IP addresses (of a given
v4/v6 type) can currently be picked.
We're happy however to keep the looping behavior optional with the
discussed config property, disabled by default.

As for KIP-235 that may introduce new hostnames in the bootstrap list
(the current PR rewrites the bootstrap list)
and we fail to see the conflict with KIP-302, whatever the set of
configs chosen.

We'd be happy to try understand what we are missing in a KIP call :-)

cheers
Edo

On 15 May 2018 at 16:58, Rajini Sivaram  wrote:
> Hi Edo,
>
> I agree that KIP-235 and KIP-302 address different scenarios. And I agree
> that each one is not sufficient in itself to address both the scenarios.
> But I also think that they conflict and hence they need to be looked at
> together and perhaps use a single config.
>
> As an example:
>
> If I run:
>
> for (InetAddress address : InetAddress.getAllByName("www.apache.org")) {
> System.out.printf("HostName %s canonicalHostName %s IP %s\n",
> address.getHostName(), address.getCanonicalHostName(),
> address.getHostAddress());
> }
>
> I get:
>
> HostName www.apache.org canonicalHostName tlp-eu-west.apache.org IP
> 195.154.151.36
> HostName www.apache.org canonicalHostName 40.79.78.1 IP 40.79.78.1
> HostName www.apache.org canonicalHostName themis.apache.org IP
> 140.211.11.105
> HostName www.apache.org canonicalHostName 2001:bc8:2142:300:0:0:0:0 IP
> 2001:bc8:2142:300:0:0:0:0
>
>
> If www.apache.org is used as a bootstrap address, KIP-302 would connect to (
>  www.apache.org/195.154.151.36 and www.apache.org/140.211.11.105) while
> KIP-235 would connect to (tlp-eu-west.apache.org/195.154.151.3. and
> themis.apache.org/140.211.11.105). This is a significant difference not
> just for Kerberos, but for any secure environment where hostname is
> verified to prevent man-in-the-middle attacks. In your case, I presume you
> would have SSL certificates with the equivalent of www.apache.org on both
> the load balancers. In Jonathan's case, I presume he has Kerberos
> principals for the equivalent of tlp-eu-west.apache.org and
> themis.apache.org. We would want to support both scenarios regardless of
> the security protocol, just need to come up with configuration options that
> don't conflict.
>
>
> On Mon, May 14, 2018 at 5:24 PM, Edoardo Comar  wrote:
>
>> Thanks Rajini
>>
>> I still don't see the overlap between the two KIPS
>>
>> KIP-235 allows an expansion of hostnames on the bootstrap list.
>>
>> KIP-302 allows alternative IPs to be used to attempt a connection
>> (either at bootstrap and when processing the MetadataResponse) to a
>> given hostname.
>>
>> A use case would be that of active/passive LB fronting the brokers.
>>
>> Arguably, if Java honored the DNS-set TTL, and the TTL was low and on
>> subsequent requests, the order of IPs returned by the
>> InetAddress.getAllByName() was random, we may not need such an
>> enhancement.
>> In practice, a Java client can get stuck on a "bad" IP forever if it
>> only relies on the first IP returned.
>>
>> HTH,
>> Edo
>>
>> On 14 May 2018 at 16:23, Rajini Sivaram  wrote:
>> > Hi Edo,
>> >
>> > Thanks for the KIP. I think it will be good to include a diagram to make
>> it
>> > easier to distinguish this scenario from that of KIP-235 without reading
>> > the PR.
>> >
>> > 

Re: [VOTE] KIP-255: OAuth Authentication via SASL/OAUTHBEARER

[Manikumar]

+1 (non-binding)

Thanks for the detailed KIP.

On Wed, May 16, 2018 at 5:59 PM, Mickael Maison 
wrote:

> Thanks for the KIP,
> +1 (non binding)
>
> On Wed, May 16, 2018 at 2:51 AM, Ron Dagostino  wrote:
> > Hi Jun.  I think you are getting at the fact that OAuth 2 is a flexible
> > framework that allows different installations to do things differently.
> It
> > is true that the principal name in Kafka could come from any claim in the
> > token.  Most of the time it would come from the 'sub' claim, but it could
> > certainly come from another claim, or it could be only indirectly based
> on
> > a claim value (maybe certain text would be trimmed or prefixed/suffixed).
> > The point, which I think you are getting at, is that because the
> framework
> > is flexible we need to accommodate that flexibility.  The callback
> handler
> > class defined by the listener.name.sasl_ssl.oauthbearer.sasl.server.
> > callback.handler.class configuration value gives us the required
> > flexibility.  As an example, I have an implementation that leverages a
> > popular open source JOSE library to parse the compact serialization,
> > retrieve the public key if it has not yet been retrieved, verify the
> > digital signature, and map the 'sub' claim to the OAuthBearerToken's
> > principal name (which becomes the SASL authorization ID, which becomes
> the
> > Kafka principal name).  I could just as easily have mapped a different
> > claim to the OAuthBearerToken's principal name, manipulated the 'sub'
> claim
> > value in some way, etc.  I write the callback handler code, so I complete
> > flexibility to do whatever my OAuth 2 installation requires me to do.
> >
> > Ron
> >
> > On Tue, May 15, 2018 at 1:39 PM, Jun Rao  wrote:
> >
> >> Hi, Ron,
> >>
> >> Thanks for the reply. I understood your answers to #2 and #3.
> >>
> >> For #1, will the server map all clients' principal name to the value
> >> associated with "sub" claim? How do we support mapping different
> clients to
> >> different principal names?
> >>
> >> Jun
> >>
> >> On Mon, May 14, 2018 at 7:02 PM, Ron Dagostino 
> wrote:
> >>
> >> > Hi Jun.  Thanks for the +1 vote.
> >> >
> >> > Regarding the first question about token claims, yes, you have it
> correct
> >> > about translating the OAuth token to a principle name via a JAAS
> module
> >> > option in the default unsecured case.  Specifically, the OAuth SASL
> >> Server
> >> > implementation is responsible for setting the authorization ID, and it
> >> gets
> >> > the authorization ID from the OAuthBearerToken's principalName()
> method.
> >> > The listener.name.sasl_ssl.oauthbearer.sasl.server.
> >> callback.handler.class
> >> > is responsible for handling an instance of
> OAuthBearerValidatorCallback
> >> to
> >> > accept a token compact serialization from the client and return an
> >> instance
> >> > of OAuthBearerToken (assuming the compact serialization validates),
> and
> >> in
> >> > the default unsecured case the builtin unsecured validator callback
> >> handler
> >> > defines the OAuthBearerToken.principalName() method to return the
> 'sub'
> >> > claim value by default (with the actual claim it uses being
> configurable
> >> > via the unsecuredValidatorPrincipalClaimName JAAS module option).  So
> >> that
> >> > is how we translate from a token to a principal name in the default
> >> > unsecured (out-of-the-box) case.
> >> >
> >> > For production use cases, the implementation associated with
> >> > listener.name.sasl_ssl.oauthbearer.sasl.server.callback.handler.class
> >> can
> >> > do whatever it wants.  As an example, I have written a class that
> wraps a
> >> > com.nimbusds.jwt.SignedJWT instance (see
> >> > https://connect2id.com/products/nimbus-jose-jwt/) and presents it as
> an
> >> > OAuthBearerToken:
> >> >
> >> > public class NimbusSignedJwtOAuthBearerToken implements
> >> OAuthBearerToken {
> >> > private final SignedJWT signedJwt;
> >> > private final String principalName;
> >> > private final Set scope;
> >> > private final Long startTimeMs;
> >> > private final long lifetimeMs;
> >> >
> >> > /**
> >> >  * Constructor
> >> >  *
> >> >  * @param signedJwt
> >> >  *the mandatory signed JWT
> >> >  * @param principalClaimName
> >> >  *the mandatory claim name identifying the claim from
> >> which
> >> > the
> >> >  *principal name will be extracted. The claim must
> exist
> >> > and must be
> >> >  *a String.
> >> >  * @param optionalScopeClaimName
> >> >  *the optional claim name identifying the claim from
> >> which
> >> > any scope
> >> >  *will be extracted. If specified and the claim exists
> >> then
> >> > the
> >> >  *value must be either a String or a String List.
> >> >  * @throws ParseException
> >> >  * if the principal claim does 

Re: [VOTE] KIP-295: Add Streams Configuration Allowing for Optional Topology Optimization

[Damian Guy]

+1
On Tue, 15 May 2018 at 15:04, Ted Yu  wrote:

> +1
>  Original message From: Guozhang Wang 
> Date: 5/15/18  2:34 PM  (GMT-08:00) To: dev@kafka.apache.org Subject: Re:
> [VOTE] KIP-295: Add Streams Configuration Allowing for Optional Topology
> Optimization
> +1 (binding).
>
> On Tue, May 15, 2018 at 2:16 PM, Matthias J. Sax 
> wrote:
>
> > +1 (binding)
> >
> >
> > On 5/15/18 1:45 PM, Bill Bejeck wrote:
> > > Hi all,
> > >
> > > I'd like to start a vote on KIP-295: Add Streams Configuration Allowing
> > for
> > > Optional Topology Optimization.
> > >
> > > KIP wiki page:
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 295%3A+Add+Streams+Configuration+Allowing+for+
> > Optional+Topology+Optimization
> > >
> > > Discussion thread:
> > > https://www.mail-archive.com/dev@kafka.apache.org/msg87593.html
> > >
> > > Thanks,
> > > Bill
> > >
> >
> >
>
>
> --
> -- Guozhang
>

Re: [DISCUSS] KIP-302 - Enable Kafka clients to use all DNS resolved IP addresses

[Edoardo Comar]

Hi Rajini,

In your example KIP-302 would attempt to connect to the first address
returned, let's say

www.apache.org/195.154.151.36

then, only if that fails, will in turn try the remaining:

www.apache.org/40.79.78.1
www.apache.org/140.211.11.105
www.apache.org/2001:bc8:2142:300:0:0:0:0

You're right to say that we expect certificates served by those
endpoints to be valid for "www.apache.org"

Without KIP-302, only one would be attempted.
Which is the first one, that can change every time
(typically changes on every Java process restart,
but may change also any time InetAddress.getAllByName it's invoked
depending on the caching).

The behavioral change that KIP-302 may introduce is that in the example above,
also an IPv6 connection may be attempted after some IPv4 ones.

InetAddress.getAllByName() implementation uses a system property
"java.net.preferIPv6Addresses"
to decide which type of address to return first (default is still IPv4
in java 10)

We will amend the KIP and PR so that the loop only uses IPs of the
same type as the first one returned.

A part from the above, KIP 302 does not seem to change any existing
client behaviour, as any one of multiple IP addresses (of a given
v4/v6 type) can currently be picked.
We're happy however to keep the looping behavior optional with the
discussed config property, disabled by default.

As for KIP-235 that may introduce new hostnames in the bootstrap list
(the current PR rewrites the bootstrap list)
and we fail to see the conflict with KIP-302, whatever the set of
configs chosen.

We'd be happy to try understand what we are missing in a KIP call :-)

cheers
Edo

On 15 May 2018 at 16:58, Rajini Sivaram  wrote:
> Hi Edo,
>
> I agree that KIP-235 and KIP-302 address different scenarios. And I agree
> that each one is not sufficient in itself to address both the scenarios.
> But I also think that they conflict and hence they need to be looked at
> together and perhaps use a single config.
>
> As an example:
>
> If I run:
>
> for (InetAddress address : InetAddress.getAllByName("www.apache.org")) {
> System.out.printf("HostName %s canonicalHostName %s IP %s\n",
> address.getHostName(), address.getCanonicalHostName(),
> address.getHostAddress());
> }
>
> I get:
>
> HostName www.apache.org canonicalHostName tlp-eu-west.apache.org IP
> 195.154.151.36
> HostName www.apache.org canonicalHostName 40.79.78.1 IP 40.79.78.1
> HostName www.apache.org canonicalHostName themis.apache.org IP
> 140.211.11.105
> HostName www.apache.org canonicalHostName 2001:bc8:2142:300:0:0:0:0 IP
> 2001:bc8:2142:300:0:0:0:0
>
>
> If www.apache.org is used as a bootstrap address, KIP-302 would connect to (
>  www.apache.org/195.154.151.36 and www.apache.org/140.211.11.105) while
> KIP-235 would connect to (tlp-eu-west.apache.org/195.154.151.3. and
> themis.apache.org/140.211.11.105). This is a significant difference not
> just for Kerberos, but for any secure environment where hostname is
> verified to prevent man-in-the-middle attacks. In your case, I presume you
> would have SSL certificates with the equivalent of www.apache.org on both
> the load balancers. In Jonathan's case, I presume he has Kerberos
> principals for the equivalent of tlp-eu-west.apache.org and
> themis.apache.org. We would want to support both scenarios regardless of
> the security protocol, just need to come up with configuration options that
> don't conflict.
>
>
> On Mon, May 14, 2018 at 5:24 PM, Edoardo Comar  wrote:
>
>> Thanks Rajini
>>
>> I still don't see the overlap between the two KIPS
>>
>> KIP-235 allows an expansion of hostnames on the bootstrap list.
>>
>> KIP-302 allows alternative IPs to be used to attempt a connection
>> (either at bootstrap and when processing the MetadataResponse) to a
>> given hostname.
>>
>> A use case would be that of active/passive LB fronting the brokers.
>>
>> Arguably, if Java honored the DNS-set TTL, and the TTL was low and on
>> subsequent requests, the order of IPs returned by the
>> InetAddress.getAllByName() was random, we may not need such an
>> enhancement.
>> In practice, a Java client can get stuck on a "bad" IP forever if it
>> only relies on the first IP returned.
>>
>> HTH,
>> Edo
>>
>> On 14 May 2018 at 16:23, Rajini Sivaram  wrote:
>> > Hi Edo,
>> >
>> > Thanks for the KIP. I think it will be good to include a diagram to make
>> it
>> > easier to distinguish this scenario from that of KIP-235 without reading
>> > the PR.
>> >
>> > It may be worth considering if KIP-235 and this KIP could use a single
>> > config name with different values instead of two boolean configs:
>> >
>> > bootstrap.reverse.dns.lookup = true/false
>> > enable.all.dns.ips = true/false
>> >
>> > Not all values of (bootstrap.reverse.dns.lookup, enable.all.dns.ips) seem
>> > to make sense. And not all scenarios are handled. Even if we use multiple
>> > configs, it seems to me that we may want to name them 

Re: [DISCUSS] KIP-297: Externalizing Secrets for Connect Configurations

[Robert Yokota]

Hi Rajini,

Thanks for the excellent feedback!

I've made the API changes that you've requested in the KIP.


> 1. Are we expecting one provider instance with different contexts
> provided to `ConfigProvider.get()`? If we created a different provider
> instance for each context, we could deal with scheduling reloads in the
> provider implementation?

Yes, there would be one provider instance.  I've collapsed the
ConfigContext and the ConfigChangeCallback by adding a parameter delayMs to
indicate when the change will happen.  When a particular ConfigProvider
retrieves a lease duration along with a key, it can either 1)  schedule a
background thread to push out the change when it happens (at which time the
delayMs will be 0), or invoke the callback immediately with the lease
duration set as delayMs (of course, in this case the values for the keys
will be the old values).  A ConfProvider could be parameterized to do one
or the other.


> 2. Couldn't ConfigData  be an interface that just returns a map of
> key-value pairs. Providers that return metadata could extend it to provide
> metadata in a meaningful format instead of Map.

I've replaced ConfigData with Map as you suggested.


> 3. For ZK, we would use ConfigProvider.get() without `keys` to get all
> keys in the path. Do we have two get() methods since some providers need
> keys to be specified and some don't? How do we decide which one to use?

The ConfigProvider should be thought of like a Map interface and does not
require that one signature of get() be preferred over the other.  KIP-226
can use get(String path) while Connect will use get(String path,
Set) since it knows which keys it is interested in.


A few more updates to the KIP:

- I've elided the ConfigTransformer implementation as Colin suggested.
- The variable reference now looks like ${provider:[path:]key} where the
path is optional.


Thanks!
Robert




On Wed, May 16, 2018 at 4:30 AM, Rajini Sivaram 
wrote:

> Hi Robert,
>
> Thanks for the KIP updates.
>
> The interfaces look suitable for brokers, with some small changes. If we
> can adapt the interface to implement the existing DynamicBrokerConfig, then
> we are good.
>
> With broker configs:
>
>1. We don't know what configs are in ZK since we allow custom configs.
>So we would use `ConfigProvider.get()` without specifying keys.
>2. We want to see all changes (i.e. changes under a path). We can deal
>with this internally by ignoring `keys` and subscribing to everything
>3. We have two paths (one for per-broker config and another for default
>config shared by all brokers). All methods should ideally provide path -
>see changes suggested below.
>4. Keys are not independent. We update in batches (e.g keystore +
>password). We want to see batches of changes, not individual changes. We
>retrieve all values from a path when a change is detected. We can do
> this
>by ignoring values from the callback, but it would be better if the
>callback interface could be changed - see below.
>
>
> public interface ConfigProvider extends Configurable, Closeable {
>
> *//** KIP-226 will use this*
> ConfigData get(ConfigContext ctx, String path);
>
> *// **KIP-226 will never use this, we don't know what keys are in ZK
> since we allow custom configs*
> ConfigData get(ConfigContext ctx, String path, Set keys);
>
> *// KIP-226 will ignore `key` and subscribe to all changes.*
> *// But based on the above method, this should perhaps be:*
> *//  subscribe(String path, Set keys,
> ConfigurationChangeCallback callback)?*
> void subscribe(String key, ConfigurationChangeCallback callback);
>
>  *<== As above, un**subscribe(String path, Set keys)**?*
> void unsubscribe(String key);
> }
>
> public interface ConfigurationChangeCallback {
> *// **For brokers, we want to process all updated keys in a single
> callback. P**erhaps this could be: *
>
> *//   onChange(String path, Map values)?*
>
> void onChange(String key, String value);
> }
>
> A few other questions (I read your response to Colin, but still didn't get
> it. Could be because I am not familiar with the interfaces required for
> vaults, sorry):
>
>1. Are we expecting one provider instance with different contexts
>provided to `ConfigProvider.get()`? If we created a different provider
>instance for each context, we could deal with scheduling reloads in the
>provider implementation?
>2. Couldn't ConfigData  be an interface that just returns a map of
>key-value pairs. Providers that return metadata could extend it to
> provide
>metadata in a meaningful format instead of Map.
>3. For ZK, we would use ConfigProvider.get() without `keys` to get all
>keys in the path. Do we have two get() methods since some providers need
>keys to be specified and some don't? How do we decide which one to use?

[jira] [Created] (KAFKA-6910) Ability to specify a default state store type or factory

[Antony Stubbs (JIRA)]

Antony Stubbs created KAFKA-6910:


 Summary: Ability to specify a default state store type or factory
 Key: KAFKA-6910
 URL: https://issues.apache.org/jira/browse/KAFKA-6910
 Project: Kafka
  Issue Type: New Feature
  Components: streams
Affects Versions: 1.1.0, 1.1.1
Reporter: Antony Stubbs


For large projects, it's a huge pain and not really practically at all to use a 
custom state store everywhere just to use in memory or avoid rocksdb, for 
example for running a test suite on windows.

 

It would be great to be able to set a global config for KS so that it uses a 
different state store implementation everywhere.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Permission request to open a KIP

[Matthias J. Sax]

Done.

On 5/16/18 7:55 AM, Saulius Valatka wrote:
> Hi,
> 
> could someone please grant me permission to open a KIP? My wiki id is
> saulius.vl
> 
> Thanks
> 



signature.asc
Description: OpenPGP digital signature

Re: [DISCUSS] KIP-303: Add Dynamic Routing in Streams Sink

[Guozhang Wang]

I have thought about exposing record context as well, and in the end I
decided to piggy-back it with KIP-159. And if we want to indeed reuse the
class it will be:

```
public interface RichKeyValueMapper {
VR apply(final K key, final V value, final RecordContext recordContext);
}
```



Guozhang


On Tue, May 15, 2018 at 10:04 PM, Matthias J. Sax 
wrote:

> Just my 2 cents:
>
> I am fine with `KeyValueMapper` (+1 for code reusage) -- the JavaDocs
> will explain what the `KeyValueMapper` is supposed to do, ie, extract
> and return the sink topic name from the key-value pair.
>
> A side remark though: do we think that accessing key/value is
> sufficient? Or should we provide access to the full metadata? We could
> also do this with KIP-159 of course -- but this would come earliest in
> 2.1. As an alternative we could add a `TopicNameExtractor` to expose the
> whole record context. The advantage would be, that we don't need to
> change it via KIP-159 later again. WDYT?
>
> -Matthias
>
> On 5/15/18 5:57 PM, Bill Bejeck wrote:
> > Thanks for the KIP Guozhang, it's a +1 for me.
> >
> > As for re-using the KeyValueMapper for choosing the topic, I am on the
> > fence, a more explicitly named class would be more clear, but I'm not
> sure
> > it's worth a new class that will primarily perform the same actions as
> the
> > KeyValueMapper.
> >
> > Thanks,
> > Bill
> >
> > On Tue, May 15, 2018 at 5:52 PM, Guozhang Wang 
> wrote:
> >
> >> Hello John:
> >>
> >> * As for the type superclass, it is mainly for allowing super class
> serdes.
> >> More details can be found here:
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> 100+-+Relax+Type+constraints+in+Kafka+Streams+API
> >>
> >> * I may have slight preference on reusing existing classes but I think
> most
> >> of my rationales are quite subjective. Personally I do not find `self
> >> documenting` worth a new class but I can be convinced if people have
> other
> >> motivations doing it :)
> >>
> >>
> >> Guozhang
> >>
> >>
> >> On Tue, May 15, 2018 at 11:19 AM, John Roesler 
> wrote:
> >>
> >>> Thanks for the KIP, Guozhang.
> >>>
> >>> It looks good overall to me; I just have one question:
> >>> * Why do we bound the generics of KVMapper in KStream to be
> >> superclass-of-K
> >>> and superclass-of-V instead of exactly K and V, as in Topology? I might
> >> be
> >>> thinking about it wrong, but that seems backwards to me. If anything,
> >>> bounding to be a subclass-of-K or subclass-of-V would seem right to me.
> >>>
> >>> One extra thought: I agree that KVMapper is
> an
> >>> applicable type for extracting the topic name, but I wonder what the
> >> value
> >>> of reusing the KVMapper for this purpose is. Would defining a new
> class,
> >>> say TopicNameExtractor, provide the same functionality while
> being a
> >>> bit more self-documenting?
> >>>
> >>> Thanks,
> >>> -John
> >>>
> >>> On Tue, May 15, 2018 at 12:32 AM, Guozhang Wang 
> >>> wrote:
> >>>
>  Hello folks,
> 
>  I'd like to start a discussion on adding dynamic routing functionality
> >> in
>  Streams sink node. I.e. users do not need to specify the topic name at
>  compilation time but can dynamically determine which topic to send to
> >>> based
>  on each record's key value pairs. Please find a KIP here:
> 
>  https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>  303%3A+Add+Dynamic+Routing+in+Streams+Sink
> 
>  Any feedbacks are highly appreciated.
> 
>  Thanks!
> 
>  -- Guozhang
> 
> >>>
> >>
> >>
> >>
> >> --
> >> -- Guozhang
> >>
> >
>
>


-- 
-- Guozhang

Permission request to open a KIP

[Saulius Valatka]

Hi,

could someone please grant me permission to open a KIP? My wiki id is
saulius.vl

Thanks

Re: Kafka system tests contribution

[Andriy Sorokhtey]

Hi,

Did anyone had a chance to take a look at this issue?

2018-05-08 15:01 GMT+03:00 Andriy Sorokhtey :

> Hello Kafka team
>
> I’d like to contribute to the Kafka system tests.
>
> I’ve tried to execute system tests locally and I have some issues. Can
> anyone give me a hand to figure out what’s wrong?
>
> So, I see that multiple system tests are failing when I try to run it with
> the docker or with vagrant.
> Maybe there is some way to debug it using PyCharm. For example, put some
> breakpoint and start debugging, when the test goes to the breakpoint I’d
> like to go to instances and check what’s going on there.
> I’ll be thankful for any advice.
>
>  Here is an example of one test failure:
>
>> [INFO:2018-05-03 06:37:19,861]: Triggering test 1 of 37...
>> [INFO:2018-05-03 06:37:19,870]: RunnerClient: Loading test {'directory':
>> '/opt/kafka-dev/tests/kafkatest/tests/streams', 'file_name':
>> 'streams_broker_compatibility_test.py', 'method_name':
>> 'test_compatible_brokers_eos_disabled', 'cls_name':
>> 'StreamsBrokerCompatibility', 'injected_args': {'broker_version':
>> '0.10.1.1'}}
>> [INFO:2018-05-03 06:37:19,874]: RunnerClient: kafkatest.tests.streams.
>> streams_broker_compatibility_test.StreamsBrokerCompatibility.
>> test_compatible_brokers_eos_disabled.broker_version=0.10.1.1: Setting
>> up...
>> [INFO:2018-05-03 06:37:22,484]: RunnerClient: kafkatest.tests.streams.
>> streams_broker_compatibility_test.StreamsBrokerCompatibility.
>> test_compatible_brokers_eos_disabled.broker_version=0.10.1.1: Running...
>> [INFO:2018-05-03 06:38:34,129]: RunnerClient: kafkatest.tests.streams.
>> streams_broker_compatibility_test.StreamsBrokerCompatibility.
>> test_compatible_brokers_eos_disabled.broker_version=0.10.1.1: FAIL:
>> Never saw message indicating StreamsTest finished startup on ducker@ducker05
>> Traceback (most recent call last):
>> File 
>> "/usr/local/lib/python2.7/dist-packages/ducktape/tests/runner_client.py",
>> line 132, in run
>> data = self.run_test()
>> File 
>> "/usr/local/lib/python2.7/dist-packages/ducktape/tests/runner_client.py",
>> line 185, in run_test
>> return self.test_context.function(self.test)
>> File "/usr/local/lib/python2.7/dist-packages/ducktape/mark/_mark.py",
>> line 324, in wrapper
>> return functools.partial(f, *args, **kwargs)(*w_args, **w_kwargs)
>> File "/opt/kafka-dev/tests/kafkatest/tests/streams/
>> streams_broker_compatibility_test.py", line 84, in
>> test_compatible_brokers_eos_disabled
>> processor.start()
>> File "/usr/local/lib/python2.7/dist-packages/ducktape/services/service.py",
>> line 234, in start
>> self.start_node(node)
>> File "/opt/kafka-dev/tests/kafkatest/services/streams.py", line 138, in
>> start_node
>> monitor.wait_until('StreamsTest instance started', timeout_sec=60,
>> err_msg="Never saw message indicating StreamsTest finished startup on " +
>> str(node.account))
>> File 
>> "/usr/local/lib/python2.7/dist-packages/ducktape/cluster/remoteaccount.py",
>> line 668, in wait_until
>> allow_fail=True) == 0, **kwargs)
>> File "/usr/local/lib/python2.7/dist-packages/ducktape/utils/util.py",
>> line 36, in wait_until
>> raise TimeoutError(err_msg)
>> TimeoutError: Never saw message indicating StreamsTest finished startup
>> on ducker@ducker05
>
>
> If I figure out what's wrong I can try to fix other tests.
>
> --
>
> *Sincerely*
> *Andriy Sorokhtey*
> +380681890146
>



-- 

*Sincerely*
*Andriy Sorokhtey*
+380681890146

[jira] [Created] (KAFKA-6909) Message Accumulation in case of no connection to Kafka

[Alexander Stepanov (JIRA)]

Alexander Stepanov created KAFKA-6909:
-

 Summary: Message Accumulation in case of no connection to Kafka
 Key: KAFKA-6909
 URL: https://issues.apache.org/jira/browse/KAFKA-6909
 Project: Kafka
  Issue Type: Improvement
  Components: clients
Reporter: Alexander Stepanov


Simple situation:

0) We have service A which works with Kafka as client

1) No connection to Kafka

2) Service A accumulates messages for sending to Kafka, but still no connection 
to Kafka

3) Then service A is crashed for some reason, for example, OOM or just server 
is dead

4) How to recover accumulated messages?

I think - it will be good to have some general code basement to persist 
service's outgoing messages to Kafka. For example, if service A is crashed, it 
can fetch messages from some local storage on startup and resend it to Kafka.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Resolved] (KAFKA-6908) Update LogDirsCommand's prompt information

[darion yaphet (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-6908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

darion yaphet resolved KAFKA-6908.
--
Resolution: Won't Fix

> Update LogDirsCommand's prompt information
> --
>
> Key: KAFKA-6908
> URL: https://issues.apache.org/jira/browse/KAFKA-6908
> Project: Kafka
>  Issue Type: Improvement
>  Components: admin, tools
>Affects Versions: 1.1.0
>Reporter: darion yaphet
>Priority: Minor
>
> LogDirsCommand command line argument broker List and topic List are marked 
> with RequiredArg . 
> So we should append REQUIRED in the command info . 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (KAFKA-6908) Update LogDirsCommand's prompt information

[darion yaphet (JIRA)]

darion yaphet created KAFKA-6908:


 Summary: Update LogDirsCommand's prompt information
 Key: KAFKA-6908
 URL: https://issues.apache.org/jira/browse/KAFKA-6908
 Project: Kafka
  Issue Type: Improvement
  Components: admin, tools
Affects Versions: 1.1.0
Reporter: darion yaphet


LogDirsCommand command line argument broker List and topic List are marked with 
RequiredArg . So we should append REQUIRED in the command info . 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (KAFKA-6907) Not able to delete topic

[praveen (JIRA)]

praveen created KAFKA-6907:
--

 Summary: Not able to delete topic
 Key: KAFKA-6907
 URL: https://issues.apache.org/jira/browse/KAFKA-6907
 Project: Kafka
  Issue Type: Bug
  Components: config
Affects Versions: 1.1.0
 Environment: Development
Reporter: praveen
 Fix For: 1.1.0


Not able to delte kafka topic

 

./kafka-topics.sh --delete --zookeeper zoo1:2181 --topic test1
Topic test1 is marked for deletion.
Note: This will have no impact if delete.topic.enable is not set to true.

 ./kafka-topics.sh --describe --zookeeper zoo1:2181 --topic test1
Topic:test1 PartitionCount:1 ReplicationFactor:2 Configs: MarkedForDeletion:true
 Topic: test1 Partition: 0 Leader: -1 Replicas: 1,0 Isr: 0

 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: [VOTE] KIP-255: OAuth Authentication via SASL/OAUTHBEARER

[Mickael Maison]

Thanks for the KIP,
+1 (non binding)

On Wed, May 16, 2018 at 2:51 AM, Ron Dagostino  wrote:
> Hi Jun.  I think you are getting at the fact that OAuth 2 is a flexible
> framework that allows different installations to do things differently.  It
> is true that the principal name in Kafka could come from any claim in the
> token.  Most of the time it would come from the 'sub' claim, but it could
> certainly come from another claim, or it could be only indirectly based on
> a claim value (maybe certain text would be trimmed or prefixed/suffixed).
> The point, which I think you are getting at, is that because the framework
> is flexible we need to accommodate that flexibility.  The callback handler
> class defined by the listener.name.sasl_ssl.oauthbearer.sasl.server.
> callback.handler.class configuration value gives us the required
> flexibility.  As an example, I have an implementation that leverages a
> popular open source JOSE library to parse the compact serialization,
> retrieve the public key if it has not yet been retrieved, verify the
> digital signature, and map the 'sub' claim to the OAuthBearerToken's
> principal name (which becomes the SASL authorization ID, which becomes the
> Kafka principal name).  I could just as easily have mapped a different
> claim to the OAuthBearerToken's principal name, manipulated the 'sub' claim
> value in some way, etc.  I write the callback handler code, so I complete
> flexibility to do whatever my OAuth 2 installation requires me to do.
>
> Ron
>
> On Tue, May 15, 2018 at 1:39 PM, Jun Rao  wrote:
>
>> Hi, Ron,
>>
>> Thanks for the reply. I understood your answers to #2 and #3.
>>
>> For #1, will the server map all clients' principal name to the value
>> associated with "sub" claim? How do we support mapping different clients to
>> different principal names?
>>
>> Jun
>>
>> On Mon, May 14, 2018 at 7:02 PM, Ron Dagostino  wrote:
>>
>> > Hi Jun.  Thanks for the +1 vote.
>> >
>> > Regarding the first question about token claims, yes, you have it correct
>> > about translating the OAuth token to a principle name via a JAAS module
>> > option in the default unsecured case.  Specifically, the OAuth SASL
>> Server
>> > implementation is responsible for setting the authorization ID, and it
>> gets
>> > the authorization ID from the OAuthBearerToken's principalName() method.
>> > The listener.name.sasl_ssl.oauthbearer.sasl.server.
>> callback.handler.class
>> > is responsible for handling an instance of OAuthBearerValidatorCallback
>> to
>> > accept a token compact serialization from the client and return an
>> instance
>> > of OAuthBearerToken (assuming the compact serialization validates), and
>> in
>> > the default unsecured case the builtin unsecured validator callback
>> handler
>> > defines the OAuthBearerToken.principalName() method to return the 'sub'
>> > claim value by default (with the actual claim it uses being configurable
>> > via the unsecuredValidatorPrincipalClaimName JAAS module option).  So
>> that
>> > is how we translate from a token to a principal name in the default
>> > unsecured (out-of-the-box) case.
>> >
>> > For production use cases, the implementation associated with
>> > listener.name.sasl_ssl.oauthbearer.sasl.server.callback.handler.class
>> can
>> > do whatever it wants.  As an example, I have written a class that wraps a
>> > com.nimbusds.jwt.SignedJWT instance (see
>> > https://connect2id.com/products/nimbus-jose-jwt/) and presents it as an
>> > OAuthBearerToken:
>> >
>> > public class NimbusSignedJwtOAuthBearerToken implements
>> OAuthBearerToken {
>> > private final SignedJWT signedJwt;
>> > private final String principalName;
>> > private final Set scope;
>> > private final Long startTimeMs;
>> > private final long lifetimeMs;
>> >
>> > /**
>> >  * Constructor
>> >  *
>> >  * @param signedJwt
>> >  *the mandatory signed JWT
>> >  * @param principalClaimName
>> >  *the mandatory claim name identifying the claim from
>> which
>> > the
>> >  *principal name will be extracted. The claim must exist
>> > and must be
>> >  *a String.
>> >  * @param optionalScopeClaimName
>> >  *the optional claim name identifying the claim from
>> which
>> > any scope
>> >  *will be extracted. If specified and the claim exists
>> then
>> > the
>> >  *value must be either a String or a String List.
>> >  * @throws ParseException
>> >  * if the principal claim does not exist or is not a
>> > String; the
>> >  * scope claim is neither a String nor a String List; the
>> > 'exp'
>> >  * claim does not exist or is not a number; the 'iat'
>> claim
>> > exists
>> >  * but is not a number; or the 'nbf' claim exists and is
>> > not a
>> >  * number.
>> >  */
>> > public 

Re: [VOTE] #2 KIP-248: Create New ConfigCommand That Uses The New AdminClient

[Viktor Somogyi]

Hi Colin,

> Doing get-merge-set is buggy, though.  If someone else does get-merge-set at 
> the same time as you, you might overwrite that person's changes, or vice 
> versa.  So I really don't think we should try to do this.  Also, having both 
> an incremental and a full API is useful, and it's just a single boolean at 
> the protocol and API level.

Overwriting somebody's change is currently possible with the
ConfigCommand, as it will do this get-merge-set behavior on the client
side, in the command. From this perspective I think it's not much
different to do this with the admin client. Also I think admins don't
modify the quotas/configs of a client/user/topic/broker often (and
multiple admins would do it even more rarely), so I don't think it is
a big issue. What I think would be useful here but may be out of scope
is to version the changes similarly to leader epochs. So when an admin
updates the configs, it will increment a version number and won't let
other admins to push changes in with lower than that. Instead it would
return an error.

I would be also interested what others think about this?

Cheers,
Viktor


On Sat, May 12, 2018 at 2:29 AM, Colin McCabe  wrote:
> On Wed, May 9, 2018, at 05:41, Viktor Somogyi wrote:
>> Hi Colin,
>>
>> > We are going to need to create a new version of AlterConfigsRequest to add 
>> > the "incremental" boolean.  So while we're doing that, maybe we can change 
>> > the type to NULLABLE_STRING.
>>
>> I was just talking to a colleague yesterday and we came to the
>> conclusion that we should keep the boolean flag only on the client
>> side (as you may have suggested earlier?) and not make part of the
>> protocol as it might lead to a very complicated API on the long term.
>> Also we would keep the server side API simpler. Instead of the
>> protocol change we could just simply have the boolean flag in
>> AlterConfigOptions and the AdminClient should do the get-merge-set
>> logic which corresponds to the behavior of the current ConfigCommand.
>> That way we won't need to change the protocol for now but still have
>> both functionality. What do you think?
>
>  Hi Viktor,
>
> Doing get-merge-set is buggy, though.  If someone else does get-merge-set at 
> the same time as you, you might overwrite that person's changes, or vice 
> versa.  So I really don't think we should try to do this.  Also, having both 
> an incremental and a full API is useful, and it's just a single boolean at 
> the protocol and API level.
>
>>
>> > Hmm.  Not sure I follow.  KIP-133 doesn't use the empty string or 
>> > "" to indicate defaults, does it?
>>
>> No it doesn't. It was just my early idea to indicate "delete" on the
>> protocol level. (We are using  for denoting the default
>> client id or user in zookeeper.) Rajini was referring that we
>> shouldn't expose this to the protocol level but instead denote delete
>> with an empty string.
>>
>> > This comes from DescribeConfigsResponse.
>> > Unless I'm missing something, I think your suggestion to not expose 
>> > "" is already implemented?
>>
>> In some way, yes. Although this one is used in describe and not in
>> alter. For alter I don't think we'd need my early "" idea.
>
> OK.  Thanks for the explanation.  Using an empty string to indicate delete, 
> as Rajini suggested, seems pretty reasonable to me.  null would work as well.
>
>>
>> >> And we use STRING rather than NULLABLE_STRING in describe configs etc. So 
>> >> we
>> >> should probably do the same for quotas."
>> >
>> > I think nearly all responses treat ERROR_MESSAGE as a nullable string.  
>> > CommonFields#ERROR_MESSAGE, which is used by most of them, is a nullable 
>> > string.  It's DescribeConfigsResponse that is the black sheep here.
>> >
>> >  > public static final Field.NullableStr ERROR_MESSAGE = new 
>> > Field.NullableStr("error_message", "Response error message");
>>
>> Looking at DescribeConfigsResponse (and AlterConfigsResponse) they use
>> nullable_string in the code. KIP-133 states otherwise though. So in
>> this case it's not a problem luckily.
>
> Thanks for finding this inconsistency.  I'll change the KIP to reflect what 
> was actually implemented (nullable string for error).
>
> cheers,
> Colin
>
>>
>> > What about writing a small script that just handles setting up SCRAM 
>> > credentials?  It would probably be easier to maintain than the old config 
>> > command.  Otherwise we have to explain when each tool should be used, 
>> > which will be confusing to users.
>>
>> I'd like that, yes :).
>>
>> Cheers,
>> Viktor
>>
>> On Mon, May 7, 2018 at 6:52 PM, Colin McCabe  wrote:
>> > On Fri, May 4, 2018, at 05:49, Viktor Somogyi wrote:
>> >> Hi Colin,
>> >>
>> >> > Rather than breaking compatibility, we should simply add a new 
>> >> > "incremental" boolean to AlterConfigsOptions.  Callers can set this 
>> >> > boolean to true when they want the update to be incremental.  It should 
>> >> > default to false so that old code 

Re: [DISCUSS] KIP-297: Externalizing Secrets for Connect Configurations

[Rajini Sivaram]

Hi Robert,

Thanks for the KIP updates.

The interfaces look suitable for brokers, with some small changes. If we
can adapt the interface to implement the existing DynamicBrokerConfig, then
we are good.

With broker configs:

   1. We don't know what configs are in ZK since we allow custom configs.
   So we would use `ConfigProvider.get()` without specifying keys.
   2. We want to see all changes (i.e. changes under a path). We can deal
   with this internally by ignoring `keys` and subscribing to everything
   3. We have two paths (one for per-broker config and another for default
   config shared by all brokers). All methods should ideally provide path -
   see changes suggested below.
   4. Keys are not independent. We update in batches (e.g keystore +
   password). We want to see batches of changes, not individual changes. We
   retrieve all values from a path when a change is detected. We can do this
   by ignoring values from the callback, but it would be better if the
   callback interface could be changed - see below.


public interface ConfigProvider extends Configurable, Closeable {

*//** KIP-226 will use this*
ConfigData get(ConfigContext ctx, String path);

*// **KIP-226 will never use this, we don't know what keys are in ZK
since we allow custom configs*
ConfigData get(ConfigContext ctx, String path, Set keys);

*// KIP-226 will ignore `key` and subscribe to all changes.*
*// But based on the above method, this should perhaps be:*
*//  subscribe(String path, Set keys,
ConfigurationChangeCallback callback)?*
void subscribe(String key, ConfigurationChangeCallback callback);

 *<== As above, un**subscribe(String path, Set keys)**?*
void unsubscribe(String key);
}

public interface ConfigurationChangeCallback {
*// **For brokers, we want to process all updated keys in a single
callback. P**erhaps this could be: *

*//   onChange(String path, Map values)?*

void onChange(String key, String value);
}

A few other questions (I read your response to Colin, but still didn't get
it. Could be because I am not familiar with the interfaces required for
vaults, sorry):

   1. Are we expecting one provider instance with different contexts
   provided to `ConfigProvider.get()`? If we created a different provider
   instance for each context, we could deal with scheduling reloads in the
   provider implementation?
   2. Couldn't ConfigData  be an interface that just returns a map of
   key-value pairs. Providers that return metadata could extend it to provide
   metadata in a meaningful format instead of Map.
   3. For ZK, we would use ConfigProvider.get() without `keys` to get all
   keys in the path. Do we have two get() methods since some providers need
   keys to be specified and some don't? How do we decide which one to use?

Thanks,

Rajini


On Wed, May 16, 2018 at 2:40 AM, Robert Yokota  wrote:

> Thanks, Ron!  I will take a look.
>
> Regards,
> Robert
>
> On Tue, May 15, 2018 at 5:59 PM, Ron Dagostino  wrote:
>
> > Hi Robert.  Regarding your comment "use the lease duration to schedule a
> > configuration reload in the future", you might be interested in the code
> > that does refresh for OAuth Bearer Tokens in KIP-255; specifically, the
> > class
> > org.apache.kafka.common.security.oauthbearer.internal.expiring.
> > ExpiringCredentialRefreshingLogin.
> > The class performs JAAS logins/relogins based on the expiration time of a
> > retrieved expiring credential.  The implementation of that class is
> > inspired by the code that currently does refresh for Kerberos tickets but
> > is more reusable.  I don't know if you will leverage JAAS for defining
> how
> > to go get a credential (you could since you have to provide credentials
> to
> > authenticate to the remote systems anyway), but regardless, that class
> > should be useful at least in some minimal sense if not more than that.
> See
> > https://github.com/apache/kafka/pull/4994.
> >
> > Ron
> >
> > Ron
> >
> > On Tue, May 15, 2018 at 8:01 PM, Robert Yokota 
> wrote:
> >
> > > Hi Colin,
> > >
> > > Thanks for the feedback!
> > >
> > >
> > > > The KIP says that "Vault is very popular and has been described as
> 'the
> > > current gold standard in secret management and provisioning'."  I think
> > > this might be a bit too much detail -- we don't really need to
> > > > favorites, right? :)
> > >
> > > I've removed this line :)
> > >
> > >
> > > > I think we should make the substitution part of the generic
> > configuration
> > > code, rather than specific to individual ConfigProviders.  We don't
> > really
> > > want it to work differently for Vault vs. KeyWhiz vs.
> > > > AWS secrets, etc. etc.
> > >
> > > Yes, the ConfigProviders merely serve up key-value pairs.  A helper
> class
> > > like ConfigTransformer would perform the variable substitutions if
> > desired.
> > >
> > >
> > > > We should also spell 

Re: [VOTE] KIP-282: Add the listener name to the authentication context

[Mickael Maison]

Thanks to everyone who voted and reviewed the KIP.

The vote has passed with 3 binding votes (Rajini, Jun and Ismael) and
2 non-binding votes (Ted and Manikumar).

Link to the PR: https://github.com/apache/kafka/pull/4829

On Tue, May 15, 2018 at 7:05 PM, Ismael Juma  wrote:
> Thanks for the KIP, +1 (binding).
>
> Ismael
>
> On Wed, Apr 25, 2018 at 1:52 AM Mickael Maison 
> wrote:
>
>> Hi,
>>
>> There has been no objections in the DISCUSS thread so I'd like to
>> start a vote on KIP-282:
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-282%3A+Add+the+listener+name+to+the+authentication+context
>>
>> Thanks
>>

Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs

[Rajini Sivaram]

Colin/Piyush,

I was under the impression that we are keeping both Resource and
ResourceFilter since they are used in different contexts. And hence we
would retain both AclBinding and AclBindingFilter. Is that not the case?
Anyway, an updated KIP would be useful since it has got to a stage where it
is hard to work out exactly where we have got to.

Piyush,

Since there is a lot of interest in this KIP, it would be great to get it
into 2.0.0. It will be good to start the vote by tomorrow if there is
general agreement on an updated KIP today.

Thanks,

Rajini

On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram 
wrote:

> Hi Piyush,
>
> It is possible to configure PrincipalBuilder for SASL (
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 189%3A+Improve+principal+builder+interface+and+add+support+for+SASL). If
> that satisfies your requirements, perhaps we can move wildcarded principals
> out of this KIP and focus on wildcarded resources?
>
>
> On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay 
> wrote:
>
>> Hi Colin,
>>
>> Escaping at this level is making sense to me but let me think more and get
>> back to you.
>>
>> But should we not just get rid of one of AclBinding or AclBindingFilter
>> then? Is there a reason to keep both given that AclBindingFilter and
>> AclBinding look exact copy of each other after this change? This will be a
>> one-time breaking change in APIs marked as "Evolving", but makes sense in
>> the long term? Am I missing something here?
>>
>>
>>
>> Piyush Vijay
>>
>> On Tue, May 15, 2018 at 9:01 AM, Colin McCabe  wrote:
>>
>> > Hi Piyush,
>> >
>> > I think AclBinding should operate the same way as AclBindingFilter.
>> >
>> > So you should be able to do something like this:
>> > > AclBindingFilter filter = new AclBindingFiler(new
>> > ResourceFilter(ResourceType.GROUP, "foo*"))
>> > > AclBinding binding = new AclBinding(new Resource(ResourceType.GROUP,
>> > "foo*"))
>> > > assertTrue(filter.matches(binding));
>> >
>> > Thinking about this more, it's starting to feel really messy to create
>> new
>> > "pattern" constructors for Resource and ResourceFilter.  I don't think
>> > people will be able to figure this out.  Maybe we should just have a
>> > limited compatibility break here, where it is now required to escape
>> weird
>> > consumer group names when creating ACLs for them.
>> >
>> > To future-proof this, we should reserve a bunch of characters at once,
>> > like *, @, $, %, ^, &, +, [, ], etc.  If these characters appear in a
>> > resource name, it should be an error, unless they are escaped with a
>> > backslash.  That way, we can use them in the future.  We should create a
>> > Resource.escapeName function which adds the correct escape characters to
>> > resource names (so it would translate foo* into foo\*, foo+bar into
>> > foo\+bar, etc. etc.
>> >
>> > best,
>> > Colin
>> >
>> >
>> > On Mon, May 14, 2018, at 17:08, Piyush Vijay wrote:
>> > > Colin,
>> > >
>> > > createAcls take a AclBinding, however, instead of AclBindingFilter.
>> What
>> > > are your thoughts here?
>> > >
>> > > public abstract DescribeAclsResult describeAcls(AclBindingFilter
>> > > filter, DescribeAclsOptions options);
>> > >
>> > > public abstract CreateAclsResult createAcls(Collection
>> > > acls, CreateAclsOptions options);
>> > >
>> > > public abstract DeleteAclsResult
>> > > deleteAcls(Collection filters, DeleteAclsOptions
>> > > options);
>> > >
>> > >
>> > > Thanks
>> > >
>> > > Piyush Vijay
>> > >
>> > > On Mon, May 14, 2018 at 9:26 AM, Andy Coates 
>> wrote:
>> > >
>> > > > +1
>> > > >
>> > > > On 11 May 2018 at 17:14, Colin McCabe  wrote:
>> > > >
>> > > > > Hi Andy,
>> > > > >
>> > > > > I see what you mean.  I guess my thought here is that if the
>> fields
>> > are
>> > > > > private, we can change it later if we need to.
>> > > > >
>> > > > > I definitely agree that we should use the scheme you describe for
>> > sending
>> > > > > ACLs over the wire (just the string + version number)
>> > > > >
>> > > > > cheers,
>> > > > > Colin
>> > > > >
>> > > > >
>> > > > > On Fri, May 11, 2018, at 09:39, Andy Coates wrote:
>> > > > > > i think I'm agreeing with you. I was merely suggesting that
>> having
>> > an
>> > > > > > additional field that controls how the current field is
>> > interpreted is
>> > > > > more
>> > > > > > flexible / extensible in the future than using a 'union' style
>> > > > approach,
>> > > > > > where only one of several possible fields should be populated.
>> But
>> > > > it's a
>> > > > > > minor thing.
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > On 10 May 2018 at 09:29, Colin McCabe 
>> wrote:
>> > > > > >
>> > > > > > > Hi Andy,
>> > > > > > >
>> > > > > > > The issue that I was trying to solve here is the Java API.
>> Right
>> > > > now,
>> > > > > > > someone can write "new 

Jenkins build is back to normal : kafka-trunk-jdk10 #108

[Apache Jenkins Server]

See