Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-26 Thread via GitHub
chia7712 merged PR #15775: URL: https://github.com/apache/kafka/pull/15775 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail:

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
OmniaGM commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075595461 > > > Maybe we should rename RaftConfig to QuorumConfig as all configs in QuorumConfig have prefix QUORUM_ > > > This is good point, maybe this can be a followup PR > >

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
chia7712 commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075392820 >> Maybe we should rename RaftConfig to QuorumConfig as all configs in QuorumConfig have prefix QUORUM_ > This is good point, maybe this can be a followup PR After renaming

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
OmniaGM commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075357578 > Maybe we should rename RaftConfig to QuorumConfig as all configs in QuorumConfig have prefix QUORUM_ This is good point, maybe this can be a followup PR -- This is an

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
OmniaGM commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075356299 > It seems org.apache.kafka.raft.KafkaRaftManager needs org.apache.kafka.server.config.KafkaConfig won't happen if the 5 getters are moved, right? Sorry I maybe wan't clear

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
chia7712 commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075340243 > I don't think we should merge them with RaftConfig as RaftConfig should be separate as it is only for the quorum raft and not KRAFT mode which are a bit different in my option.

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
chia7712 commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075295917 > KafkaConfig will move out of core and into server, however, for the time being I don't plan to make raft depend on server for now as far as I can see the only case I might need to do

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
OmniaGM commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075270699 > > If we start to move related kraft classes to raft module like KafkaRaftManager this will be tricky as now org.apache.kafka.raft.KafkaRaftManager needs

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
chia7712 commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075158081 > If we start to move related kraft classes to raft module like KafkaRaftManager this will be tricky as now org.apache.kafka.raft.KafkaRaftManager needs

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
OmniaGM commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075131518 > just curios. Why moving to raft module can cause circle dependencies? Currently raft doesn't depend of core or server but I just fear that mixing quorum raft and KRaft mode related

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
chia7712 commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2075082750 > Personally I think the first option (which keep the pr as it is) is easier to navigate as we know where all the config used by Kafka Raft server (maybe renaming this to

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
OmniaGM commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2074952186 > > Note: We have already RaftConfig but it seems to contain limited amount of configs that only configure controller raft and shouldn't include configs shared by both broker/controller

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
chia7712 commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2074832899 What about moving `KRaftConfig` to raft module? and we can moving all configs from `RaftConfig` to `KRaftConfig`. With those changes, all raft-related configs are in `KRaftConfig`.

Re: [PR] KAFKA-15853: Move KRAFT configs out of KafkaConfig [kafka]

2024-04-24 Thread via GitHub
OmniaGM commented on PR #15775: URL: https://github.com/apache/kafka/pull/15775#issuecomment-2074488094 @chia7712 & @mimaison can you please have a look into this PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use