Autobuild is not working when ports are changed

[quentinus95]

If I change the ports, for example:

docker run -itd -v "$(pwd)":/web -p 35729:35729 -p 9000:8000 --name sphinx-server dldl/sphinx-server

(9000 instead of 8000)

Autobuild is not working (errors in console are expecting server to listen on 8000 port).

Maybe there is a way to get the user chosen ports and change it in the server.py file ? (it would be useful to be able to change the port for the websocket too)

If not, maybe add to the yml file two entries to specify the ports (but I don't really like this option as we would create a kind of low end docker compose alternative) ?

[PLoic]

@quentinus95 Can you give me some logs please ?

[quentinus95]

There isn't any logs in fact. Just try to run the server using the above command; autorefresh is expecting the server to be listening on port 8000 instead of 9000 (check out in your browser console).

This is due to this line: https://github.com/dldl/sphinx-server/blob/master/server.py#L85

We have to find a solution to "detect" the user chosen port (in this case: 9000) or add the port to the Yaml config file (but I don't really like this option).

Maybe Docker is defining environment variables with these values ?

[quentinus95]

In fact it is even more complicated... I don't think this can be solved as the internal port should always be 8000, but the external port can changes. Do you have an idea to solve this ?

[quentinus95]

I think it is an issue in Sphinx autobuild, it should use the current server address (in the browser) instead of the configured address.

[quentinus95]

Sorry for spamming, maybe the solution is to change the liveport ? https://github.com/lepture/python-livereload/blob/master/livereload/server.py#L264

[PLoic]

I have some idea, I hope have the time to solve it tonight, I give you some news as soon as possible, I'm on it ;)

[quentinus95]

No emergency :smile:

[quentinus95]

up :trollface:

[PLoic]

To keep our container simple, we do decide to do not support the feature now, but we hope we can integrate a way to do that as soon as possible. We keep the issue as reminder.

[PLoic]

@quentinus95 Do you think we need to integrate a service discovery like Ectd or consul ?

[quentinus95]

I don't really know how these services work. If it is not too overkill, why not :smile:

[andreycizov]

This is a monkey-patched version of the solution: https://github.com/andreycizov/sphinx-server/commit/b72aa12ad16bbec66a2c9bd72bc8de0674eacda3

I'll try to push a similar change directly to python-livereload so that the ports reported to the client would be configurable separately from the listen ports.

[quentinus95]

I see that you use an environment variable OVERRIDE_PORT, does this mean this variable should be used along with the configured port in Docker?

If true I guess we should add this as a configuration in .sphinx-server.yml as stated in this issue, but having a way to detect to exposed port would be cleaner.

@hileef do you have an idea on how to do this? @PLoic wrote about integrating a service discovery but I want to be sure it won't be overkill.

(Edit: note that we can already customize the port in sphinx-autobuild https://pypi.org/project/sphinx-autobuild/)

[andreycizov]

If it was ready for release I would have sent a pull request - we need to wait till my changes are merged into python-livereload so that we would not need to monkey-patch the upstream. I shared that so that anyone who stumbles upon this issue could see that there's a working solution.

I see that you use an environment variable OVERRIDE_PORT, does this mean this variable should be used along with the configured port in Docker?

Yes.

If true I guess we should add this as a configuration in .sphinx-server.yml as stated in this issue, but having a way to detect to exposed port would be cleaner.

I am pretty sure you can't do that in the general scenario. Not on the Docker side, and not on the HTTP server side (python-livereload generates static HTML before it's served, so it can't use the port number supplied by the client).

[hileef]

Hi all !

To the best of my understanding, it is currently not possible to obtain the port mapping from within the container itself without obtaining access to the docker socket or API, which implies a significant security risk and should not be forced onto the user.

When using container orchestration, such as kubernetes or mesos for example, it is possible to obtain port mappings within the container through automatically provisioned environment variables, however this approach is specific for each orchestrator, and again, a choice of orchestrator should not be forced onto the user.

It is a feature that has been requested on the docker engine itself for a multitude of use cases however, and work has been done to implement such a feature, but none have been merged so far : see :

• EXPERIMENTAL new mount type: introspection
• Detect exposed ports from inside container
• Introspection of external port
• Proposal: container introspection

So it seems that this feature may be coming in the future from the docker engine itself.

Using service discovery would work, however it would require changes/overrides to the livereload code as well as requiring more containers, implying additional configuration work which should not be forced onto the user. So I think it would be overkill, especially given the fact that this container is supposed to be able to work as standalone.

---

I believe the main problem for resolving this issue comes from the fact that the livereload code exposes a port number (two actually) directly to the web client, which is problematic for a number of use cases (SDN, proxies, TLS terminators, etc.).

This shouldn't be required. The web client does have knowledge of the port through which it communicates, which can be different from the one the server is actually listening to. Furhtermore, it is possible to expose a websocket and an http endpoint on the same port, namespacing them by path, such that the same port which is visible to the web client can be used for both protocols.

As @andreycizov has proposed, I believe that the best course of action would be to submit PRs to python-livereload, or even fork it, and implement changes. I have these ideas in mind:

• add flags to the livereload server (such as, for example override_endpoint_bywebclient and override_endpoint_namespace_ws_and_http)
• the javascript should obtain the endpoints configuration (domain, protocol (TLS or not), port number) directly from the web client if a flag is set
• the websocket endpoint and the http endpoint should be provided on the same port and namespaced by path (for example : '/ws' and '/live') if a flag is set

If these changes are implemented, this would mean that :

• the user has only one port to bind -p 8000:<whatever>
• the user does not have to configure environment variables for the port mapping
• the user can have multiple instances of the container running without encountering port problems such as "port 35729 already in use"

What do you guys think ? Would this be a good idea ?

[andreycizov]

the javascript should obtain the endpoints configuration (domain, protocol (TLS or not), port number) directly from the web client if a flag is set

@hileef I like that one. Should be even easier than the solution with the OVERRIDE_PORT.

[PLoic]

@quentinus95 I think deploy a discovery system is little bit overkill, the ideas proposed by @hileef looks alright

[quentinus95]

@hileef Based on your answer, I guess the best scenario would be:

• to serve the websocket and http connections over the same port
• to retrieve the target port based on the current url directly on the frontend

This solves all the issues with no configuration or hacky things in the container, and allow customizing the port and running multiple sphinx-server at the same time.

[andreycizov]

Here's the monkey-patched version of it: https://github.com/andreycizov/sphinx-server/commit/055692d74cfcae08e0d762c95051e6c7956d34f9

I'll add the modifications to python-livereload soon enough. They don't seem to be too responsive unfortunately.

[andreycizov]

@hileef, wouldn't it be easier (in terms of coordinating between projects) to allow for the user of the docker container to instead provide the port through argv at that line, which is currently a constant: https://github.com/dldl/sphinx-server/blob/04e33f777837f3e4ad5788597f8940e70298106e/server.py#L91

This way you could start this container that way: docker run -itd -v "$(pwd)":/web -p 8948:8948 --name sphinx-server dldl/sphinx-server 127.0.0.1 8948

Otherwise I am still finishing the necessary merge required in python-livereload.

[quentinus95]

Won't this conflict with #29?

[andreycizov]

@quentinus95 Nothing stops us from partially consuming sys.argv in the presence of a flag that is not matched by the SphinxBuilder interface. I suggest something longer along the lines of --bind_address which will probably never be matched by the downstream because that interface does not include server binding anyway.

@hileef The issue with client-side guessing is that livereload.Server provides two arguments for ws and non-ws ports. So the original author of livereload is (as of now) against merging the commit because we can not handle that condition on the client-side. See here: https://github.com/lepture/python-livereload/pull/182

[hileef]

I have done some tests with the dldl/sphinx-server and lepture/python-livereload codebases. It seems that there are some things we have missed and/or misunderstood.

@quentinus95 , The documentation for dldl/sphinx-server states that with autobuild enabled, a websocket will be listening on port 35729 to effect the page refresh polling. That is incorrect (try running netstat in the container).

The websocket listens on the liveport, which defaults to the normal port if not defined. Current configuration for dldl/sphinx-server is port=8000, liveport=None; with this configuration the websocket and http endpoints both listen on the 8000 port (which is great!).

---

@andreycizov , I have looked at the lepture/python-livereload#182 pull request, and I believe that @lepture is correct in stating that when liveport is not None, the port for the live_reload_path should be the liveport.

When the liveport is set : ws and http traffic must be seperated (for browser plugins, for example), therefore it is not possible to guess from the client side the 2 ports to use (config becomes prioritary).

When the liveport is not set : both ws and http traffic pass through the same port, which the web client can see from its URL.

In order for @lepture to accept your pull request I believe you could do some small changes such as, for example :

        if liveport is None:
            liveport = port
        else:
            port_aquisition_by_client = False

        if port_aquisition_by_client:
            acquire_port = '" + window.location.port + "'
            live_reload_path = ":{port}/livereload.js?port={port}".format(port=acquire_port)

        elif liveport == 80 or liveport == 443:
            live_reload_path = "/livereload.js?port={port}".format(port=liveport)

        else:
            live_reload_path = ":{port}/livereload.js?port={port}".format(port=liveport)

as well as the port_aquisition_by_client flag in Server.application and Server.serve.

I have tested this and was able to run 2 dldl/sphinx-server docker containers, one with -p 7949:8000 and another with -p 5252:8000, with the livereload functionality working as intended (including the websockets), which is awesome :D

---

Meanwhile, as an answer to your previous question @andreycizov , I think the addition of a command line argument would be a reasonable workaround until your pull request is merged into lepture/python-livereload, but I would ask @quentinus95 for confirmation first.

[hileef]

On a side note, thanks to you @andreycizov for helping us discover the incorrect documentation and all the work you have done so far on the lepture/python-livereload pull request 👍

[quentinus95]

Now that everything is clear, I guess we can wait for lepture/python-livereload#182 to be corrected and merged. Thanks for the investigation!

[andreycizov]

@quentinus95 @hileef the change had been merged, but in a different way: if you do not supply the liveport, then the port defaults to window.location.port anyway. I don't think the change had yet been released as a separate python package, but it should be fine in the future with no changes made in the context of this repository.

See the discussion here again, here: https://github.com/lepture/python-livereload/pull/182

[hileef]

That's great news !

So we can expect to close this issue once python-livereload releases a new version (I'm assuming 2.6.0) and sphinx-autobuild updates its requirements.txt dependencies.

No more changes are required for this codebase ( except the documentation @quentinus95 ). Thanks again @andreycizov 👍

[quentinus95]

I've just corrected the README.md file. We'll remove the limitation It is not possible to customize ports when autobuild is enabled when everything is merged on python-livereload and requirements are updated in this repo.

[quentinus95]

Fixed in #31.