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

Change default http port #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xardael
Copy link

@xardael xardael commented Sep 26, 2022

Default port 80 is not a good choice for running in host network, which you probably should when you want to run UPnP or MPD.

@kuzi-moto
Copy link
Member

It's not a bad idea, I just wonder if it should be implemented another way. Changing the default port will cause existing installs to break.

Perhaps there is a way we can modify the port using an environment variable which would modify the port before Apache starts on running the container.

@lachlan-00
Copy link
Member

isn't 80 just the port that the container is running?

I don't really know enough but when i run a docker container i usually redirect 80 to the port i want with the run/compose file

docker run --name=ampache-latest -d -v /path/to/your/music:/media:ro -p 8989:80 ampache/ampache

it hasn't affected any of my internal services using 80 for the container.

@kuzi-moto
Copy link
Member

isn't 80 just the port that the container is running?

I don't really know enough but when i run a docker container i usually redirect 80 to the port i want with the run/compose file

docker run --name=ampache-latest -d -v /path/to/your/music:/media:ro -p 8989:80 ampache/ampache

it hasn't affected any of my internal services using 80 for the container.

Right, but I believe the issue xardael is running into is running Ampache with host networking. When doing so, you're unable to redirect the ports.

@lachlan-00
Copy link
Member

oh, i didn't know you could even do that. well then jut make it default to 80 in the pull would be the easiest way to change that

@KaeTuuN
Copy link

KaeTuuN commented Oct 12, 2023

I would not do that change and decline this MR.

  1. The use case here is a niche and not the standard.
  2. If you are in that niche you should be able to adapt the example compose file.

Greetings Kae

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