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 internal container ports 2.0 #181

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jpfleischer
Copy link
Contributor

NOTE: The previous pull request was closed due to an accidental commit.

The intention of this PR is for the docker logs to tell me straightaway the right port to connect to (instead of having to remember that the right port is 47962).

@shankari
Copy link
Contributor

shankari commented Jan 4, 2025

@Abby-Wheelis @jpfleischer I am finally getting caught up with code reviews in the plane on the way to DC, and I am not sure if/how this works. I don't see any "Testing done" in the comments, and I don't see how this mapping will fix anything, given that the http server is still starting at port 6060

The intent behind the change is good, and there is a way to make it work, but I don't think this is it.

EXPOSE 6060
CMD ["http-server", "-p", "6060"]

Unless one of you can show that it works, or plans to fix this as a way to better understand docker, I am going to close this PR.

@jpfleischer
Copy link
Contributor Author

Hi all,
Currently the documentation says

Go to http://localhost:3274/ to see the front-end. Note that the port is 3274 instead of the 8080 in the logs, since we remap it as part of the docker-compose.

My PR changes the logs to say 3274 because it changes the http-server to serve on port 3274. In main it serves on 6060.

The port 6060 is arbitrarily taken from an example prg and can be changed. This makes it more straightforward.

@Abby-Wheelis
Copy link
Member

Log messages I see when I run

