KAFKA-18229: Move configs out of "kraft" directory #18389
Conversation
github-actions
bot
added
triage
frankvicky
force-pushed
the
KAFKA-18229
branch
2 times, most recently
from
dc04d08 to
78efb6d
Compare
frankvicky
force-pushed
the
KAFKA-18229
branch
from
78efb6d to
4408afe
Compare
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
frankvicky
force-pushed
the
KAFKA-18229
branch
from
4408afe to
c85a930
Compare
bin/kafka-storage.sh
Outdated
| 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" |
There was a problem hiding this 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?
There was a problem hiding this 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.
There was a problem hiding this comment.
Yes, we should remind users that config/kraft is not used anymore after 4.0.0
There was a problem hiding this comment.
Thanks for the changes @frankvicky
I also filed this issue https://issues.apache.org/jira/browse/KAFKA-18497. Do you want to address that issue in this PR too? It basically asks to move config/reconfig-server.properties to override config/server.properties. What do you think?
bin/kafka-storage.sh
Outdated
| 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" |
There was a problem hiding this 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
Outdated
| 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 |
There was a problem hiding this 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.
There was a problem hiding this 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.
There was a problem hiding this 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.
There was a problem hiding this comment.
No, the config files are moving to a different path.
|
Hi @jsancio, I have read the ticket description and agree it would be good to address KAFKA-18497 in this PR. |
frankvicky
force-pushed
the
KAFKA-18229
branch
from
c85a930 to
9727c3f
Compare
github-actions
bot
removed
triage
frankvicky
force-pushed
the
KAFKA-18229
branch
from
9727c3f to
ec210d4
Compare
frankvicky
force-pushed
the
KAFKA-18229
branch
from
ec210d4 to
66bc4fd
Compare
JIRA: KAFKA-18229
We should move configs out of the
kraftdirectory since AK no longer relies on ZK.Committer Checklist (excluded from commit message)