-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Enhance serve: randomize the port, allow to open a browser
#3497
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
Conversation
|
The flag name |
|
or I think |
| host = config.dev_addr.host | ||
| if port is None: | ||
| port = config.dev_addr.port | ||
| if port is None: | ||
| port = get_random_port(config) |
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.
Does it mean that it won't always start with 8000 anymore? It would be a strong -1 for me.
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.
What do you mean by "start"?
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.
I mean that if I currently have zero server instance running, I expect mkdocs serve to run on port 8000. Then only if I run additional instances, I expect it to use random ports.
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.
Yeah it will not simply pick 8000 anymore. I describe why I think it should be done that way in the top post. To get the old behavior you can pass -p 8000
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.
That will break a lot of existing resources. And not knowing which port will be used each time I run mkdocs serve in a different project will be super annoying IMO. Also if I hardcode -p 8000 in my scripts, then I don't get the retries, and I can't even override it. So it basically just makes me hardcode the value that previously was a default, for no benefit 😕
I recommend not doing that and keeping 8000 as default. It would probably make things more complex (catch the error that the server could not run because the port is already used, and try again on 3 random ports), but I think it's definitely worth it.
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.
As per my prior messages, I think trying several ports is maybe not a good thing.
Instead I want to encourage users to pick a distinct port for each of their sites.
Here is the alternative I propose:
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.
I absolutely second that we should keep 8000 as a default. Please keep in mind that MkDocs is used by many not particularly tech-savvy users. Allowing for just that is a critical factor of its success.
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.
It is seems reasonable to keep other things as much as much as there were as possible, typically to keep the original 8000 port as default (principle of least astonishment); on the other hand, flexibility is always welcome.
It could be that, somewhere, someone wrote a script that expected port 8000 and finds something else (or nothing at all).
However, I would keep in mind that mkdocs serve is a dev/debug command, not a production one (in the command help it is described as Run the builtin development server). I don't see a good reason why anyone would use that command in a dev ops production chain?
If something went wrong, I would assume that someone would quickly notice it and be able to fix it.
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.
The line between development and production environments is not always clear. If gitpod.io provides a default image with MkDocs, and expects and expose the default port 8000, changing it would break gitpod.io images for MkDocs: it's production. Same thing in any company that deploys web IDEs (CodeReady Workspaces for example): the deployed IDE is considered production, yet it's meant for developers, so mkdocs serve will be used there.
It's an easy fix, sure, but "easy fix" on thousands of users is not super friendly 🙂 You'd still make many devs lose between 10 minutes and 1 day of work (probably more in many cases).
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.
An other argument would be: look at what others do. I mentioned Flask and Uvicorn earlier: they do not randomize ports. If the default or specified port is in use, it errors out, and the user must specify another one, simple as that. Well Uvicorn is meant for production, so it's probably not a good comparison, but the Flask server is for development, like us. If anyone knows a tool that retries with randomized ports, please share!
The port is actually not random but is reproducible based on the hash of `site_name` and `site_url`. Bonus: `dev_addr` config can now be just the address, to allow for this default selection behavior to remain.
This is done only for the default "random" choice of port, not when it's explicitly specified
|
After discussing here and elsewhere, my opinion is that anything more than a default port and a way to override it is not worth it. See my comment in this other PR: #3498 (comment). However as long as such a feature is opt-in and does not change the current default behavior, I won't push against it, as it might still be useful for some users. |
That's sounds a good compromise to me. |
|
Sounds good to me, it can just be the first commit of the other PR |
I did not "complain". I just pointed out my concerns regarding the design, which is not consistent. I also explained the rationale behind my thoughts, as it's backed by past experiences talking to users. |
Choose a random port for
serve, add flag to override itThe range is 8000-8099.
The port is actually not random but is reproducible based on the hash of
site_nameandsite_url.Bonus:
dev_addrconfig can now be just the address, to allow for this default selection behavior to remain.Try 3 following ports for
serveif the initial one is takenThis is done only for the default "random" choice of port, not when it's explicitly specified
Closes With
mkdocs serveswitch to another port if the current port is busy #3496I think to resolve this feature request in a good way, both of these actions needed to be done together. Just trying next ports would make a mess if you regularly use 2 sites and they arbitrarily switch port numbers between each other.
So first we make the collisions extremely unlikely in the first place, but each site still has its predictable port number (for example, MkDocs' own site will always be 127.0.0.1:8062). Then still let it try next ports in case it's the same site or a collision happened.