for the notebooks
em-public-dashboard-test-notebook-server-1  | For more information see https://docs.conda.io/projects/conda/en/stable/user-guide/configuration/use-condarc.html
em-public-dashboard-test-notebook-server-1  | 
em-public-dashboard-test-notebook-server-1  |   deprecated.topic(
em-public-dashboard-test-notebook-server-1  | # conda environments:
em-public-dashboard-test-notebook-server-1  | #
em-public-dashboard-test-notebook-server-1  | base                     /root/miniconda-23.5.2
em-public-dashboard-test-notebook-server-1  | emission              *  /root/miniconda-23.5.2/envs/emission
em-public-dashboard-test-notebook-server-1  | 
em-public-dashboard-test-notebook-server-1  | Running notebook in docker, change host:port to localhost:47962 in the URL below
em-public-dashboard-test-notebook-server-1  | [I 03:55:34.119 NotebookApp] Writing notebook server cookie secret to /root/.local/share/jupyter/runtime/notebook_cookie_secret
em-public-dashboard-test-notebook-server-1  | [W 03:55:35.978 NotebookApp] Loading JupyterLab as a classic notebook (v6) extension.
em-public-dashboard-test-notebook-server-1  | [C 03:55:35.979 NotebookApp] You must use Jupyter Server v1 to load JupyterLab as notebook extension. You have v2.14.2 installed.
em-public-dashboard-test-notebook-server-1  |     You can fix this by executing:
em-public-dashboard-test-notebook-server-1  |         pip install -U "jupyter-server<2.0.0"
em-public-dashboard-test-notebook-server-1  | [I 03:55:35.982 NotebookApp] Serving notebooks from local directory: /usr/src/app/saved-notebooks
em-public-dashboard-test-notebook-server-1  | [I 03:55:35.982 NotebookApp] Jupyter Notebook 6.5.7 is running at:
em-public-dashboard-test-notebook-server-1  | [I 03:55:35.982 NotebookApp] http://486a99408728:47962/?token=ee8f08f4308860c02d14dea84695998e4d4a389205f3adc9
em-public-dashboard-test-notebook-server-1  | [I 03:55:35.982 NotebookApp]  or http://127.0.0.1:47962/?token=ee8f08f4308860c02d14dea84695998e4d4a389205f3adc9
em-public-dashboard-test-notebook-server-1  | [I 03:55:35.983 NotebookApp] Use Control-C to stop this server and shut down all kernels (twice to skip confirmation).
em-public-dashboard-test-notebook-server-1  | [C 03:55:35.990 NotebookApp] 
em-public-dashboard-test-notebook-server-1  |     
em-public-dashboard-test-notebook-server-1  |     To access the notebook, open this file in a browser:
em-public-dashboard-test-notebook-server-1  |         file:///root/.local/share/jupyter/runtime/nbserver-22-open.html
em-public-dashboard-test-notebook-server-1  |     Or copy and paste one of these URLs:
em-public-dashboard-test-notebook-server-1  |         http://486a99408728:47962/?token=ee8f08f4308860c02d14dea84695998e4d4a389205f3adc9
em-public-dashboard-test-notebook-server-1  |      or http://127.0.0.1:47962/?token=ee8f08f4308860c02d14dea84695998e4d4a389205f3adc9

Copy pasted url works, same as when I used to change it manually. There is both a message to change the port and the port is changed in the log message. We probably only need one or the other of those things.

for the dashboard
em-public-dashboard-test-dashboard-1        | Starting up http-server, serving ./
em-public-dashboard-test-dashboard-1        | 
em-public-dashboard-test-dashboard-1        | http-server version: 14.1.1
em-public-dashboard-test-dashboard-1        | 
em-public-dashboard-test-dashboard-1        | http-server settings: 
em-public-dashboard-test-dashboard-1        | CORS: disabled
em-public-dashboard-test-dashboard-1        | Cache: 3600 seconds
em-public-dashboard-test-dashboard-1        | Connection Timeout: 120 seconds
em-public-dashboard-test-dashboard-1        | Directory Listings: visible
em-public-dashboard-test-dashboard-1        | AutoIndex: visible
em-public-dashboard-test-dashboard-1        | Serve GZIP Files: false
em-public-dashboard-test-dashboard-1        | Serve Brotli Files: false
em-public-dashboard-test-dashboard-1        | Default File Extension: none
em-public-dashboard-test-dashboard-1        | 
em-public-dashboard-test-dashboard-1        | Available on:
em-public-dashboard-test-dashboard-1        |   http://127.0.0.1:6060
em-public-dashboard-test-dashboard-1        |   http://172.18.0.3:6060
em-public-dashboard-test-dashboard-1        | Hit CTRL-C to stop the server
But neither of those links worked, and neither does clicking the link in Docker Desktop which is what I usually do. @jpfleischer were you able to work with both the notebooks and the dashboard?

@jpfleischer
Copy link
Contributor Author

jpfleischer commented Jan 5, 2025

Hi Abby,

I am getting some different output in my branch called port, with the correct port for the dashboard:

dashboard-1        | Available on:
dashboard-1        |   http://127.0.0.1:3274
dashboard-1        |   http://172.21.0.4:3274

The first one works, but just like main, the second link won't work as it is using the docker container's hostname which won't be accessible on host. I tried looking for a solution for http-server to stop printing that second useless link but it doesn't exist. Still, the advantage is that the user will see the correct port from the get-go and does not need to do manual substitution for the port.

For the notebook server, I just pushed a commit that will give us only one correct URL instead of that second one (it also printed one with the docker container's hostname) so it looks like this now

notebook-server-1  | [I 05:02:57.841 NotebookApp] Jupyter Notebook 6.5.7 is running at:
notebook-server-1  | [I 05:02:57.841 NotebookApp] localhost:47962/?token=70a243abdf4fb6f516501d1e37df84439ab54f579f97c969
notebook-server-1  | [I 05:02:57.841 NotebookApp] Use Control-C to stop this server and shut down all kernels (twice to skip confirmation).
notebook-server-1  | [C 05:02:57.844 NotebookApp]
notebook-server-1  |

Thanks to

diff --git a/viz_scripts/docker/start_notebook.sh b/viz_scripts/docker/start_notebook.sh
index e0deb7e..4bbb171 100755
--- a/viz_scripts/docker/start_notebook.sh
+++ b/viz_scripts/docker/start_notebook.sh
@@ -29,8 +29,8 @@ cd saved-notebooks
 # launch the notebook server
 # tail -f /dev/null
 if [ -z ${CRON_MODE} ] ; then
-    echo "Running notebook in docker, change host:port to localhost:47962 in the URL below"
-    PYTHONPATH=/usr/src/app jupyter notebook --no-browser --ip=0.0.0.0 --port=47962 --allow-root
+    echo "Running notebook in docker..."
+    PYTHONPATH=/usr/src/app jupyter notebook --no-browser --ip=0.0.0.0 --port=47962 --NotebookApp.custom_display_url=localhost:47962 --allow-root
 else
     echo "Running crontab without user interaction, setting python path"
     export PYTHONPATH=/usr/src/app

@Abby-Wheelis
Copy link
Member

I just tested again, and am seeing the same results as described above. The dashboard works the same as it does on main, but I don't need to manually change 6060 to 3274. Same with the notebooks. Behavior wise, this looks good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review done; Changes requested
Development

Successfully merging this pull request may close these issues.

3 participants