Skip to content

Conversation

@oprypin
Copy link
Contributor

@oprypin oprypin commented Dec 2, 2023

  • Choose a random port for serve, add flag to override it
    The range is 8000-8099.
    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.

  • Try 3 following ports for serve if the initial one is taken
    This is done only for the default "random" choice of port, not when it's explicitly specified

    Closes With mkdocs serve switch to another port if the current port is busy #3496

I 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.

@oprypin
Copy link
Contributor Author

oprypin commented Dec 2, 2023

The flag name serve --open,-o is up for discussion. Sadly -w is taken.

@trel
Copy link
Contributor

trel commented Dec 2, 2023

or -b/--browser ?

I think -o is not bad.

Comment on lines 58 to 64
host = config.dev_addr.host
if port is None:
port = config.dev_addr.port
if port is None:
port = get_random_port(config)
Copy link
Contributor

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.

Copy link
Contributor Author

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"?

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

@pawamoy pawamoy Dec 2, 2023

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.

Copy link
Contributor Author

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:

Copy link
Contributor

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.

Copy link

@fralau fralau Dec 3, 2023

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.

Copy link
Contributor

@pawamoy pawamoy Dec 3, 2023

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).

Copy link
Contributor

@pawamoy pawamoy Dec 3, 2023

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
@pawamoy
Copy link
Contributor

pawamoy commented Dec 4, 2023

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.

@fralau
Copy link

fralau commented Dec 4, 2023

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.(...) 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.

@oprypin
Copy link
Contributor Author

oprypin commented Dec 4, 2023

Sounds good to me, it can just be the first commit of the other PR
but keep in mind that @squidfunk also complained about the suggested ability to override the port
#3498 (review)

@squidfunk
Copy link
Contributor

keep in mind that @squidfunk also complained about the suggested ability to override the port

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.

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.

With mkdocs serve switch to another port if the current port is busy

6 participants