-
Notifications
You must be signed in to change notification settings - Fork 682
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.
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
Fixed update_run.sh path and permission #84
Conversation
Wouldn't start on my Mac, probably needs the absolute path to the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbreevoort thanks for the PR!
@@ -27,8 +27,8 @@ services: | |||
KAFKA_CONTROLLER_LISTENER_NAMES: 'CONTROLLER' | |||
KAFKA_LOG_DIRS: '/tmp/kraft-combined-logs' | |||
volumes: | |||
- ./update_run.sh:/tmp/update_run.sh | |||
command: "bash -c 'if [ ! -f /tmp/update_run.sh ]; then echo \"ERROR: Did you forget the update_run.sh file that came with this docker-compose.yml file?\" && exit 1 ; else /tmp/update_run.sh && /etc/confluent/docker/run ; fi'" | |||
- ${PWD}/update_run.sh:/tmp/update_run.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbreevoort good catch
- ./update_run.sh:/tmp/update_run.sh | ||
command: "bash -c 'if [ ! -f /tmp/update_run.sh ]; then echo \"ERROR: Did you forget the update_run.sh file that came with this docker-compose.yml file?\" && exit 1 ; else /tmp/update_run.sh && /etc/confluent/docker/run ; fi'" | ||
- ${PWD}/update_run.sh:/tmp/update_run.sh | ||
command: "bash -c 'if [ ! -f /tmp/update_run.sh ]; then echo \"ERROR: Did you forget the update_run.sh file that came with this docker-compose.yml file?\" && exit 1 ; else chmod +x /tmp/update_run.sh && /tmp/update_run.sh && /etc/confluent/docker/run ; fi'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbreevoort Can you please elaborate why this should be required? Shouldn't the permissions be inherited from the file, which should already be executable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saved the files first, my bad. After checkout the repo it works... my bad. Keep up the good work removing zookeeper :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably ${PWD}
isn't needed either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbreevoort that mean your PR is no more necessary ?
@raphaelauv PR is not needed anymore, works as designed and will close this issue and PR |
If somebody else will come here searching why their |
Wouldn't start on my Mac, probably needs the absolute path to the file
Description
Fixed path to update_run.sh (couldn't mount the relative path)
Permissions fix when run the script
Author Validation
After this fix
docker-compose up
starts correct.Reviewer Tasks
Test if
docker-compose up
still starts