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.

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

Add LavalinkServer Dockerfile #74

Merged
merged 3 commits into from
Mar 6, 2018
Merged

Add LavalinkServer Dockerfile #74

merged 3 commits into from
Mar 6, 2018

Conversation

itslukej
Copy link
Contributor

This is to be built assuming you have compiled the jars with gradle initally, as discussed with Napster this could be used for an official docker image in the future.

docker build -f LavalinkServer/docker/Dockerfile -t lavalink .

@freyacodes
Copy link
Member

I believe it is bad practice to hardcode the config values into the image

@itslukej
Copy link
Contributor Author

I have put the defaults into the dockerfile, they are meant to be overidden using the -e flag on docker run e.g docker run -e LAVALINK_SERVER_PASSWORD="youshallpass" lavalink. My mistake for not mentioning that

@freyacodes
Copy link
Member

Better practice would be to use volumes

@schnapster
Copy link
Contributor

schnapster commented Feb 17, 2018

The goal here was to use OS environment vars as the main source of the configuration.
Seeing that OS environment vars override any application.yaml, yeah, packaging the application.yml.example as application.yml and exposing it as a volume could be an option to bring both worlds together.

@iCrawl
Copy link

iCrawl commented Feb 18, 2018

I disagree that volumes are a better practices. Docker containers should not have an open link to the host if its not needed. And considering Lavalink is pretty stateless in itself it would be a waste.

@freyacodes freyacodes merged commit 3c54c05 into lavalink-devs:dev Mar 6, 2018
freyacodes pushed a commit to freyacoded/repo-test that referenced this pull request Oct 11, 2022
* Add LavalinkServer dockerfile

* Raise Xmx value

* Remove ENV Vars
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants