chia7712 merged PR #15569:
URL: https://github.com/apache/kafka/pull/15569
--
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:
chia7712 commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2067222576
The failed test `testParseAndValidateAddressesWithReverseLookup` will get
fixed by #15758. I will merge this PR
--
This is an automated message from the Apache Git Service.
To respond
chia7712 commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2066120621
```
[2024-04-18T17:26:16.149Z] [ant:checkstyle] [ERROR]
chia7712 commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1571142879
##
server-common/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF)
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1571119979
##
server-common/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1571099798
##
server-common/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under
chia7712 commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1571093805
##
server-common/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF)
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2064450248
> > However I'm bit concern that LogConfig seems already huge. What others
prefer? Keep it in KafkaLogConfigs or move them to LogConfig.ServerLogConfig
>
> the most default values
chia7712 commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2064037126
> However I'm bit concern that LogConfig seems already huge. What others
prefer? Keep it in KafkaLogConfigs or move them to LogConfig.ServerLogConfig
the most default values of
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1570406354
##
server/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
mimaison commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1570367941
##
server/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1569436277
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -80,10 +80,10 @@ class ZkAdminManager(val config: KafkaConfig,
private val configHelper = new
mimaison commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1569237791
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -80,10 +80,10 @@ class ZkAdminManager(val config: KafkaConfig,
private val configHelper = new
mimaison commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1569073857
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -79,10 +80,10 @@ class ZkAdminManager(val config: KafkaConfig,
private val configHelper = new
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2059643602
> Let's just let Omnia decides which PR she prefers doing first. Then we can
all review that PR, and once merged move onto the next one. This would avoid
unnecessary churn as I expect
mimaison commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2059050678
Let's just let Omnia decides which PR she prefers doing first. Then we can
all review that PR, and once merged move onto the next one. This would avoid
unnecessary churn as I expect
chia7712 commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2059036032
> hopefully for the last time :)
I feel #15684 will cause many conflicts to this PR, and vice versa :_
--
This is an automated message from the Apache Git Service.
To respond to
fvaleri commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2058739860
@OmniaGM please rebase, hopefully for the last time :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1566156280
##
server/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1566156280
##
server/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
chia7712 commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1566129710
##
server/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
chia7712 commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2050238482
There are build error. please fix it :)
--
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
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2050114827
> maybe we should move the constants out of `LogConfig`
We can try to do this in a follow up after we merge all PRs for KafkaConfig
as am worried about potential circular
chia7712 commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1561034133
##
server/src/main/java/org/apache/kafka/server/config/KafkaLogConfigs.java:
##
@@ -0,0 +1,166 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
chia7712 commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2049705638
> Keep it in KafkaLogConfigs
I prefer this way since `LogConfig` includes too many stuff now ... maybe we
should move the constants out of `LogConfig`
--
This is an automated
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2049673128
The last commit renamed the class to `KafkaLogConfigs` instead similar to
`KafkaSecurityConfigs` and `ZKConfigs`. I tried to move some to
`LogConfig.ServerLogConfig` similar to
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1561000630
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1560999306
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2049642497
> @OmniaGM Is this the next PR in line for moving broker configs?
Yes, I rebased the PR and also pushed a refactor to follow the same pattern
we adopted in
mimaison commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2049236023
@OmniaGM Is this the next PR in line for moving broker configs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2035559842
@fvaleri, @nizhikov @mimaison just rebased the pr and addressed the feedback
will wait for this one to be merged before rebasing the other pr for
KafkaConfig
--
This is an automated
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2035219431
> @OmniaGM I think this is almost ready, but there are a couple of minor
comments to address and some conflicts to fix.
Will be addressing this soon after another pr
--
This is
fvaleri commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2035032922
@OmniaGM I think this is almost ready, but there are a couple of minor
comments to address and some conflicts to fix.
--
This is an automated message from the Apache Git Service.
To
showuon commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2031006912
Since Mickael is already reviewing it, I'll focus on other PRs. Thanks
@mimaison !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on
mimaison commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1543199493
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2024620566
rebased the pr
--
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
mimaison commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2023212508
Now that https://github.com/apache/kafka/pull/15075 got merged, this needs
rebasing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log
mimaison commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541117312
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541026665
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541026665
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541026665
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541032203
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541026665
##
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1540968019
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1540992038
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1540968019
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
mimaison commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1540885105
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1537488997
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
nizhikov commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1535292239
##
build.gradle:
##
@@ -1837,6 +1837,8 @@ project(':storage') {
testImplementation project(':clients').sourceSets.test.output
testImplementation
nizhikov commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1535286350
##
server/src/main/java/org/apache/kafka/server/config/KafkaConfig.java:
##
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or
OmniaGM commented on PR #15569:
URL: https://github.com/apache/kafka/pull/15569#issuecomment-2011427102
@nizhikov can you have a look please?
--
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
OmniaGM opened a new pull request, #15569:
URL: https://github.com/apache/kafka/pull/15569
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade
52 matches
Mail list logo
