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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to