It would work out. it's just a matter to find what's easier for users... a builder would work equally.
I listed the rejected alternative in case the constructor wasn't accepted. I don't think it adds complexity. it's the same quite frankly. refactoring a method out to a builder of factory is a simple refactoring. the question really is: do you want the connection string support? if it lives as a constructor overload or a builder utility provided by the client library.. it doesn't really matter IMO. On Mon, Nov 13, 2017 at 9:31 AM, charly molter <charly.mol...@gmail.com> wrote: > 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.suco...@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.suco...@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 <cmcc...@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 <cmcc...@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.pea...@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.suco...@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.suco...@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.dugg...@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.suco...@gmail.com> >> >>> >>>>>> wrote: >> >>> >>>>>> >> >>> >>>>>>> That works. >> >>> >>>>>>> >> >>> >>>>>>>> On Mon, Oct 16, 2017 at 6:59 PM Ted Yu <yuzhih...@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.suco...@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 < >> ja...@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.suco...@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_ >> >>> >>>>>>>>>>> oyv...@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 -- Clebert Suconic
