Hi Tom,
I added Configuration Properties sections that document the properties and
a couple of examples.

On Tue, Jun 28, 2022 at 10:57 AM Tom Bentley <tbent...@redhat.com> wrote:

> Hi Omnia,
>
> Please could you add to the KIP the documentation that forward.admin.class
> config will have, and include that the signature of the class named by
> forward.admin.class must have a constructor that takes a `Map<String,
> Object>`?
>
> Many thanks,
>
> Tom
>
> On Wed, 22 Jun 2022 at 11:30, <o.g.h.ibra...@gmail.com> wrote:
>
> > Hi Tom
> > My initial thought was that the constructor with config and delegator
> > would be for testing while the one with config is the one that will be
> used
> > by MM2.
> > I can removed one of them and keep only one.
> >
> > Sent from my iPhone
> >
> > > On 22 Jun 2022, at 08:15, Tom Bentley <tbent...@redhat.com> wrote:
> > >
> > > Hi Omnia,
> > >
> > > Thanks for those answers. I'm a bit confused by the changes you've made
> > for
> > > 1 though. It's MM2 that's going to instantiate the class named in
> > > forwarding.admin.class, so it's MM2 that needs to know the constructor
> > > signature. The easiest way of doing this is to specify the signature as
> > > part of the contract for forwarding.admin.class. But ForwardingAdmin
> now
> > > has two constructors, which is confusing for someone who wants to
> > subclass
> > > it. Surely only one is needed? It would also be a good idea to show the
> > > documentation that forward.admin.class will have.
> > >
> > > Kind regards,
> > >
> > > Tom
> > >
> > >> On Tue, 21 Jun 2022 at 17:06, Omnia Ibrahim <o.g.h.ibra...@gmail.com>
> > wrote:
> > >>
> > >> Hi Tom
> > >>
> > >>> 1. I assume there's an undocumented requirement in the KIP that
> > whatever
> > >>> class is named for forwarding.admin.class it has a public single
> > argument
> > >>> constructor that takes an Admin instance?
> > >>>
> > >> you are right, I updated the KIP to reflect the missing one.
> > >> 3. What if the implementation needs to distinguish between creating
> > ACLs on
> > >> the source cluster and creating them on the destination cluster? E.g.
> > the
> > >> one should be done one way, but the other using a different mechanism?
> > >> Users will need to define 2 implementations, one for source and
> another
> > for
> > >> target and configure each using
> <cluster_alias>.forwarding.admin.class.
> > for
> > >> example
> > >> - target.forwarding.admin.class = TargetAdmin
> > >> - source.forwarding.admin.class = SourceAdmin
> > >>
> > >>> On Tue, Jun 21, 2022 at 2:14 PM Tom Bentley <tbent...@redhat.com>
> > wrote:
> > >>>
> > >>> Hi Omnia,
> > >>>
> > >>> Thanks for the KIP! I'm sorry for the delay in this response. I have
> a
> > >> few
> > >>> questions:
> > >>>
> > >>> 1. I assume there's an undocumented requirement in the KIP that
> > whatever
> > >>> class is named for forwarding.admin.class it has a public single
> > argument
> > >>> constructor that takes an Admin instance?
> > >>> 2. If 1 is correct then what about an implementation that requires
> > extra
> > >>> configuration, e.g. for whatever infra-as-code API it needs to use
> > >> (instead
> > >>> of using an Admin instance directly) how does it learn about that
> > >> non-Kafka
> > >>> config when it's only receiving an Admin instance?
> > >>> 3. What if the implementation needs to distinguish between creating
> > ACLs
> > >> on
> > >>> the source cluster and creating them on the destination cluster? E.g.
> > the
> > >>> one should be done one way, but the other using a different
> mechanism?
> > >>>
> > >>> Kind regards,
> > >>>
> > >>> Tom
> > >>>
> > >>>
> > >>> On Mon, 20 Jun 2022 at 17:13, Omnia Ibrahim <o.g.h.ibra...@gmail.com
> >
> > >>> wrote:
> > >>>
> > >>>> Hi Chris, sorry for the late reply.
> > >>>> ..Hi,
> > >>>>
> > >>>>> 1. I might be missing something, but can you give a concrete Java
> > >>> example
> > >>>>> of how the proposed ForwardingAdmin class is more convenient than
> > >>>>> subclassing the KafkaAdminClient class? AFAICT the two would be
> > >>> virtually
> > >>>>> identical.
> > >>>>
> > >>>> I might be misunderstanding exactly how that class will be used;
> > >>>>> I'm envisioning it as the pluggable class that users will implement
> > >> for
> > >>>>> custom administration logic and specify as the value for the
> > >>>>> "forwarding.admin.class" (or "<cluster>.forwarding.admin.class")
> > >>>> property,
> > >>>>> and that it will be instantiated with a KafkaAdminClient instance
> > >> that
> > >>>> can
> > >>>>> be used to get the same logic that MM2 provides today. In the case
> > >> you
> > >>>>> mentioned (KafkaAdminClient for read/describe, custom Admin for
> > >>>>> create/update), I'd imagine one could override the createTopics,
> > >>>>> deleteTopics, createAcls, deleteAcls (maybe?), alterConfigs
> (maybe?),
> > >>>> etc.
> > >>>>> methods, and then leave other methods such as listTopics,
> > >>> describeTopics,
> > >>>>> describeCluster, etc. as they are.
> > >>>>>
> > >>>> The Forwarding decorator is one alternative for inheritance so you
> are
> > >>>> right to say that they look identical. However, I would like to
> point
> > >> out
> > >>>> few points
> > >>>> 1. that Kafka codebase has few wrappers or decorators around
> > >> AdminClient
> > >>>> instead of inheritance so using decorator over inheritance isn't new
> > >>>> proposal for example
> > >>>>
> > >>>>   - -
> > >>> `org.apache.kafka.streams.processor.internals.InternalTopicManager`
> > >>>>   which has AdminClient as parameter instead of inheatance
> > >>>>   - - `org.apache.kafka.connect.util.SharedTopicAdmin` and
> > >>>>   `org.apache.kafka.connect.util.TopicAdmin` don't inherit
> > >>>> KafkaAdminClient
> > >>>>   instead initialize KafkaAdminClient.
> > >>>>
> > >>>> 2. Using ForwardingAdmin will make it easier to test which methods
> use
> > >>>> KafkaAdminClient and which don't this make the test for any
> customized
> > >>>> implementation easier. We can't have this with inheatcancs
> > >>>> 3. Inhearting KafkaAdminClient has the following limitation
> > >>>>       a. KafkaAdminClient doesn't have a public default constructor,
> > >> so
> > >>>> isn't gonna be easy to have contractor that initialize both
> > >>>> KafkaAdminClient (to be used for read/describe/list) and customized
> > >>>> fedrated client (to be used for create/update). (Note I don't want
> to
> > >>> touch
> > >>>> anything in KafkaAdminClient code base to change that)
> > >>>>      b. the only way to initialize instance is by
> > >> using`createInternal`
> > >>>> which is static and can't be overridden to include creating
> customized
> > >>>> fedrated client. This is the method used by `Admin.create` to
> > >>>> initialize `KafkaAdminClient`in
> > >>>> most MM2 and Kafka codebase.
> > >>>>
> > >>>> 3. KIP-158 deals with the topics that source connectors write to,
> not
> > >> the
> > >>>>> internal topics used by the Connect framework. IIUC this includes
> the
> > >>>>> topics that MM2 mirrors. I think it's still fine if we want to
> leave
> > >>>>> support for this out and say that this KIP only addresses code that
> > >>> lives
> > >>>>> inside the connect/mirror (and possibly connect/mirror-client?)
> > >>> modules,
> > >>>> I
> > >>>>> just want to make sure that whatever behavior we settle on is
> > >> specified
> > >>>>> clearly in the KIP and user-facing documentation.
> > >>>>
> > >>>> MM2 deals with a large number of topics, creating the topic if not
> > >>>> existing, adding new partitions if the number of partitions
> increases
> > >> and
> > >>>> syncing configs and ACLs, so while KIP-158 can stop the source
> connect
> > >>> from
> > >>>> creating a topic, this wouldn't be a great solution for MM2 when it
> > >> runs
> > >>> on
> > >>>> a large scale as this will add too much operation headache on the
> > teams
> > >>>> that run MM2 to create/update large number of topics manually. Also,
> > it
> > >>>> prevents teams from enabling MM2 to sync configs and ACLs as this
> > still
> > >>>> will be at odds with the federated solution.
> > >>>>
> > >>>> 4. That's fine 👍 Just out of curiosity, is the motivation for this
> > >>>>> decision to simplify the implementation? I can imagine it'd be
> easier
> > >>> to
> > >>>>> modify the MM2 codebase exclusively and not have to worry about
> > >>> touching
> > >>>>> the Connect framework as well given the possibility for unintended
> > >>>>> consequences for other connectors with the latter.
> > >>>>
> > >>>> The motivation of the KIP is to have the option to let MM2 create
> > >>> topics,
> > >>>> add partitions, and sync configs and ACLs without being at odds with
> > >>>> federated or capacity solutions when it runs on a large scale
> instead
> > >> of
> > >>>> just turning the admin client off.
> > >>>>
> > >>>>> Wondering if there's a
> > >>>>> distinction on the feature front as well that makes MM2 internal
> > >> topics
> > >>>>> different from Connect internal topics.
> > >>>>
> > >>>>
> > >>>> The only diff here is the MM2 call TopicManager directly to create
> > >> these
> > >>>> topics.
> > >>>>
> > >>>>
> > >>>> Thanks
> > >>>> Omnia
> > >>>>
> > >>>>
> > >>>> On Tue, Jun 7, 2022 at 5:05 AM Chris Egerton <
> fearthecel...@gmail.com
> > >
> > >>>> wrote:
> > >>>>
> > >>>>> Hi Omnia,
> > >>>>>
> > >>>>> 1. I might be missing something, but can you give a concrete Java
> > >>> example
> > >>>>> of how the proposed ForwardingAdmin class is more convenient than
> > >>>>> subclassing the KafkaAdminClient class? AFAICT the two would be
> > >>> virtually
> > >>>>> identical. I might be misunderstanding exactly how that class will
> be
> > >>>> used;
> > >>>>> I'm envisioning it as the pluggable class that users will implement
> > >> for
> > >>>>> custom administration logic and specify as the value for the
> > >>>>> "forwarding.admin.class" (or "<cluster>.forwarding.admin.class")
> > >>>> property,
> > >>>>> and that it will be instantiated with a KafkaAdminClient instance
> > >> that
> > >>>> can
> > >>>>> be used to get the same logic that MM2 provides today. In the case
> > >> you
> > >>>>> mentioned (KafkaAdminClient for read/describe, custom Admin for
> > >>>>> create/update), I'd imagine one could override the createTopics,
> > >>>>> deleteTopics, createAcls, deleteAcls (maybe?), alterConfigs
> (maybe?),
> > >>>> etc.
> > >>>>> methods, and then leave other methods such as listTopics,
> > >>> describeTopics,
> > >>>>> describeCluster, etc. as they are.
> > >>>>>
> > >>>>> 3. KIP-158 deals with the topics that source connectors write to,
> not
> > >>> the
> > >>>>> internal topics used by the Connect framework. IIUC this includes
> the
> > >>>>> topics that MM2 mirrors. I think it's still fine if we want to
> leave
> > >>>>> support for this out and say that this KIP only addresses code that
> > >>> lives
> > >>>>> inside the connect/mirror (and possibly connect/mirror-client?)
> > >>> modules,
> > >>>> I
> > >>>>> just want to make sure that whatever behavior we settle on is
> > >> specified
> > >>>>> clearly in the KIP and user-facing documentation.
> > >>>>>
> > >>>>> 4. That's fine 👍 Just out of curiosity, is the motivation for this
> > >>>>> decision to simplify the implementation? I can imagine it'd be
> easier
> > >>> to
> > >>>>> modify the MM2 codebase exclusively and not have to worry about
> > >>> touching
> > >>>>> the Connect framework as well given the possibility for unintended
> > >>>>> consequences for other connectors with the latter. Wondering if
> > >>> there's a
> > >>>>> distinction on the feature front as well that makes MM2 internal
> > >> topics
> > >>>>> different from Connect internal topics.
> > >>>>>
> > >>>>> Cheers,
> > >>>>>
> > >>>>> Chris
> > >>>>>
> > >>>>> On Mon, Jun 6, 2022 at 6:36 AM Omnia Ibrahim <
> > >> o.g.h.ibra...@gmail.com>
> > >>>>> wrote:
> > >>>>>
> > >>>>>> Hi Chris, Thanks for having the time to look into this.
> > >>>>>>
> > >>>>>> 1. Is the introduction of the new "ForwardingAdmin" class
> > >> necessary,
> > >>> or
> > >>>>> can
> > >>>>>>> the same behavior can be achieved by subclassing the existing
> > >>>>>>> KafkaAdminClient class?
> > >>>>>>
> > >>>>>> forwarding decorators give more flexibility than the inheritance,
> > >> in
> > >>>> this
> > >>>>>> case, ForwardingAdmin gives the ability to use the default
> > >>>>> KafkaAdminClient
> > >>>>>> for reading/describing resources and at the same time configure
> > >>> another
> > >>>>>> client to connect to the federated solution to create and update
> > >>>>> resources.
> > >>>>>> Using forwarding seems cleaner and more flexible for this use case
> > >>> than
> > >>>>>> inheritance.
> > >>>>>>
> > >>>>>> 2. Would it be just as accurate to name the new Mirror Maker 2
> > >>> property
> > >>>>>>> "admin.class" instead of "forwarding.admin.class"? I think
> > >> brevity
> > >>>> may
> > >>>>>> work
> > >>>>>>> in our favor here
> > >>>>>>>
> > >>>>>> I don't mind renaming it to "admin.class" if this is better.
> > >>>>>>
> > >>>>>> 3. Would the admin class specified by the user also take effect
> for
> > >>>>> KIP-158
> > >>>>>>> [1] style automatic topic creation? (Forgive me if this isn't
> > >>>>> applicable
> > >>>>>>> for Mirror Maker 2; I'm asking solely based on the knowledge that
> > >>> MM2
> > >>>>> can
> > >>>>>>> be run as a source connector and has its own source task class
> > >>> [2].)
> > >>>>>>
> > >>>>>> No this only control creating/updating mirrored topics and MM2
> > >>> internal
> > >>>>>> topics. And not going to affect connect runtime's internal topics
> > >>> that
> > >>>>> are
> > >>>>>> needed by connect cluster that runs MM2.
> > >>>>>>
> > >>>>>> 4. Would the admin class specified by the user also take effect
> for
> > >>>>>>> internal topics created by the Connect framework (i.e., the
> > >> statue,
> > >>>>>> config,
> > >>>>>>> and offsets topics)?
> > >>>>>>
> > >>>>>> No this wouldn't address connect as connect "realtime" uses
> > >>> TopicAdmin
> > >>>>> to
> > >>>>>> manage topics needed for KafkaOffsetBackingStore,
> > >>>> KafkaStatusBackingStore
> > >>>>>> and KafkaConfigBackingStore directly. These 3 topics aren't a huge
> > >>>>> concern
> > >>>>>> for MM2 as they are a small set of topics and can be created
> > >> up-front
> > >>>> as
> > >>>>> a
> > >>>>>> one-time job. However, the main concern of the KIP is addressing
> > >> the
> > >>>>>> mirrored topics, synced configs, synced ACLs and the internal
> > >> topics
> > >>> of
> > >>>>> MM2
> > >>>>>> which are needed for MM2 features like heartbeat and offset
> mapping
> > >>>>> between
> > >>>>>> Kafka clusters.
> > >>>>>>
> > >>>>>> The KIP states that Mirror Maker 2 will "Use
> > >>>>>>> ForwardingAdmin in MirrorUtils instead of TopicAdmin to create
> > >>>> internal
> > >>>>>>> compacted topics", but IIUC these topics (the ones created with
> > >> the
> > >>>>>>> MirrorUtils class) are Mirror Maker 2-specific and different from
> > >>> the
> > >>>>>>> Connect framework's internal topics.
> > >>>>>>
> > >>>>>> MM2 has been using 2 patterns to create topics
> > >>>>>> 1- Use AdminClient directly to create/update mirrored topics and
> > >>> their
> > >>>>> ACLs
> > >>>>>> 2- Use TopicAdmin in MirrorUtils to create MM2 internal topics
> > >> which
> > >>>> are
> > >>>>>> heartbeat, mm2-offset-syncs.<cluster_alias>.internal and
> > >>>>>> <cluster_alias>.checkpoints.internal
> > >>>>>>
> > >>>>>> This KIP will only replace AdminClient and TopicAdmin in the MM2
> > >>>> codebase
> > >>>>>> by ForwardingAdmin and not connect related topics.
> > >>>>>> As Colin mentioned before we can have a feature KIP where we use
> > >>>>>> ForwardingAdmin outside MM2 but this is not addressed in this KIP.
> > >>>>>>
> > >>>>>> Hope this answered your questions.
> > >>>>>>
> > >>>>>> Best
> > >>>>>> Omnia
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> On Wed, Jun 1, 2022 at 2:14 AM Chris Egerton <
> > >>> fearthecel...@gmail.com>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> Hi Omnia,
> > >>>>>>>
> > >>>>>>> Thank you for your patience with this KIP! I have a few quick
> > >>>> thoughts:
> > >>>>>>>
> > >>>>>>> 1. Is the introduction of the new "ForwardingAdmin" class
> > >>> necessary,
> > >>>> or
> > >>>>>> can
> > >>>>>>> the same behavior can be achieved by subclassing the existing
> > >>>>>>> KafkaAdminClient class?
> > >>>>>>>
> > >>>>>>> 2. Would it be just as accurate to name the new Mirror Maker 2
> > >>>> property
> > >>>>>>> "admin.class" instead of "forwarding.admin.class"? I think
> > >> brevity
> > >>>> may
> > >>>>>> work
> > >>>>>>> in our favor here
> > >>>>>>>
> > >>>>>>> 3. Would the admin class specified by the user also take effect
> > >> for
> > >>>>>> KIP-158
> > >>>>>>> [1] style automatic topic creation? (Forgive me if this isn't
> > >>>>> applicable
> > >>>>>>> for Mirror Maker 2; I'm asking solely based on the knowledge that
> > >>> MM2
> > >>>>> can
> > >>>>>>> be run as a source connector and has its own source task class
> > >>> [2].)
> > >>>>>>>
> > >>>>>>> 4. Would the admin class specified by the user also take effect
> > >> for
> > >>>>>>> internal topics created by the Connect framework (i.e., the
> > >> statue,
> > >>>>>> config,
> > >>>>>>> and offsets topics)? The KIP states that Mirror Maker 2 will "Use
> > >>>>>>> ForwardingAdmin in MirrorUtils instead of TopicAdmin to create
> > >>>> internal
> > >>>>>>> compacted topics", but IIUC these topics (the ones created with
> > >> the
> > >>>>>>> MirrorUtils class) are Mirror Maker 2-specific and different from
> > >>> the
> > >>>>>>> Connect framework's internal topics.
> > >>>>>>>
> > >>>>>>> [1] -
> > >>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-158%3A+Kafka+Connect+should+allow+source+connectors+to+set+topic-specific+settings+for+new+topics
> > >>>>>>> [2] -
> > >>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://github.com/apache/kafka/blob/4c9eeef5b2dff9a4f0977fbc5ac7eaaf930d0d0e/connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceTask.java
> > >>>>>>>
> > >>>>>>> Cheers,
> > >>>>>>>
> > >>>>>>> Chris
> > >>>>>>>
> > >>>>>>> On Wed, May 25, 2022 at 5:26 AM Omnia Ibrahim <
> > >>>> o.g.h.ibra...@gmail.com
> > >>>>>>
> > >>>>>>> wrote:
> > >>>>>>>
> > >>>>>>>> Hi everyone, If there's no major concern anymore, I'll start
> > >> the
> > >>>>>>>> voting process.
> > >>>>>>>>
> > >>>>>>>> On Fri, May 20, 2022 at 5:58 PM Omnia Ibrahim <
> > >>>>> o.g.h.ibra...@gmail.com
> > >>>>>>>
> > >>>>>>>> wrote:
> > >>>>>>>>
> > >>>>>>>>> Hi Colin,
> > >>>>>>>>>
> > >>>>>>>>>> Thanks for the clarification. I agree it's reasonable for
> > >>> people
> > >>>>> to
> > >>>>>>> want
> > >>>>>>>>> to use their own implementations of Admin. And we could have
> > >> a
> > >>>>> config
> > >>>>>>> for
> > >>>>>>>>> this, so that it becomes pluggable (possibly in other places
> > >>> than
> > >>>>>>>>> MirrorMaker, although we don't have to do that in this KIP).
> > >>>>>>>>>>
> > >>>>>>>>> Allowing people to plug custom implementation of Admin in
> > >> other
> > >>>>>> places
> > >>>>>>>>> sounds like a neat idea indeed. It can be nice addition for
> > >>>>> example `
> > >>>>>>>>> org.apache.kafka.connect.util.SharedTopicAdmin` in Connect to
> > >>> use
> > >>>>>>> custom
> > >>>>>>>>> Admin as well. But agree no need to have it in this KIP.
> > >>>>>>>>>> We could even try to make this easier on developers. For
> > >>>> example,
> > >>>>> we
> > >>>>>>>>> could provide a public ForwardingAdmin class that forwards
> > >> all
> > >>>>>> requests
> > >>>>>>>> to
> > >>>>>>>>> the regular KafkaAdminClient. Then, people could make their
> > >>>> custom
> > >>>>>>> class
> > >>>>>>>>> inherit from ForwardingAdmin and override >just the specific
> > >>>>> methods
> > >>>>>>> that
> > >>>>>>>>> they wanted to override. So they don't have to implement all
> > >>> the
> > >>>>>>> methods,
> > >>>>>>>>> but just the ones that are different for them.
> > >>>>>>>>>>
> > >>>>>>>>>> I just wanted to make sure we weren't creating a second
> > >> Admin
> > >>>>> client
> > >>>>>>>>> interface -- I think that would really be hard for us to
> > >>> support
> > >>>>>>>> long-term.
> > >>>>>>>>>
> > >>>>>>>>> Forwarding would defiantly make it easier. I have updated the
> > >>> KIP
> > >>>>> to
> > >>>>>>>>> introduce ForwardingAdmin as well.
> > >>>>>>>>>
> > >>>>>>>>> regards,
> > >>>>>>>>> Omnia
> > >>>>>>>>>
> > >>>>>>>>> On Mon, May 16, 2022 at 9:31 PM Colin McCabe <
> > >>> cmcc...@apache.org
> > >>>>>
> > >>>>>>> wrote:
> > >>>>>>>>>
> > >>>>>>>>>> On Mon, May 16, 2022, at 10:24, Omnia Ibrahim wrote:
> > >>>>>>>>>>> Hi Colin,
> > >>>>>>>>>>>
> > >>>>>>>>>>> Thanks for your reply.
> > >>>>>>>>>>>
> > >>>>>>>>>>> This KIP doesn’t aim to solve any security concerns, but
> > >>>> rather
> > >>>>> a
> > >>>>>>>>>> conflict
> > >>>>>>>>>>> of responsibilities within any Kafka ecosystem that
> > >> includes
> > >>>> MM2
> > >>>>>> and
> > >>>>>>>> any
> > >>>>>>>>>>> resource management solution. I’m not sure that was clear,
> > >>> so
> > >>>>> I’m
> > >>>>>>>>>> concerned
> > >>>>>>>>>>> about the motivation for your suggestion to close this
> > >> KIP.
> > >>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> Hi Omnia,
> > >>>>>>>>>>
> > >>>>>>>>>> Thanks for the clarification. I agree it's reasonable for
> > >>> people
> > >>>>> to
> > >>>>>>> want
> > >>>>>>>>>> to use their own implementations of Admin. And we could
> > >> have a
> > >>>>>> config
> > >>>>>>>> for
> > >>>>>>>>>> this, so that it becomes pluggable (possibly in other places
> > >>>> than
> > >>>>>>>>>> MirrorMaker, although we don't have to do that in this KIP).
> > >>>>>>>>>>
> > >>>>>>>>>> We could even try to make this easier on developers. For
> > >>>> example,
> > >>>>> we
> > >>>>>>>>>> could provide a public ForwardingAdmin class that forwards
> > >> all
> > >>>>>>> requests
> > >>>>>>>> to
> > >>>>>>>>>> the regular KafkaAdminClient. Then, people could make their
> > >>>> custom
> > >>>>>>> class
> > >>>>>>>>>> inherit from ForwardingAdmin and override just the specific
> > >>>>> methods
> > >>>>>>> that
> > >>>>>>>>>> they wanted to override. So they don't have to implement all
> > >>> the
> > >>>>>>>> methods,
> > >>>>>>>>>> but just the ones that are different for them.
> > >>>>>>>>>>
> > >>>>>>>>>> I just wanted to make sure we weren't creating a second
> > >> Admin
> > >>>>> client
> > >>>>>>>>>> interface -- I think that would really be hard for us to
> > >>> support
> > >>>>>>>> long-term.
> > >>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>> It is generally accepted that resource management should
> > >> be
> > >>>>>>>> centralized,
> > >>>>>>>>>>> especially on the scale of mirroring N number of clusters.
> > >>> The
> > >>>>>> point
> > >>>>>>>> of
> > >>>>>>>>>>> this KIP is that any sort of topic management / federate
> > >>>>> solution
> > >>>>>> /
> > >>>>>>>>>>> up-front capacity planning system will be at odds with MM2
> > >>> if
> > >>>>> MM2
> > >>>>>>>> keeps
> > >>>>>>>>>>> using the Admin client directly.
> > >>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> Thanks for the explanation. That makes sense.
> > >>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>> I understand your concern that the interface proposed in
> > >> the
> > >>>>> first
> > >>>>>>>>>> approach
> > >>>>>>>>>>> may become too similar to the existing Admin interface.
> > >> I’ll
> > >>>>>> update
> > >>>>>>>> the
> > >>>>>>>>>>> proposal by moving Ryanne’s previous suggestion to re-use
> > >>> the
> > >>>>>> Admin
> > >>>>>>>>>>> interface and add configuration to accept a custom
> > >>>>> implementation.
> > >>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> +1.
> > >>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>> If you still feel this KIP should be closed but can
> > >>> understand
> > >>>>> its
> > >>>>>>>>>>> motivation I can close this one and create a new one.
> > >>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>> I think it's reasonable to keep this one open and make the
> > >>>> changes
> > >>>>>> you
> > >>>>>>>>>> talked about above.
> > >>>>>>>>>>
> > >>>>>>>>>> regards,
> > >>>>>>>>>> Colin
> > >>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>> Thanks,
> > >>>>>>>>>>> Omnia
> > >>>>>>>>>>>
> > >>>>>>>>>>> On Fri, May 13, 2022 at 6:10 PM Colin McCabe <
> > >>>>> cmcc...@apache.org>
> > >>>>>>>>>> wrote:
> > >>>>>>>>>>>
> > >>>>>>>>>>>> On Wed, May 11, 2022, at 15:07, Omnia Ibrahim wrote:
> > >>>>>>>>>>>>> Hi Colin,
> > >>>>>>>>>>>>> I don't mind the idea of MM2 users implementing the
> > >>>>> AdminClient
> > >>>>>>>>>>>> interface.
> > >>>>>>>>>>>>> However, there're two disadvantages to this.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>   1. Having around 70 methods definitions to have
> > >>>>>>> "NotImplemented"
> > >>>>>>>>>> is
> > >>>>>>>>>>>> one
> > >>>>>>>>>>>>>   downside, and keep up with these if the AdminClient
> > >>>>>> interface
> > >>>>>>>>>> changes.
> > >>>>>>>>>>>>>   2. It makes it hard to list what admin functionality
> > >>> MM2
> > >>>>>> uses
> > >>>>>>> as
> > >>>>>>>>>> MM2
> > >>>>>>>>>>>>>   interactions with AdminClient in the codebase are in
> > >>>> many
> > >>>>>>>> places.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I guess it's OK for MM2 users who want to build their
> > >>> admin
> > >>>>>>> client
> > >>>>>>>> to
> > >>>>>>>>>>>> carry
> > >>>>>>>>>>>>> this burden, as I explained in my previous response to
> > >>> the
> > >>>>>>>> discussion
> > >>>>>>>>>>>>> thread. And we can do some cleanup to the codebase to
> > >>> have
> > >>>>> all
> > >>>>>>>> Admin
> > >>>>>>>>>>>>> interactions in MM2 in a utils class or something like
> > >>> that
> > >>>>> to
> > >>>>>>> make
> > >>>>>>>>>> it
> > >>>>>>>>>>>>> easier to navigate what MM2 needs from the Admin
> > >>> interface.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Hi Omnia,
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Anyone who wants to extend Kafka with proprietary tooling
> > >>>> does
> > >>>>>> need
> > >>>>>>>> to
> > >>>>>>>>>>>> keep up with the Kafka API. We have done everything we
> > >> can
> > >>> to
> > >>>>>> make
> > >>>>>>>> this
> > >>>>>>>>>>>> easier. We rigorously define what the API is through the
> > >>> KIP
> > >>>>>>> process,
> > >>>>>>>>>> and
> > >>>>>>>>>>>> make it possible to extend by making it an interface
> > >> rather
> > >>>>> than
> > >>>>>>>>>> concrete
> > >>>>>>>>>>>> class. We also have a pretty lengthy deprecation process
> > >>> for
> > >>>>>> these
> > >>>>>>>>>> APIs.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Maybe I'm misunderstanding the use-case you're
> > >> describing
> > >>>>> here.
> > >>>>>>> But
> > >>>>>>>>>> it
> > >>>>>>>>>>>>>> seems to me that if you create a proxy that has the
> > >>>> ability
> > >>>>> to
> > >>>>>>> do
> > >>>>>>>>>> any
> > >>>>>>>>>>>> admin
> > >>>>>>>>>>>>>> operation, and give MM2 access to that proxy, the
> > >>> security
> > >>>>>> model
> > >>>>>>>> is
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>> same as just giving MM2 admin access. (Or it may be
> > >>> worse
> > >>>> if
> > >>>>>> the
> > >>>>>>>>>>>> sysadmin
> > >>>>>>>>>>>>>> doesn't know what this proxy is doing, and doesn't
> > >> lock
> > >>> it
> > >>>>>>>> down...)
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> MM2 runs with the assumption that it has
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>   - "CREATE" ACLs for topics on the source clusters to
> > >>>>> create
> > >>>>>>>>>>>> `heartbeat`
> > >>>>>>>>>>>>>   topics.
> > >>>>>>>>>>>>>   - "CREATE"  and "ALTER" ACLs to create topics, add
> > >>>>>> partitions,
> > >>>>>>>>>> update
> > >>>>>>>>>>>>>   topics' config and topics' ACLs (in future, will
> > >> also
> > >>>>>> include
> > >>>>>>>>>> group
> > >>>>>>>>>>>> ACLS as
> > >>>>>>>>>>>>>   Mikael mentioned before in the thread) on the
> > >>>> destination
> > >>>>>>>>>> clusters.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Most organisations have some resource management or
> > >>>> federated
> > >>>>>>>>>> solutions
> > >>>>>>>>>>>>> (some would even have a budget system as part of these
> > >>>>> systems)
> > >>>>>>> to
> > >>>>>>>>>> manage
> > >>>>>>>>>>>>> Kafka resources, and these systems are usually the only
> > >>>>>>> application
> > >>>>>>>>>>>> allowed
> > >>>>>>>>>>>>> to initializing a client with "CREATE" and "ALTER"
> > >> ACLs.
> > >>>> They
> > >>>>>>> don't
> > >>>>>>>>>> grant
> > >>>>>>>>>>>>> these ACLs to any other teams/groups/applications to
> > >>> create
> > >>>>>> such
> > >>>>>>> a
> > >>>>>>>>>> client
> > >>>>>>>>>>>>> outside these systems, so assuming MM2 can bypass these
> > >>>>> systems
> > >>>>>>> and
> > >>>>>>>>>> use
> > >>>>>>>>>>>> the
> > >>>>>>>>>>>>> AdminClient directly to create/update resources isn't
> > >>>> valid.
> > >>>>>> This
> > >>>>>>>> is
> > >>>>>>>>>> the
> > >>>>>>>>>>>>> primary concern here.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> The KIP is trying to give MM2 more flexibility to allow
> > >>>>>>>>>> organisations to
> > >>>>>>>>>>>>> integrate MM2 with their resource management system as
> > >>> they
> > >>>>> see
> > >>>>>>> fit
> > >>>>>>>>>>>> without
> > >>>>>>>>>>>>> forcing them to disable most MM2 features.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Hope this make sense and clear it up.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> The point I was trying to make is that there is no
> > >>> additional
> > >>>>>>>> security
> > >>>>>>>>>>>> here. If you have some agent that has all the
> > >> permissions,
> > >>>> and
> > >>>>>> MM2
> > >>>>>>>> can
> > >>>>>>>>>> talk
> > >>>>>>>>>>>> to that agent and tell it what to do, then that is
> > >>> equivalent
> > >>>>> to
> > >>>>>>> just
> > >>>>>>>>>>>> giving MM2 all the permissions. So while there may be
> > >> other
> > >>>>>> reasons
> > >>>>>>>> to
> > >>>>>>>>>> use
> > >>>>>>>>>>>> this kind of agent-based architecture, added security
> > >> isn't
> > >>>>> one.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> In any case, I think we should close this KIP since we
> > >>>> already
> > >>>>>> have
> > >>>>>>>> an
> > >>>>>>>>>>>> Admin API. There isn't a need to create a public API for
> > >>>> admin
> > >>>>>>>>>> operations.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> best,
> > >>>>>>>>>>>> Colin
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> On Wed, May 11, 2022 at 9:09 PM Colin McCabe <
> > >>>>>> cmcc...@apache.org
> > >>>>>>>>
> > >>>>>>>>>> wrote:
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Hi Omnia Ibrahim,
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> I'm sorry, but I am -1 on adding competing Admin
> > >>>> interfaces.
> > >>>>>>> This
> > >>>>>>>>>> would
> > >>>>>>>>>>>>>> create confusion and a heavier maintenance burden for
> > >>> the
> > >>>>>>> project.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Since the org.apache.kafka.clients.admin.Admin
> > >> interface
> > >>>> is
> > >>>>> a
> > >>>>>>> Java
> > >>>>>>>>>>>>>> interface, any third-party software that wants to
> > >> insert
> > >>>> its
> > >>>>>> own
> > >>>>>>>>>>>>>> implementation of the interface can do so already.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> A KIP to make the Admin class used pluggable for MM2
> > >>> would
> > >>>>> be
> > >>>>>>>>>>>> reasonable.
> > >>>>>>>>>>>>>> Adding a competing admin API is not.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> It's true that there are many Admin methods, but you
> > >> do
> > >>>> not
> > >>>>>> need
> > >>>>>>>> to
> > >>>>>>>>>>>>>> implement all of them -- just the ones that
> > >> MirrorMaker
> > >>>>> uses.
> > >>>>>>> The
> > >>>>>>>>>> other
> > >>>>>>>>>>>>>> ones can throw a NotImplementedException or similar.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> The current approach also assumes that the user
> > >>> running
> > >>>>> MM2
> > >>>>>>> has
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>> Admin right to
> > >>>>>>>>>>>>>>> create/update topics, which is only valid if the
> > >> user
> > >>>> who
> > >>>>>> runs
> > >>>>>>>> MM2
> > >>>>>>>>>>>> also
> > >>>>>>>>>>>>>> manages both
> > >>>>>>>>>>>>>>> source and destination clusters.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Maybe I'm misunderstanding the use-case you're
> > >>> describing
> > >>>>>> here.
> > >>>>>>>> But
> > >>>>>>>>>> it
> > >>>>>>>>>>>>>> seems to me that if you create a proxy that has the
> > >>>> ability
> > >>>>> to
> > >>>>>>> do
> > >>>>>>>>>> any
> > >>>>>>>>>>>> admin
> > >>>>>>>>>>>>>> operation, and give MM2 access to that proxy, the
> > >>> security
> > >>>>>> model
> > >>>>>>>> is
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>> same as just giving MM2 admin access. (Or it may be
> > >>> worse
> > >>>> if
> > >>>>>> the
> > >>>>>>>>>>>> sysadmin
> > >>>>>>>>>>>>>> doesn't know what this proxy is doing, and doesn't
> > >> lock
> > >>> it
> > >>>>>>>> down...)
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> best,
> > >>>>>>>>>>>>>> Colin
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> On Mon, May 9, 2022, at 13:21, Omnia Ibrahim wrote:
> > >>>>>>>>>>>>>>> Hi, I gave the KIP another look after talking to
> > >> some
> > >>>>> people
> > >>>>>>> at
> > >>>>>>>>>> the
> > >>>>>>>>>>>> Kafka
> > >>>>>>>>>>>>>>> Summit in London. And I would like to clear up the
> > >>>>>> motivation
> > >>>>>>> of
> > >>>>>>>>>> this
> > >>>>>>>>>>>>>> KIP.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> At the moment, MM2 has some opinionated decisions
> > >> that
> > >>>> are
> > >>>>>>>>>> creating
> > >>>>>>>>>>>>>> issues
> > >>>>>>>>>>>>>>> for teams that use IaC, federated solutions or have
> > >> a
> > >>>>>>>>>> capacity/budget
> > >>>>>>>>>>>>>>> planning system for Kafka destination clusters. To
> > >>>> explain
> > >>>>>> it
> > >>>>>>>>>> better,
> > >>>>>>>>>>>>>> let's
> > >>>>>>>>>>>>>>> assume we have MM2 with the following configurations
> > >>> to
> > >>>>>>>> highlight
> > >>>>>>>>>>>> these
> > >>>>>>>>>>>>>>> problems.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> ```
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> topics = .*
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> refresh.topics.enabled = true
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> sync.topic.configs.enabled = true
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> sync.topic.acls.enabled = true
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> // Maybe in futrue we can have
> > >>> sync.group.acls.enabled =
> > >>>>>> true
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> ```
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> These configurations allow us to run MM2 with the
> > >>> value
> > >>>> of
> > >>>>>> its
> > >>>>>>>>>> full
> > >>>>>>>>>>>>>>> features. However, there are two main concerns when
> > >> we
> > >>>> run
> > >>>>>> on
> > >>>>>>> a
> > >>>>>>>>>> scale
> > >>>>>>>>>>>>>> with
> > >>>>>>>>>>>>>>> these configs:
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> 1. *Capacity/Budgeting Planning:*
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> Functionality or features that impact capacity
> > >>> planning
> > >>>>>> using
> > >>>>>>>> MM2
> > >>>>>>>>>> are:
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>   1. MM2 automatically creates topics (breaking the
> > >>>> rule
> > >>>>> of
> > >>>>>>>>>>>>>>>   `auto.create.topics.enable=false`) and creates
> > >>> topic
> > >>>>>>>>>> partitions on
> > >>>>>>>>>>>>>>>   destination clusters if the number of partitions
> > >>>>>> increases
> > >>>>>>> on
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>> source.
> > >>>>>>>>>>>>>>>   In the previous example, this functionality will
> > >>>> apply
> > >>>>> to
> > >>>>>>> any
> > >>>>>>>>>> topic
> > >>>>>>>>>>>>>> that
> > >>>>>>>>>>>>>>>   matches the regex of the `topics` config.
> > >>>>>>>>>>>>>>>   2. Sync topic configs include configurations that
> > >>>>> impact
> > >>>>>>>>>> capacity
> > >>>>>>>>>>>>>> like `
> > >>>>>>>>>>>>>>>   retention.ms` and `retention.bytes`.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> These 2 points lead to adding new untracked capacity
> > >>> to
> > >>>>>>>>>> destination
> > >>>>>>>>>>>>>>> clusters without a way to count for them up-front or
> > >>>>>> safeguard
> > >>>>>>>> the
> > >>>>>>>>>>>>>> cluster.
> > >>>>>>>>>>>>>>> The team that runs the cluster will only see the
> > >>>> capacity
> > >>>>>>> issue
> > >>>>>>>>>> when
> > >>>>>>>>>>>>>> their
> > >>>>>>>>>>>>>>> disk usage hits the threshold for their alerts. The
> > >>> desk
> > >>>>>>>> capacity
> > >>>>>>>>>>>> issue
> > >>>>>>>>>>>>>> can
> > >>>>>>>>>>>>>>> be avoided if MM2 is flexible enough to
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>   - have a way for teams that run their ecosystem
> > >> to
> > >>>> have
> > >>>>>> MM2
> > >>>>>>>>>> behave
> > >>>>>>>>>>>>>>>   within their system.
> > >>>>>>>>>>>>>>>   - disable the auto-creation and avoid syncing
> > >>> configs
> > >>>>>> that
> > >>>>>>>>>> impact
> > >>>>>>>>>>>>>>>   capacity
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> 2. *Provisioning conflict:*
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> In the previous MM2 configurations; we ended up with
> > >>>>>> conflict
> > >>>>>>> as
> > >>>>>>>>>> MM2
> > >>>>>>>>>>>> used
> > >>>>>>>>>>>>>>> `AdminClient` directly to perform the following
> > >>>>>> functionality
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>   -  Create a Kafka topic (no way to disable this
> > >> at
> > >>>> the
> > >>>>>>>> moment)
> > >>>>>>>>>>>>>>>   -  Add new Kafka partitions (no way to disable
> > >> this
> > >>>> at
> > >>>>>> the
> > >>>>>>>>>> moment)
> > >>>>>>>>>>>>>>>   -  Sync Kafka Topic configurations (can be
> > >>> disabled,
> > >>>>> but
> > >>>>>>> then
> > >>>>>>>>>> this
> > >>>>>>>>>>>>>>>   reduces the value of MM2 potential for users)
> > >>>>>>>>>>>>>>>   -  Sync Kafka topic's ACLs (can be disabled, but
> > >>> this
> > >>>>>>> reduces
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>> users'
> > >>>>>>>>>>>>>>>   value). Disabling this feature also means that
> > >>> users
> > >>>>> must
> > >>>>>>>>>> ensure
> > >>>>>>>>>>>> they
> > >>>>>>>>>>>>>> have
> > >>>>>>>>>>>>>>>   the right ACLs to the mirrored topics on the
> > >>>>> destination
> > >>>>>>>>>> cluster
> > >>>>>>>>>>>>>> before
> > >>>>>>>>>>>>>>>   switching their consumers, especially when MM2 is
> > >>>> used
> > >>>>>> for
> > >>>>>>>>>> disaster
> > >>>>>>>>>>>>>>>   recovery. It may lead to extra downtime for them.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> All these functionalities are using AdminClient;
> > >> which
> > >>>>>> causes
> > >>>>>>> an
> > >>>>>>>>>> issue
> > >>>>>>>>>>>>>> with
> > >>>>>>>>>>>>>>> teams that
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>   - Manage their Kafka resources using tools like
> > >>>>> Strimizi
> > >>>>>> or
> > >>>>>>>>>> custom
> > >>>>>>>>>>>>>>>   federated solutions. For example, Strimizi's
> > >>>>> UserOperator
> > >>>>>>>>>> doesn't
> > >>>>>>>>>>>>>> sync the
> > >>>>>>>>>>>>>>>   topic ACLs when MM2 is enabled. Strimzi
> > >>> documentation
> > >>>>>>>> mentions
> > >>>>>>>>>> that
> > >>>>>>>>>>>>>> users
> > >>>>>>>>>>>>>>>   must to disable MM2 `sync.topic.acls.enabled` if
> > >>> they
> > >>>>> use
> > >>>>>>>>>>>>>> `UserOperator`.
> > >>>>>>>>>>>>>>>   On the other hand, Strimizi's TopicOperator
> > >> doesn't
> > >>>>> have
> > >>>>>>> the
> > >>>>>>>>>> same
> > >>>>>>>>>>>>>> issue
> > >>>>>>>>>>>>>>>   because it has a bi-directional reconciliation
> > >>>> process
> > >>>>>> that
> > >>>>>>>>>> watches
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>   topics state on the Kafka cluster and updates
> > >>>>> KafkaTopic
> > >>>>>>>>>> resources
> > >>>>>>>>>>>> for
> > >>>>>>>>>>>>>>>   Strimzi. This design works fine with Kafka MM2
> > >> for
> > >>>>> Topics
> > >>>>>>> but
> > >>>>>>>>>> not
> > >>>>>>>>>>>> for
> > >>>>>>>>>>>>>>>   syncing ACLs. Strimizi TopicOperator also doesn't
> > >>>> have
> > >>>>> a
> > >>>>>>> way
> > >>>>>>>> to
> > >>>>>>>>>>>> stop
> > >>>>>>>>>>>>>>>   syncing config that impact capacity for example
> > >>>>> retention
> > >>>>>>>>>> configs.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>   - Teams that run MM2 but don't own the
> > >> destination
> > >>>>>> cluster.
> > >>>>>>>> In
> > >>>>>>>>>> this
> > >>>>>>>>>>>>>>>   case, these teams don't have Admin access, but
> > >> they
> > >>>> may
> > >>>>>>> have
> > >>>>>>>>>> Kafka
> > >>>>>>>>>>>>>>>   management solutions, such as yahoo/CMAK or an
> > >>>> in-house
> > >>>>>>>>>> solution.
> > >>>>>>>>>>>> For
> > >>>>>>>>>>>>>> such
> > >>>>>>>>>>>>>>>   a tool as CMAK, these teams can update/create
> > >>>> resources
> > >>>>>>> using
> > >>>>>>>>>> CMAK
> > >>>>>>>>>>>>>> REST API.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> The Proposed KIP gives users the flexibility to
> > >>>> integrate
> > >>>>>> MM2
> > >>>>>>>>>> within
> > >>>>>>>>>>>>>> their
> > >>>>>>>>>>>>>>> ecosystem without disabling any MM2 features. We can
> > >>>>> achieve
> > >>>>>>>> this
> > >>>>>>>>>>>>>>> flexibility with one of the following solutions.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>   1. Introduce a new interface that hides Admin
> > >>>>>> interactions
> > >>>>>>> in
> > >>>>>>>>>> one
> > >>>>>>>>>>>>>> place.
> > >>>>>>>>>>>>>>>   Then users can provide their way of resource
> > >>>>> management.
> > >>>>>> As
> > >>>>>>>>>> well as
> > >>>>>>>>>>>>>> clean
> > >>>>>>>>>>>>>>>   up the MM2 code by having one place that manages
> > >>> the
> > >>>>>>>>>> resources, as
> > >>>>>>>>>>>> at
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>   moment, MM2 usage of AdminClient is all over the
> > >>>> code.
> > >>>>>>>>>>>>>>>   2. The second solution could be to add only a new
> > >>>>> config
> > >>>>>>> that
> > >>>>>>>>>>>> allows
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>   users to override AdminClient with another
> > >>>>> implementation
> > >>>>>>> of
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>>>   AdminClient interface, as Ryanne suggested
> > >> before.
> > >>>> The
> > >>>>>>>>>> downside is
> > >>>>>>>>>>>>>> that
> > >>>>>>>>>>>>>>>   AdminClient is enormous and constantly under
> > >>>>> development,
> > >>>>>>> so
> > >>>>>>>>>> any
> > >>>>>>>>>>>>>> users who
> > >>>>>>>>>>>>>>>   opt-in for custom implementation will need to
> > >> carry
> > >>>>> this
> > >>>>>>>>>> burden.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> I favour the first solution as it will make it
> > >> either
> > >>>>> later
> > >>>>>> to
> > >>>>>>>>>> add any
> > >>>>>>>>>>>>>> new
> > >>>>>>>>>>>>>>> feature related to resource management. But don't
> > >> mind
> > >>>> if
> > >>>>>>> others
> > >>>>>>>>>> think
> > >>>>>>>>>>>>>> that
> > >>>>>>>>>>>>>>> the second solution is easier for MM2 design.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> *Note*: There are two possible future KIPs following
> > >>>> this
> > >>>>>> KIP
> > >>>>>>> to
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>   1. Add config to disable MM2 from auto creating
> > >> or
> > >>>>> adding
> > >>>>>>> new
> > >>>>>>>>>> topic
> > >>>>>>>>>>>>>>>   partitions.
> > >>>>>>>>>>>>>>>   2. Add a way to exclude a specific topic's
> > >>>>> configuration
> > >>>>>>> from
> > >>>>>>>>>> being
> > >>>>>>>>>>>>>>>   synced.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> I hope this clears up the problem better. Please let
> > >>> me
> > >>>>> know
> > >>>>>>>> what
> > >>>>>>>>>> do
> > >>>>>>>>>>>> you
> > >>>>>>>>>>>>>>> think.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> On Mon, Feb 7, 2022 at 3:26 PM Omnia Ibrahim <
> > >>>>>>>>>> o.g.h.ibra...@gmail.com
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> Hi Mickael. Thanks for the feedback. I address some
> > >>> of
> > >>>>> your
> > >>>>>>>>>> points
> > >>>>>>>>>>>>>> below.
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> *> This seems to address a relatively advanced and
> > >>>>> specific
> > >>>>>>> use
> > >>>>>>>>>> case*
> > >>>>>>>>>>>>>>>> The main point of the KIP is that MM2 is making a
> > >>>> massive
> > >>>>>>>>>> assumption
> > >>>>>>>>>>>>>> that
> > >>>>>>>>>>>>>>>> it has the right to alter/create resources. This
> > >>>>> assumption
> > >>>>>>>> isn't
> > >>>>>>>>>>>> valid
> > >>>>>>>>>>>>>> in
> > >>>>>>>>>>>>>>>> the world of Infra-as-Code, federated solutions and
> > >>>>>>> popularity
> > >>>>>>>>>> of OS
> > >>>>>>>>>>>>>> Kafka
> > >>>>>>>>>>>>>>>> Kubernetes operators; these infra/resource
> > >> management
> > >>>>>>> solutions
> > >>>>>>>>>>>> aren't
> > >>>>>>>>>>>>>>>> advanced use-cases anymore nowadays. These concerns
> > >>> had
> > >>>>>> been
> > >>>>>>>>>> raised
> > >>>>>>>>>>>> in
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>> past, especially regarding the assumption that MM2
> > >>> can
> > >>>>>> create
> > >>>>>>>>>> topics
> > >>>>>>>>>>>> on
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>> destination cluster. For example,
> > >>>>>>>>>>>>>>>> https://issues.apache.org/jira/browse/KAFKA-12753
> > >>> and
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>
> > >> https://www.mail-archive.com/dev@kafka.apache.org/msg119340.html
> > >>>>>>>>>> .
> > >>>>>>>>>>>>>>>> The primary motivation is giving some power to data
> > >>>>>>>>>>>>>>>> platform/infrastructure team to make MM2 part of
> > >>> their
> > >>>>>>> internal
> > >>>>>>>>>> Kafka
> > >>>>>>>>>>>>>>>> ecosystem without dropping the main features that
> > >>> make
> > >>>>> MM2
> > >>>>>>>>>> valuable,
> > >>>>>>>>>>>>>> like
> > >>>>>>>>>>>>>>>> syncing topic configs. For example, if someone uses
> > >>> any
> > >>>>> OS
> > >>>>>>>> Kafka
> > >>>>>>>>>> k8s
> > >>>>>>>>>>>>>>>> operator, they can implement the class to interact
> > >>> with
> > >>>>> the
> > >>>>>>> k8s
> > >>>>>>>>>>>>>> operator to
> > >>>>>>>>>>>>>>>> create these resources.
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> *> My initial concern is this may make it hard to
> > >>>> evolve
> > >>>>>>>>>> MirrorMaker
> > >>>>>>>>>>>> as
> > >>>>>>>>>>>>>>>> we'll likely need to update this new interface if
> > >> new
> > >>>>>>> features
> > >>>>>>>>>> are
> > >>>>>>>>>>>>>> added.*
> > >>>>>>>>>>>>>>>> I agree it's a disadvantage to adding a new
> > >> interface
> > >>>>>> however
> > >>>>>>>>>> adding
> > >>>>>>>>>>>>>> more
> > >>>>>>>>>>>>>>>> admin interactions from MM2 to alter/create
> > >> resources
> > >>>> and
> > >>>>>>>> access
> > >>>>>>>>>> will
> > >>>>>>>>>>>>>> feed
> > >>>>>>>>>>>>>>>> the main issue as I mentioned above with the
> > >>> popularity
> > >>>>> of
> > >>>>>>> IaC
> > >>>>>>>>>> and
> > >>>>>>>>>>>>>>>> federated solutions; most data
> > >>> platform/infrastructure
> > >>>>>> teams
> > >>>>>>>> will
> > >>>>>>>>>>>> endup
> > >>>>>>>>>>>>>>>> disabling these new features.
> > >>>>>>>>>>>>>>>> Also, at the moment, most MM2 interactions with the
> > >>>> admin
> > >>>>>>>> client
> > >>>>>>>>>> are
> > >>>>>>>>>>>>>>>> scattered across the codebase so having one place
> > >>> where
> > >>>>> all
> > >>>>>>>> admin
> > >>>>>>>>>>>>>>>> interactions are listed isn't a bad thing.
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> *> For example if we wanted to sync group ACLs.*
> > >>>>>>>>>>>>>>>> As I mentioned before, altering any resource's
> > >>>>>> configurations
> > >>>>>>>>>> with
> > >>>>>>>>>>>> MM2
> > >>>>>>>>>>>>>> is
> > >>>>>>>>>>>>>>>> the one main concern for any data
> > >>>> platform/infrastructure
> > >>>>>>> team
> > >>>>>>>>>> that
> > >>>>>>>>>>>>>> wants
> > >>>>>>>>>>>>>>>> to have control over their clusters and use MM2. So
> > >>> the
> > >>>>>> main
> > >>>>>>>>>> question
> > >>>>>>>>>>>>>> with
> > >>>>>>>>>>>>>>>> adding any new altering feature like sync group
> > >> ACLs
> > >>>> will
> > >>>>>>> raise
> > >>>>>>>>>> the
> > >>>>>>>>>>>> same
> > >>>>>>>>>>>>>>>> question of how many teams will actually enable
> > >> this
> > >>>>>> feature.
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> *>Regarding the proposed API, I have a few
> > >>> suggestions:
> > >>>>>> -
> > >>>>>>> What
> > >>>>>>>>>> about
> > >>>>>>>>>>>>>>>> using configure() instead of the constructor to
> > >> pass
> > >>>> the
> > >>>>>>>>>>>>> configuration,
> > >>>>>>>>>>>>>>>> especially as it's implementing Configurable >-
> > >> It's
> > >>>> not
> > >>>>>>> clear
> > >>>>>>>>>> what
> > >>>>>>>>>>>> all
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>> arguments of createTopicPartitions()>are. What's
> > >> the
> > >>>>>>> difference
> > >>>>>>>>>>>> between
> > >>>>>>>>>>>>>>>> partitionCounts and newPartitions?>Should we have
> > >>>>> separate
> > >>>>>>>>>> methods
> > >>>>>>>>>>>> for
> > >>>>>>>>>>>>>>>> creating topics and partitions? >- Do we really
> > >> need
> > >>>>>>>>>>>>>>>> createCompactedTopic()? >- Instead of
> > >>>>> updateTopicConfigs()
> > >>>>>>> and
> > >>>>>>>>>>>>>> updateAcls()
> > >>>>>>>>>>>>>>>> should we use the >"alter" prefix to stay
> > >> consistent
> > >>>> with
> > >>>>>>>> Admin?*
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> These are good suggestions that will update the KIP
> > >>> to
> > >>>>>>> address
> > >>>>>>>>>> these.
> > >>>>>>>>>>>>>>>> Regarding the `createCompactedTopic` MM2 is using
> > >>> this
> > >>>>>> method
> > >>>>>>>> to
> > >>>>>>>>>>>> create
> > >>>>>>>>>>>>>>>> internal topics.
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> Thanks
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> On Wed, Jan 26, 2022 at 1:55 PM Mickael Maison <
> > >>>>>>>>>>>>>> mickael.mai...@gmail.com>
> > >>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> Hi Omnia,
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> Thanks for the KIP, sorry for taking so long to
> > >>>> comment.
> > >>>>>>> I've
> > >>>>>>>>>> only
> > >>>>>>>>>>>> had
> > >>>>>>>>>>>>>>>>> time to take a quick look so far.
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> This seems to address a relatively advanced and
> > >>>> specific
> > >>>>>> use
> > >>>>>>>>>> case.
> > >>>>>>>>>>>> My
> > >>>>>>>>>>>>>>>>> initial concern is this may make it hard to evolve
> > >>>>>>> MirrorMaker
> > >>>>>>>>>> as
> > >>>>>>>>>>>>>>>>> we'll likely need to update this new interface if
> > >>> new
> > >>>>>>> features
> > >>>>>>>>>> are
> > >>>>>>>>>>>>>>>>> added. For example if we wanted to sync group
> > >> ACLs.
> > >>>>>>>>>>>>>>>>> I'm wondering if it's something you've thought
> > >>> about.
> > >>>>> I'm
> > >>>>>>> not
> > >>>>>>>>>> saying
> > >>>>>>>>>>>>>>>>> it's a blocker but we have to weigh the pros and
> > >>> cons
> > >>>>> when
> > >>>>>>>>>>>> introducing
> > >>>>>>>>>>>>>>>>> new features.
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> Regarding the proposed API, I have a few
> > >>> suggestions:
> > >>>>>>>>>>>>>>>>> - What about using configure() instead of the
> > >>>>> constructor
> > >>>>>> to
> > >>>>>>>>>> pass
> > >>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>>> configuration, especially as it's implementing
> > >>>>>> Configurable
> > >>>>>>>>>>>>>>>>> - It's not clear what all the arguments of
> > >>>>>>>>>> createTopicPartitions()
> > >>>>>>>>>>>>>>>>> are. What's the difference between partitionCounts
> > >>> and
> > >>>>>>>>>>>> newPartitions?
> > >>>>>>>>>>>>>>>>> Should we have separate methods for creating
> > >> topics
> > >>>> and
> > >>>>>>>>>> partitions?
> > >>>>>>>>>>>>>>>>> - Do we really need createCompactedTopic()?
> > >>>>>>>>>>>>>>>>> - Instead of updateTopicConfigs() and updateAcls()
> > >>>>> should
> > >>>>>> we
> > >>>>>>>>>> use the
> > >>>>>>>>>>>>>>>>> "alter" prefix to stay consistent with Admin?
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> Thanks,
> > >>>>>>>>>>>>>>>>> Mickael
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> On Wed, Jan 26, 2022 at 11:26 AM Omnia Ibrahim <
> > >>>>>>>>>>>>>> o.g.h.ibra...@gmail.com>
> > >>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>> Hi,
> > >>>>>>>>>>>>>>>>>> If there are no more concerns regarding the
> > >>> proposal
> > >>>>>> can I
> > >>>>>>>> get
> > >>>>>>>>>>>> some
> > >>>>>>>>>>>>>>>>> votes on the KIP here
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>
> > >>>> https://lists.apache.org/thread/950lpxjb5kbjm8qdszlpxm9h4j4sfyjx
> > >>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>> Thanks
> > >>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>> On Wed, Oct 27, 2021 at 3:54 PM Ryanne Dolan <
> > >>>>>>>>>>>> ryannedo...@gmail.com>
> > >>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> Well I'm convinced! Thanks for looking into it.
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> Ryanne
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> On Wed, Oct 27, 2021, 8:49 AM Omnia Ibrahim <
> > >>>>>>>>>>>>>> o.g.h.ibra...@gmail.com>
> > >>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>> I checked the difference between the number
> > >> of
> > >>>>>> methods
> > >>>>>>> in
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>> Admin
> > >>>>>>>>>>>>>>>>>>>> interface and the number of methods MM2
> > >> invokes
> > >>>>> from
> > >>>>>>>>>> Admin, and
> > >>>>>>>>>>>>>> this
> > >>>>>>>>>>>>>>>>> diff
> > >>>>>>>>>>>>>>>>>>>> is enormous (it's more than 70 methods).
> > >>>>>>>>>>>>>>>>>>>> As far as I can see, the following methods
> > >> MM2
> > >>>>>> depends
> > >>>>>>> on
> > >>>>>>>>>> (in
> > >>>>>>>>>>>>>>>>>>>> MirrorSourceConnector, MirrorMaker,
> > >>>>>>> MirrorCheckpointTask
> > >>>>>>>>>> and
> > >>>>>>>>>>>>>>>>>>>> MirrorCheckpointConnector), this will leave
> > >> 73
> > >>>>>> methods
> > >>>>>>> on
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>> Admin
> > >>>>>>>>>>>>>>>>>>>> interface that customer will need to return
> > >>> dummy
> > >>>>>> data
> > >>>>>>>> for,
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>   1. create(conf)
> > >>>>>>>>>>>>>>>>>>>>   2. close
> > >>>>>>>>>>>>>>>>>>>>   3. listTopics()
> > >>>>>>>>>>>>>>>>>>>>   4. createTopics(newTopics,
> > >>>> createTopicsOptions)
> > >>>>>>>>>>>>>>>>>>>>   5. createPartitions(newPartitions)
> > >>>>>>>>>>>>>>>>>>>>   6. alterConfigs(configs)  // this method
> > >> is
> > >>>>> marked
> > >>>>>>> for
> > >>>>>>>>>>>>>>>>> deprecation in
> > >>>>>>>>>>>>>>>>>>>>   Admin and the ConfigResource MM2 use is
> > >> only
> > >>>>> TOPIC
> > >>>>>>>>>>>>>>>>>>>>   7. createAcls(aclBindings) // the list of
> > >>>>> bindings
> > >>>>>>>>>> always
> > >>>>>>>>>>>>>>>>> filtered by
> > >>>>>>>>>>>>>>>>>>>>   TOPIC
> > >>>>>>>>>>>>>>>>>>>>   8. describeAcls(aclBindingFilter) //
> > >> filter
> > >>> is
> > >>>>>>> always
> > >>>>>>>>>>>>>>>>> ANY_TOPIC_ACL
> > >>>>>>>>>>>>>>>>>>>>   9. describeConfigs(configResources) //
> > >>> Always
> > >>>>> for
> > >>>>>>>> TOPIC
> > >>>>>>>>>>>>>> resources
> > >>>>>>>>>>>>>>>>>>>>   10. listConsumerGroupOffsets(groupId)
> > >>>>>>>>>>>>>>>>>>>>   11. listConsumerGroups()
> > >>>>>>>>>>>>>>>>>>>>   12. alterConsumerGroupOffsets(groupId,
> > >>>> offsets)
> > >>>>>>>>>>>>>>>>>>>>   13. describeCluster() // this is invoked
> > >>> from
> > >>>>>>>>>>>>>>>>>>>> ConnectUtils.lookupKafkaClusterId(conf),
> > >>>>>>>>>>>>>>>>>>>>   but MM2 isn't the one that initialize the
> > >>>>>>> AdminClient
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>> Going with the Admin interface in practice
> > >> will
> > >>>>> make
> > >>>>>>> any
> > >>>>>>>>>> custom
> > >>>>>>>>>>>>>> Admin
> > >>>>>>>>>>>>>>>>>>>> implementation humongous even for a fringe
> > >> use
> > >>>> case
> > >>>>>>>>>> because of
> > >>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>>> number
> > >>>>>>>>>>>>>>>>>>>> of methods that need to return dummy data,
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>> I am still leaning toward a new interface as
> > >> it
> > >>>>>>> abstract
> > >>>>>>>>>> all
> > >>>>>>>>>>>> MM2's
> > >>>>>>>>>>>>>>>>>>>> interaction with Kafka Resources in one
> > >> place;
> > >>>> this
> > >>>>>>> makes
> > >>>>>>>>>> it
> > >>>>>>>>>>>>>> easier
> > >>>>>>>>>>>>>>>>> to
> > >>>>>>>>>>>>>>>>>>>> maintain and make it easier for the use cases
> > >>>> where
> > >>>>>>>>>> customers
> > >>>>>>>>>>>>>> wish to
> > >>>>>>>>>>>>>>>>>>>> provide a different method to handle
> > >> resources.
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>> Omnia
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>> On Tue, Oct 26, 2021 at 4:10 PM Ryanne Dolan
> > >> <
> > >>>>>>>>>>>>>> ryannedo...@gmail.com>
> > >>>>>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>> I like the idea of failing-fast whenever a
> > >>>> custom
> > >>>>>>> impl
> > >>>>>>>> is
> > >>>>>>>>>>>>>>>>> provided, but I
> > >>>>>>>>>>>>>>>>>>>>> suppose that that could be done for Admin
> > >> as
> > >>>>> well.
> > >>>>>> I
> > >>>>>>>>>> agree
> > >>>>>>>>>>>> your
> > >>>>>>>>>>>>>>>>> proposal
> > >>>>>>>>>>>>>>>>>>>> is
> > >>>>>>>>>>>>>>>>>>>>> more ergonomic, but maybe it's okay to
> > >> have a
> > >>>>>> little
> > >>>>>>>>>>>> friction in
> > >>>>>>>>>>>>>>>>> such
> > >>>>>>>>>>>>>>>>>>>>> fringe use-cases.
> > >>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>> Ryanne
> > >>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>> On Tue, Oct 26, 2021, 6:23 AM Omnia
> > >> Ibrahim <
> > >>>>>>>>>>>>>>>>> o.g.h.ibra...@gmail.com>
> > >>>>>>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>> Hey Ryanne, Thanks fo the quick feedback.
> > >>>>>>>>>>>>>>>>>>>>>> Using the Admin interface would make
> > >>>> everything
> > >>>>>>>>>> easier, as
> > >>>>>>>>>>>> MM2
> > >>>>>>>>>>>>>>>>> will
> > >>>>>>>>>>>>>>>>>>>> need
> > >>>>>>>>>>>>>>>>>>>>>> only to configure the classpath for the
> > >> new
> > >>>>>>>>>> implementation
> > >>>>>>>>>>>> and
> > >>>>>>>>>>>>>>>>> use it
> > >>>>>>>>>>>>>>>>>>>>>> instead of AdminClient.
> > >>>>>>>>>>>>>>>>>>>>>> However, I have two concerns
> > >>>>>>>>>>>>>>>>>>>>>> 1. The Admin interface is enormous, and
> > >> the
> > >>>> MM2
> > >>>>>>> users
> > >>>>>>>>>> will
> > >>>>>>>>>>>>>> need
> > >>>>>>>>>>>>>>>>> to know
> > >>>>>>>>>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>>>>>>>> list of methods MM2 depends on and
> > >> override
> > >>>>> these
> > >>>>>>>> only
> > >>>>>>>>>>>>>> instead of
> > >>>>>>>>>>>>>>>>>>>>>> implementing the whole Admin interface.
> > >>>>>>>>>>>>>>>>>>>>>> 2. MM2 users will need keep an eye on any
> > >>>>> changes
> > >>>>>>> to
> > >>>>>>>>>> Admin
> > >>>>>>>>>>>>>>>>> interface
> > >>>>>>>>>>>>>>>>>>>> that
> > >>>>>>>>>>>>>>>>>>>>>> impact MM2 for example deprecating
> > >> methods.
> > >>>>>>>>>>>>>>>>>>>>>> Am not sure if adding these concerns on
> > >> the
> > >>>>> users
> > >>>>>>> is
> > >>>>>>>>>>>>>> acceptable
> > >>>>>>>>>>>>>>>>> or not.
> > >>>>>>>>>>>>>>>>>>>>>> One solution to address these concerns
> > >>> could
> > >>>> be
> > >>>>>>>> adding
> > >>>>>>>>>> some
> > >>>>>>>>>>>>>>>>> checks to
> > >>>>>>>>>>>>>>>>>>>>> make
> > >>>>>>>>>>>>>>>>>>>>>> sure the methods MM2 uses from the Admin
> > >>>>>> interface
> > >>>>>>>>>> exists
> > >>>>>>>>>>>> to
> > >>>>>>>>>>>>>> fail
> > >>>>>>>>>>>>>>>>>>>> faster.
> > >>>>>>>>>>>>>>>>>>>>>> What do you think
> > >>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>> Omnia
> > >>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>> On Mon, Oct 25, 2021 at 11:24 PM Ryanne
> > >>>> Dolan <
> > >>>>>>>>>>>>>>>>> ryannedo...@gmail.com>
> > >>>>>>>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>> Thanks Omnia, neat idea. I wonder if we
> > >>>> could
> > >>>>>> use
> > >>>>>>>> the
> > >>>>>>>>>>>>>> existing
> > >>>>>>>>>>>>>>>>> Admin
> > >>>>>>>>>>>>>>>>>>>>>>> interface instead of defining a new
> > >> one?
> > >>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>> Ryanne
> > >>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>> On Mon, Oct 25, 2021, 12:54 PM Omnia
> > >>>> Ibrahim
> > >>>>> <
> > >>>>>>>>>>>>>>>>>>>> o.g.h.ibra...@gmail.com>
> > >>>>>>>>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>>> Hey everyone,
> > >>>>>>>>>>>>>>>>>>>>>>>> Please take a look at KIP-787
> > >>>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-787%3A+MM2+Interface+to+manage+Kafka+resources
> > >>>>>>>>>>>>>>>>>>>>>>>> <
> > >>>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-787%3A+MM2+Interface+to+manage+Kafka+resources
> > >>>>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>>> Thanks for the feedback and support
> > >>>>>>>>>>>>>>>>>>>>>>>> Omnia
> > >>>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> >
>

Reply via email to