Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MINOR: fix the README and 3.7 jsa_launch command #18300

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open

MINOR: fix the README and 3.7 jsa_launch command #18300

wants to merge 4 commits into from
+4 −4

Conversation

m1a2st
Copy link
Contributor

@m1a2st m1a2st commented Dec 22, 2024 •
edited
Loading

  1. In kafka README, we use Kraft standalone command to start the server, but the docker image is KAFKA 3.7.0, this would be misunderstanding user, so we should update it to 3.9.
  2. the jsa_launch 3.7 also has the same typo

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community docker Official Docker image small Small PRs labels Dec 22, 2024
@m1a2st m1a2st changed the title MINOR: fix the README and docker command error MINOR: fix the README and 3.7 jsa_launch command Dec 22, 2024
ijuma
ijuma reviewed Dec 23, 2024
View reviewed changes
README.md Outdated
@@ -106,7 +106,7 @@ Using compiled files:

Using docker image:

docker run -p 9092:9092 apache/kafka:3.7.0
docker run -p 9092:9092 apache/kafka:3.9.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, can we use an alias like latest instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed it

ijuma
ijuma reviewed Dec 23, 2024
View reviewed changes
docker/docker_official_images/3.7.0/jvm/jsa_launch

KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=kafka.jsa" opt/kafka/bin/kafka-server-start.sh opt/kafka/config/kraft/reconfig-server.properties &
KAFKA_JVM_PERFORMANCE_OPTS="-XX:ArchiveClassesAtExit=kafka.jsa" opt/kafka/bin/kafka-server-start.sh opt/kafka/config/kraft/server.properties &
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsancio @cmccabe Thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice find. @m1a2st Could you please update Dockerfile as well? the file uses the nonexistent config reconfig-server.properties :(

@github-actions github-actions bot removed the triage PRs from the community label Dec 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chia7712 chia7712 chia7712 left review comments

@ijuma ijuma ijuma left review comments

Assignees
No one assigned
Labels
ci-approved docker Official Docker image small Small PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@m1a2st @ijuma @chia7712