You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "frankvicky (via GitHub)" <gi...@apache.org> on 2025/01/05 06:41:48 UTC
[PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
frankvicky opened a new pull request, #18389:
URL: https://github.com/apache/kafka/pull/18389
JIRA: KAFKA-18229
We should move configs out of the `kraft` directory since AK no longer relies on ZK.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1913603814
##########
bin/kafka-storage.sh:
##########
@@ -14,4 +14,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+for i in $(seq 1 $#); do
+ if [ "${!i}" = "-c" ] || [ "${!i}" = "--config" ]; then
+ next_i=$((i + 1))
+ [[ "${!next_i}" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
Review Comment:
Deprecated would imply we still support it, but plan to remove it in the future. It looks like we are doing a move here. Perhaps you want to say that they were moved to a new location and state that new location?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1916941023
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
You're right, we download Kafka 3.7.0 above. `reconfig-server.properties` did not exist in 3.7.0, so this must be broken at the moment. So we need your change.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1918483228
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
The docker folder allows developer download different version of Kafka but different version use different config or folder path.
This is painful for maintenance, should we keep this design?
In my mind, we just need to run docker on current version instead of all versions, it can simply every thing.
If developer want to use specific version, he/she can checkout to that version.
Related jira https://issues.apache.org/jira/browse/KAFKA-18534
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1920052801
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: The 'config/kraft' path has been removed. Now all configuration files are in 'config'."
Review Comment:
@chia7712 @ijuma @jsancio Should we have these warnings? WDYT?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2602548404
Hi @mimaison
Sorry for the inconvenience, I will keep that in mind.
Thanks for the review. 🙇🏼
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1916884845
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: The 'config/kraft' path has been removed. Now all configuration files are concentrated in 'config'."
Review Comment:
I guess we can remove `concentrated`
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
We need to remove `kraft` from the path here
##########
docker/jvm/jsa_launch:
##########
@@ -17,9 +17,9 @@
KAFKA_CLUSTER_ID="$(opt/kafka/bin/kafka-storage.sh random-uuid)"
TOPIC="test-topic"
-KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/kraft/reconfig-server.properties
+KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/server.properties
Review Comment:
Not related to this PR but it's a bit odd we don't use absolute paths like `/opt/...` in this script.
##########
bin/kafka-storage.sh:
##########
@@ -14,4 +14,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+for i in $(seq 1 $#); do
+ if [ "${!i}" = "-c" ] || [ "${!i}" = "--config" ]; then
+ next_i=$((i + 1))
+ [[ "${!next_i}" == *"config/kraft"* ]] && echo "Warning: The 'config/kraft' path has been removed. Now all configuration files are concentrated in 'config'."
Review Comment:
ditto
##########
docs/upgrade.html:
##########
@@ -75,6 +75,8 @@ <h5><a id="upgrade_400_notable" href="#upgrade_400_notable">Notable changes in 4
</li>
<li>The function <code>onNewBatch</code> in <code>org.apache.kafka.clients.producer.Partitioner</code> class was removed.
</li>
+ <li>The default properties files for KRaft mode are no longer stored in the separate <code>config/kraft</code> directory since Zookeeper has been removed. These files have been consolidated with other configuration files.
Review Comment:
Just to be extra clear, we can explicitly tell these files are under config
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1913649620
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
+ break
+ fi
+done
Review Comment:
No, the config files are moving to a different path.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1916982368
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
Glad it was a lucky guess that turned out to be the right fix! 😅
I just checked the branch 3.7 and I understand now.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison merged PR #18389:
URL: https://github.com/apache/kafka/pull/18389
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2588509264
Hi @jsancio,
I have read the ticket description and agree it would be good to address KAFKA-18497 in this PR.
I will update the patch in the next commit. 🐱
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1916932014
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
I shouldn't change this one. This dockfile somehow is only for AK 3.7.
We should revert the change. WDYT ?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1917896006
##########
docker/jvm/jsa_launch:
##########
@@ -17,9 +17,9 @@
KAFKA_CLUSTER_ID="$(opt/kafka/bin/kafka-storage.sh random-uuid)"
TOPIC="test-topic"
-KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/kraft/reconfig-server.properties
+KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/server.properties
Review Comment:
https://issues.apache.org/jira/browse/KAFKA-18564
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2621706672
@frankvicky Not sure if it's already tracked but shall we fix the `reconfig-server` usage on `kafka-site` as well? https://github.com/apache/kafka-site/blob/4605cf3a58b33c1715c70bbf52ac424ad0c224c7/39/quickstart.html#L62
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1913603814
##########
bin/kafka-storage.sh:
##########
@@ -14,4 +14,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+for i in $(seq 1 $#); do
+ if [ "${!i}" = "-c" ] || [ "${!i}" = "--config" ]; then
+ next_i=$((i + 1))
+ [[ "${!next_i}" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
Review Comment:
Deprecated would imply we still support it. Perhaps you want to say that they were moved to a new location and state that new location?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1917888706
##########
docker/jvm/jsa_launch:
##########
@@ -17,9 +17,9 @@
KAFKA_CLUSTER_ID="$(opt/kafka/bin/kafka-storage.sh random-uuid)"
TOPIC="test-topic"
-KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/kraft/reconfig-server.properties
+KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/server.properties
Review Comment:
I see. I will file a jira for this issue. 😺
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1913979735
##########
bin/kafka-storage.sh:
##########
@@ -14,4 +14,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+for i in $(seq 1 $#); do
+ if [ "${!i}" = "-c" ] || [ "${!i}" = "--config" ]; then
+ next_i=$((i + 1))
+ [[ "${!next_i}" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
Review Comment:
Yes, we should remind users that config/kraft is not used anymore after 4.0.0
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1922426124
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: The 'config/kraft' path has been removed. Now all configuration files are in 'config'."
Review Comment:
Printing warnings might be awkward in this scenario, as it can be difficult to determine whether the configuration file originates from the 'removed' folder. Users might continue to utilize the path `config/kraft`.
Maybe we can just remove the `config/kraft`.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1913617063
##########
bin/kafka-storage.sh:
##########
@@ -14,4 +14,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+for i in $(seq 1 $#); do
+ if [ "${!i}" = "-c" ] || [ "${!i}" = "--config" ]; then
+ next_i=$((i + 1))
+ [[ "${!next_i}" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
Review Comment:
Why is this needed to address the intent of the Jira? It is odd for a script to be checking the path of a file. The CLI should only be concern with the content of the file.
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
+ break
+ fi
+done
Review Comment:
Why is this needed to address the intent of the Jira? It is odd for a script to be checking the path of a file. The CLI should only be concern with the content of the file.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1918732928
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: The 'config/kraft' path has been removed. Now all configuration files are in 'config'."
Review Comment:
Should the pattern be `*"config/kraft/"*`? Otherwise you get the message printed if your config file is called `kraft.properties` for example.
If so let's do it in `bin/kafka-storage.sh` too.
I'm actually wondering if we need this warning at all. Users could get this message if their configuration files are in custom paths. What do the other committers think?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1918784490
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: The 'config/kraft' path has been removed. Now all configuration files are in 'config'."
Review Comment:
I agree on the trailing slash pattern. This would better distinguish directories from files like `kraft*.properties.`
I will update it soon.
As for the warning, I also don't think it is necessary since the user might put their config files wherever they want. But I'm open for this solution. If we eventually want to keep this warning, we could make it more informative, as mentioned before, it might catch the user custom path.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1918483228
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
The docker folder allows developer download different version of Kafka but different version use different config or folder path.
This is painful for maintenance, should we keep this design?
Related jira https://issues.apache.org/jira/browse/KAFKA-18534
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1918491119
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
Hi @TaiJuWu
I will file a jira to raise a discussion about 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 the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2602539978
Because you squashed all your commits, it's hard to tell what has changed. As far as I can tell, you just removed the warning messages from the scripts. I think that makes sense, I could see them being more confusing than helpful.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2586099249
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "jsancio (via GitHub)" <gi...@apache.org>.
jsancio commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1913637799
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
+ break
+ fi
+done
Review Comment:
The breaking change being that `process.roles` must be provided? In 3.x `process.roles` was optional but in 4.x that property must be specified since ZK is not supported and KRaft is the only valid configuration.
If so, let `KafkaConfig`'s schema validation catch invalid configurations and warn the user.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1917586599
##########
docker/jvm/jsa_launch:
##########
@@ -17,9 +17,9 @@
KAFKA_CLUSTER_ID="$(opt/kafka/bin/kafka-storage.sh random-uuid)"
TOPIC="test-topic"
-KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/kraft/reconfig-server.properties
+KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/server.properties
Review Comment:
Yes, it is weird. We should use absolute paths here especially since it's for containers. 🤔
I will update it in next commit.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2621715144
> @frankvicky Not sure if it's already tracked but shall we fix the `reconfig-server` usage on `kafka-site` as well? https://github.com/apache/kafka-site/blob/4605cf3a58b33c1715c70bbf52ac424ad0c224c7/39/quickstart.html#L62 cc: @mimaison
Ahhh, the current generated one is for 3.9 on kafka site.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "askew (via GitHub)" <gi...@apache.org>.
askew commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2610692838
When will an updated archive file be published with these changes? Currently I cannot build the docker image as the archive does not have the new file structure.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1922472554
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: The 'config/kraft' path has been removed. Now all configuration files are in 'config'."
Review Comment:
It seems that most people consider not leaving this warning (as it's not 100% accurate), so I have removed them.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1917883881
##########
docker/jvm/jsa_launch:
##########
@@ -17,9 +17,9 @@
KAFKA_CLUSTER_ID="$(opt/kafka/bin/kafka-storage.sh random-uuid)"
TOPIC="test-topic"
-KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/kraft/reconfig-server.properties
+KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=storage.jsa" opt/kafka/bin/kafka-storage.sh format --standalone -t $KAFKA_CLUSTER_ID -c opt/kafka/config/server.properties
Review Comment:
Sorry I wasn't very clear but I think it would make more sense to address that in a different PR as that's is not related to the config files moving.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "TaiJuWu (via GitHub)" <gi...@apache.org>.
TaiJuWu commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1918483228
##########
docker/docker_official_images/3.7.0/jvm/Dockerfile:
##########
@@ -78,7 +78,7 @@ RUN set -eux ; \
chmod -R ug+w /etc/kafka /var/lib/kafka /etc/kafka/secrets; \
cp /opt/kafka/config/log4j.properties /etc/kafka/docker/log4j.properties; \
cp /opt/kafka/config/tools-log4j.properties /etc/kafka/docker/tools-log4j.properties; \
- cp /opt/kafka/config/kraft/reconfig-server.properties /etc/kafka/docker/server.properties; \
+ cp /opt/kafka/config/kraft/server.properties /etc/kafka/docker/server.properties; \
Review Comment:
The docker folder allows developer download different version of Kafka but different version use different config or folder path.
This is painful for maintenance, should we keep this design?
Or we just need to run docker on current version instead of all versions?
Related jira https://issues.apache.org/jira/browse/KAFKA-18534
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #18389:
URL: https://github.com/apache/kafka/pull/18389#issuecomment-2607475721
Applied to 4.0 too: https://github.com/apache/kafka/commit/ac87155ba46d3135256ea00c34e80df3e00c4575
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18229: Move configs out of "kraft" directory [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18389:
URL: https://github.com/apache/kafka/pull/18389#discussion_r1913631544
##########
bin/kafka-server-start.sh:
##########
@@ -19,6 +19,14 @@ then
echo "USAGE: $0 [-daemon] server.properties [--override property=value]*"
exit 1
fi
+
+for arg in "$@"; do
+ if [[ "$arg" == *.properties ]]; then
+ [[ "$arg" == *"config/kraft"* ]] && echo "Warning: If using default properties file, config/kraft path has been deprecated"
+ break
+ fi
+done
Review Comment:
We are making a breaking change without a deprecation cycle, so this is trying to provide a warning related to that. I haven't checked if the warning is effective though - good to think through that.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org