kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From charly molter <charly.mol...@gmail.com>
Subject Re: [DISCUSS] KIP-209 Connection String Support
Date Mon, 13 Nov 2017 14:31:19 GMT
Hi Clebert,

You mention in rejected alternatives "builders" but you don't give a
justification.

Could you explain why a utility method or class doesn't work out?

There's already 5 constructors in KafkaConsumer and I'm afraid this change
would just make the API more complex.

Thanks,
Charly

On Mon, Nov 13, 2017 at 2:13 PM, Clebert Suconic <clebert.suconic@gmail.com>
wrote:

> I was waiting for more input after my last update.. what should be done
> now?
>
>
>
> On Sat, Nov 4, 2017 at 2:19 PM, Clebert Suconic
> <clebert.suconic@gmail.com> wrote:
> > I have updated KIP-209.
> >
> >
> > Determined the \", \\ and \; meanings
> >
> > Also introduced the possibility of using $ to translate as system
> properties.
> >
> > I"m not using an BNF formal language here, as I don't think it's
> > needed.. it seems pretty obvious what it would be accomplished here.
> >
> > On Fri, Oct 27, 2017 at 2:05 PM, Colin McCabe <cmccabe@apache.org>
> wrote:
> >> On Tue, Oct 24, 2017, at 22:51, Michael André Pearce wrote:
> >>> Fair enough on URL encoding but as mentioned it is important to be able
> >>> to escape, I agree with backslash option.
> >>>
> >>> I would still like some form of prefix to the string to denote it is
> for
> >>> kafka.
> >>
> >> I don't think a prefix is necessary here.  URLs have a prefix because
> >> there are multiple services which you could access (HTTP vs HTTPS, etc.)
> >>  If you are passing a string to Kafka, the string is for Kafka, not for
> >> something else, so the issue doesn't exist.
> >>
> >> best,
> >> Colin
> >>
> >>
> >>>
> >>>  E.g. kafka:: (if semicolon separators)
> >>>
> >>> Sent from my iPad
> >>>
> >>> > On 24 Oct 2017, at 17:27, Colin McCabe <cmccabe@apache.org> wrote:
> >>> >
> >>> > Hi Clebert,
> >>> >
> >>> > As some other people mentioned, a comma is probably not a great
> choice
> >>> > for the entry separator.  We have a lot of configuration values that
> >>> > already include commas.  How about using a semicolon instead?
> >>> >
> >>> > You also need an escaping system in case someone needs a semicolon
> (or
> >>> > whatever) that is part of a configuration key or configuration value.
> >>> > How about a simple backslash?  And then if you want a literal
> backslash,
> >>> > you put in two backslashes.
> >>> >
> >>> >> On Thu, Oct 19, 2017, at 18:10, Michael André Pearce wrote:
> >>> >> Just another point to why I’d propose the below change to the
string
> >>> >> format I propose , is an ability to encode the strings easily.
> >>> >>
> >>> >> We should note that it’s quite typical for serializers to user
a
> >>> >> schematic registry where one of their properties they will need
to
> set
> >>> >> would be in some form like:
> >>> >>
> >>> >> schema.registry.url=http://schema1:80,schema2:80/api
> >>> >>
> >>> >> So being able to safely encode this is important.
> >>> >>
> >>> >> Sent from my iPhone
> >>> >>
> >>> >>> On 20 Oct 2017, at 01:47, Michael André Pearce <
> michael.andre.pearce@me.com> wrote:
> >>> >>>
> >>> >>> Hi Clebert
> >>> >>>
> >>> >>> Great kip!
> >>> >>>
> >>> >>> Instead of ‘;’ to separate the host sections with the params
> section could it be a ‘?’
> >>> >>>
> >>> >>> And like wise ‘,’ param separator could this be ‘&’
(keep the ‘,’
> for host separator just makes easier to distinguish)
> >>> >>>
> >>> >>> Also this was it makes it easier to encode params etc as can
just
> re use url encoders.
> >>> >
> >>> > Please, no.  URL encoders will mangle a lot of things horribly (like
> >>> > less than signs, greater than signs, etc.)  We should not make this
a
> >>> > URL or pseudo-URL (see the discussion above).  We should make it
> clear
> >>> > that this is not a URL.
> >>> >
> >>> >> Invalid conversions would throw InvalidArgumentException (with
a
> description of the invalid conversion)
> >>> >> Invalid parameters would throw InvalidArgumentException (with the
> name of the invalid parameter).
> >>> >
> >>> > This will cause a lot of compatibility problems, right?  If I switch
> >>> > back and forth between two Kafka versions, they will support slightly
> >>> > different sets of configuration parameters.  It seems saner to simply
> >>> > ignore configuration parameters that we don't understand, like we do
> >>> > now.
> >>> >
> >>> > best,
> >>> > Colin
> >>> >
> >>> >
> >>> >>>
> >>> >>> Also as like many systems it typical to note what the connection
> string is for with a prefix eg ‘kafka://‘
> >>> >>>
> >>> >>> Just makes it obvious when an app has a list of connection
strings
> in their runtime properties which is for which technology.
> >>> >>>
> >>> >>> Eg example connection string would be:
> >>> >>>
> >>> >>> kafka://host1:port1,host2:port2?param1=value1&parm2=value2
> >>> >>>
> >>> >>> Cheers
> >>> >>> Mike
> >>> >>>
> >>> >>> Sent from my iPhone
> >>> >>>
> >>> >>>> On 19 Oct 2017, at 19:29, Clebert Suconic <
> clebert.suconic@gmail.com> wrote:
> >>> >>>>
> >>> >>>> Do I have to do anything here?
> >>> >>>>
> >>> >>>> I wonder how long I need to wait before proposing the vote.
> >>> >>>>
> >>> >>>> On Tue, Oct 17, 2017 at 1:17 PM, Clebert Suconic
> >>> >>>> <clebert.suconic@gmail.com> wrote:
> >>> >>>>> I had these updates in already... you just changed
the names at
> the
> >>> >>>>> string.. but it was pretty much the same thing I think...
I had
> taken
> >>> >>>>> you suggestions though.
> >>> >>>>>
> >>> >>>>>
> >>> >>>>> The Exceptions.. these would be implementation details...
all I
> wanted
> >>> >>>>> to make sure is that users would get the name of the
invalid
> parameter
> >>> >>>>> as part of a string on a message.
> >>> >>>>>
> >>> >>>>> On Tue, Oct 17, 2017 at 3:15 AM, Satish Duggana
> >>> >>>>> <satish.duggana@gmail.com> wrote:
> >>> >>>>>> You may need to update KIP with the details discussed
in this
> thread in
> >>> >>>>>> proposed changes section.
> >>> >>>>>>
> >>> >>>>>>>> My proposed format for the connection string
would be:
> >>> >>>>>>>> IP1:host1,IP2:host2,...IPN:hostn;parameterName=value1;
> parameterName2=value2;...
> >>> >>>>>> parameterNameN=valueN
> >>> >>>>>> Format should be
> >>> >>>>>> host1:port1,host2:port2,…host:portn;param-name1=param-val1,..
> >>> >>>>>>
> >>> >>>>>>>> Invalid conversions would throw InvalidArgumentException
> (with a
> >>> >>>>>> description of the invalid conversion)
> >>> >>>>>>>> Invalid parameters would throw InvalidArgumentException
(with
> the name of
> >>> >>>>>> the invalid parameter).
> >>> >>>>>>
> >>> >>>>>> Should throw IllegalArgumentException with respective
message.
> >>> >>>>>>
> >>> >>>>>> Thanks,
> >>> >>>>>> Satish.
> >>> >>>>>>
> >>> >>>>>> On Tue, Oct 17, 2017 at 4:46 AM, Clebert Suconic
<
> clebert.suconic@gmail.com>
> >>> >>>>>> wrote:
> >>> >>>>>>
> >>> >>>>>>> That works.
> >>> >>>>>>>
> >>> >>>>>>>> On Mon, Oct 16, 2017 at 6:59 PM Ted Yu
<yuzhihong@gmail.com>
> wrote:
> >>> >>>>>>>>
> >>> >>>>>>>> Can't you use IllegalArgumentException
?
> >>> >>>>>>>>
> >>> >>>>>>>> Some example in current code base:
> >>> >>>>>>>>
> >>> >>>>>>>> clients/src/main/java/org/apache/kafka/clients/Metadata.java:
> >>> >>>>>>>> throw new IllegalArgumentException("Max
time to wait for
> metadata
> >>> >>>>>>> updates
> >>> >>>>>>>> should not be < 0 milliseconds");
> >>> >>>>>>>>
> >>> >>>>>>>> On Mon, Oct 16, 2017 at 3:06 PM, Clebert
Suconic <
> >>> >>>>>>>> clebert.suconic@gmail.com>
> >>> >>>>>>>> wrote:
> >>> >>>>>>>>
> >>> >>>>>>>>> I updated the wiki with the list on
the proposed arguments.
> >>> >>>>>>>>>
> >>> >>>>>>>>> I also changed it to include a new
Exception class that
> would be named
> >>> >>>>>>>>> InvalidParameterException (since I
couldn't find an existing
> Exception
> >>> >>>>>>>>> class that I could reuse into this).
(I could review the
> name or the
> >>> >>>>>>>>> exception of course.. just my current
proposal)
> >>> >>>>>>>>>
> >>> >>>>>>>>>> On Mon, Oct 16, 2017 at 5:55 PM,
Jakub Scholz <
> jakub@scholz.cz> wrote:
> >>> >>>>>>>>>> Hi Clebert,
> >>> >>>>>>>>>>
> >>> >>>>>>>>>> I think it would be good if this
could cover not only
> KafkaConsumer
> >>> >>>>>>> and
> >>> >>>>>>>>>> KafkaProducer but also the AdminClient.
So that all three
> can be
> >>> >>>>>>>>> configured
> >>> >>>>>>>>>> the same way.
> >>> >>>>>>>>>>
> >>> >>>>>>>>>> The bootstrap servers are a list
- you can provide multiple
> bootstrap
> >>> >>>>>>>>>> servers. Maybe you add an example
of how that will be
> configured. I
> >>> >>>>>>>>> assume
> >>> >>>>>>>>>> it will be
> >>> >>>>>>>>>> "host:port,host2:port2;parameterName=value1;
> parameterName2=value2"
> >>> >>>>>>> but
> >>> >>>>>>>>> it
> >>> >>>>>>>>>> would be great to have it documented.
> >>> >>>>>>>>>>
> >>> >>>>>>>>>> Thanks & Regards
> >>> >>>>>>>>>> Jakub
> >>> >>>>>>>>>>
> >>> >>>>>>>>>> On Mon, Oct 16, 2017 at 11:30 PM,
Clebert Suconic <
> >>> >>>>>>>>> clebert.suconic@gmail.com
> >>> >>>>>>>>>>> wrote:
> >>> >>>>>>>>>>
> >>> >>>>>>>>>>> I would like to start a discussion
about KIP-209
> >>> >>>>>>>>>>> (https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>> >>>>>>>>>>> 209+-+Connection+String+Support)
> >>> >>>>>>>>>>>
> >>> >>>>>>>>>>> This is an extension of my
previous thread:
> >>> >>>>>>>>>>> http://mail-archives.apache.org/mod_mbox/kafka-dev/201710.
> >>> >>>>>>>>>>> mbox/%3cCAKF+bsoFbN13D-u20tUsP6G+aHX4BUNk=S8M4KyJxAt_
> >>> >>>>>>>>>>> Oyvtqw@mail.gmail.com%3e
> >>> >>>>>>>>>>>
> >>> >>>>>>>>>>> this could make the bootstrap
of a consumer or producer
> similar to
> >>> >>>>>>>>>>> what users are already used
when connecting into other
> systems,
> >>> >>>>>>> being
> >>> >>>>>>>>>>> a simple addition to Producer
and Consumer, without
> breaking any
> >>> >>>>>>>>>>> previous client usage.
> >>> >>>>>>>>>>>
> >>> >>>>>>>>>>>
> >>> >>>>>>>>>>> --
> >>> >>>>>>>>>>> Clebert Suconic
> >>> >>>>>>>>>>>
> >>> >>>>>>>>>
> >>> >>>>>>>>>
> >>> >>>>>>>>>
> >>> >>>>>>>>> --
> >>> >>>>>>>>> Clebert Suconic
> >>> >>>>>>>>>
> >>> >>>>>>>>
> >>> >>>>>>> --
> >>> >>>>>>> Clebert Suconic
> >>> >>>>>>>
> >>> >>>>>
> >>> >>>>>
> >>> >>>>>
> >>> >>>>> --
> >>> >>>>> Clebert Suconic
> >>> >>>>
> >>> >>>>
> >>> >>>>
> >>> >>>> --
> >>> >>>> Clebert Suconic
> >
> >
> >
> > --
> > Clebert Suconic
>
>
>
> --
> Clebert Suconic
>



-- 
Charly Molter

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message