-
Notifications
You must be signed in to change notification settings - Fork 390
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
Fix docker: dynamic app.config and dockerhub integration #717
Conversation
@MichaelsJP let me know if you have some suggestions. I guess it'd make sense to do a multi-stage build here too, the image size (even with "slim" Dockerfile) is 1.1 GB. Even though the JDK base image is already 500 MB. As a comparison: OSRM is 100 MB. But really, we only have to keep the |
Argh, that's not right.. Sorry @MichaelsJP, still a glitch. This won't work if I already did it the right way before, then I changed to this and apparently forgot to test it. I'll revert quickly and update. |
It's fixed now @MichaelsJP. Tested more extensively, seems it's working fine now, no matter from which directory |
@nilsnolde For me the build fails. I guess something with the configs but I couldn't figure it out in a short session. Error log:
|
Weird.. You're sure you build an entirely new image and updated the base image @MichaelsJP ? I just tried again (my whole system is pretty new) on Arch with the more recent PRs included and it still works:
and then run the container:
|
I confirmed, all builds and tests fine here too. |
Seems to have been a local issue in my machine. @takb Thx for helping. I think the pr is fine. |
Pull Request Checklist
Fixes #696 , EDIT: fixes #718 as well.
Information about the changes
app.config
are reflected now inside the containerCATALINA/JAVA_OPTS
are set correctlyapp.config
doesn't have to be present when starting a container, which finally makes it independent of this repository and fully available for Dockerhub imagesThings to test
app.config
:/tmp
)app.config.sample
which is now supposed to be mapped to./conf/app.config.sample
BUILD_GRAPHS=True
, which should delete./graphs/*
and regenerate