Hi, Thank you all for the positive feedbacks so far.
Timo discussed with me offline about the format properties, and we think that this FLIP should also cover formats, otherwise we need to touch the properties again in the future. The bad experience of format properties is that `format.derive-schema=true` should be set explicitly, otherewise users have to define schema for formats again in the properties. So I added a new section in the doc[1] to improve this, to make `derive-schema=true` as the default behavior and make all formats support it. Please take a look at the changes when you are free, and I will start a vote soon if no objections. Best, Jark [1]: https://docs.google.com/document/d/14MlXFB45NUmUtesFMFZFjFhd4pqmYamxbfNwUPrfdL4/edit# On Thu, 14 Nov 2019 at 21:20, Kurt Young <ykt...@gmail.com> wrote: > +1 to the general idea of this FLIP. > > Regarding to connector properties, IIUC it would be divided into 2 parts: > 1. connector.key1 = value1, these are interpreted by Flink framework and > do whatever needed for such property. > 2. connector.properties.xxx.key = value. There items are prefixed with > "connector.properties" and will passed into maybe connector's client > directly. > > Just want to confirm this and I'm +1 to this approach. > > Best, > Kurt > > > On Thu, Nov 14, 2019 at 5:36 PM Dawid Wysakowicz <dwysakow...@apache.org> > wrote: > > > Yes, personally I would also go with the external system's format for > > the exact reason you mentioned. > > > > Best, > > > > Dawid > > > > On 14/11/2019 10:32, Jark Wu wrote: > > > Hi Dawid, > > > > > > The `connector.properties.zookeeper.connect` sounds good to me. > > > > > > Just to clarify, in this way, we can support arbitrary properties and > we > > > can document the important ones. > > > And regarding to the value format, shall we use the external system's > > > format or use Flink's format, e.g. the list separator? > > > I would prefer the external system's format, so that keys and values > are > > > all forwarded directly. > > > > > > Best, > > > Jark > > > > > > > > > On Thu, 14 Nov 2019 at 17:02, Dawid Wysakowicz <dwysakow...@apache.org > > > > > wrote: > > > > > >> Hi Jark, > > >> > > >> I see the not readable point and that is also my concern about my > > >> suggestion with all the properties under a single key as the list > might > > get > > >> quite long. > > >> > > >> What do you think about modifying your suggestion a bit and at least > add > > >> the `properties` prefix? So that we can distinguish the Kafka defined > > >> properties and the Flink's ones? > > >> > > >> What do you think about properties like: > > >> > > >> connector.properties.zookeeper.connect: .... > > >> > > >> connector.properties.bootstrap-servers: .... > > >> > > >> > > >> I think that's also what Konstantin was suggesting. I might be wrong > on > > >> that though :) > > >> > > >> Best, > > >> > > >> Dawid > > >> > > >> > > >> On 14/11/2019 04:52, Jark Wu wrote: > > >> > > >> Hi Dawid, Konstantin, > > >> > > >> This is an interesting topic. I will list the prons and cons of these > 2 > > >> options. > > >> > > >> 1) under a single key: `connector.properties` > > >> Cons: > > >> - Users can pass in arbitrary properties defined in kafka docs > > >> Prons: > > >> - The property value is collapsed together and will be long and not > > >> readable. > > >> - Some values may need to be escaped because of the separator. > > >> > > >> 2) flattened keys: `connector.zookeeper.connect`, > `connector.bootstrap. > > >> servers` > > >> Cons: > > >> - Better validation and helpful exception message if the value is > wrong. > > >> - Better documentation about which properties are required and what it > > >> means. > > >> - Easy to write. > > >> Prons: > > >> - If users want to pass in other kafka properties, need to support > them > > and > > >> wait until the next major release. > > >> > > >> From my point of view, we can support both #1 and #2, but #2 will > > override > > >> the values defined in #1. > > >> So that, users can define the important properties in falttented keys, > > and > > >> can define unsupported properties in `connector.properties`. > > >> > > >> What do you think? > > >> > > >> Best, > > >> Jark > > >> > > >> > > >> > > >> On Thu, 14 Nov 2019 at 01:57, Konstantin Knauf < > > konstan...@ververica.com> <konstan...@ververica.com> > > >> wrote: > > >> > > >> > > >> Hi Dawid, Hi Jark, > > >> > > >> in my experience it is very important to be able to forward arbitrary > > >> properties to the underlying KafkaClient. This seems to be possible in > > both > > >> cases. I am leaning towards Jark's original suggestion. Flink's > > >> documentation would only need to state that it forwards everything > under > > >> "connector.properties" to the KafkaClient. > > >> > > >> If the external system has a different formatting scheme, we could > still > > >> document this on a per-connector basis (use Flink's formatting scheme > > and > > >> document how it is translated to the external system's native > > properties) > > >> or if possible use the external system's scheme under > > >> "connector.properties". I don't really know the limitations on our > side > > >> w.r.t this. > > >> > > >> Cheers, > > >> > > >> Konstantin > > >> > > >> > > >> > > >> On Wed, Nov 13, 2019 at 3:36 PM Dawid Wysakowicz < > > dwysakow...@apache.org> <dwysakow...@apache.org> > > >> wrote: > > >> > > >> > > >> Hi Jark, > > >> > > >> Majority of the changes make sense to me. I think they will be helpful > > >> > > >> for > > >> > > >> the users. I have a slight concern about the > > >> > > >> kafka's connector.properties property :) . I wonder if we should have > > >> them under a single key: > > >> > > >> connector.properties: > > >> > > >> > > `zookeeper.connect`:`localhost:2181`;`bootstrap.servers`:`localhost:9092` > > >> > > >> As far as I understand it, these are mostly the properties that are > > >> forwarded straight to the underlying KafkaClient. Such properties are > > >> mostly defined and documented by the kafka itself rather than flink. > > They > > >> might also follow a different formatting scheme than we have for our > > >> properties. Moreover how do we decide which properties are put into > the > > >> Properties object and which are not? I would be happy to hear what > > others > > >> think about that part, as I am not convinced myself about that part. > > >> > > >> Best, > > >> > > >> Dawid > > >> On 13/11/2019 13:22, Jark Wu wrote: > > >> > > >> Hi everyone, > > >> > > >> Connector properties is a very basic component which is used to > > >> > > >> construct a > > >> > > >> connector table via YAML, descriptor API, or DDL. It is also the > > >> serialization representation when persisting into catalog. However, we > > >> > > >> have > > >> > > >> encountered some problems when using connector properties, especially > in > > >> DDL. This FLIP is aiming to solve following two issues: > > >> > > >> - FLINK-14645: Data types defined in DDL loses precision and > nullability > > >> when converting to properties. > > >> - FLINK-14649: Some properties structure is hard to define in DDL, > e.g. > > >> List and Map structure. > > >> > > >> This FLIP proposes new properties to represent data types in schema > and > > >> > > >> to > > >> > > >> flatten properties in existing connectors. > > >> > > >> FLIP-86: > > >> > > >> > > > https://docs.google.com/document/d/14MlXFB45NUmUtesFMFZFjFhd4pqmYamxbfNwUPrfdL4/edit# > > >> > > >> Thanks for any feedback! > > >> > > >> Best, > > >> Jark > > >> > > >> > > >> > > >> > > >> -- > > >> > > >> Konstantin Knauf | Solutions Architect > > >> > > >> +49 160 91394525 > > >> > > >> > > >> Follow us @VervericaData Ververica <https://www.ververica.com/> < > > https://www.ververica.com/> > > >> > > >> > > >> -- > > >> > > >> Join Flink Forward <https://flink-forward.org/> < > > https://flink-forward.org/> - The Apache Flink > > >> Conference > > >> > > >> Stream Processing | Event Driven | Real Time > > >> > > >> -- > > >> > > >> Ververica GmbH | Invalidenstrasse 115, 10115 Berlin, Germany > > >> > > >> -- > > >> Ververica GmbH > > >> Registered at Amtsgericht Charlottenburg: HRB 158244 B > > >> Managing Directors: Timothy Alexander Steinert, Yip Park Tung Jason, > Ji > > >> (Tony) Cheng > > >> > > >> > > >> > > > > >
