Skip to content

Allow all hosts in jupyter-server-proxy#6966

Open
jasonkena wants to merge 2 commits intodask:mainfrom
jasonkena:main
Open

Allow all hosts in jupyter-server-proxy#6966
jasonkena wants to merge 2 commits intodask:mainfrom
jasonkena:main

Conversation

@jasonkena
Copy link
Copy Markdown

@jasonkena jasonkena commented Aug 29, 2022

Allows the dashboard for remote workers on the bokeh info page to function by modifying host_allowlist.

This solves the error Host 'xxx' is not allowed. See https://jupyter-server-proxy.readthedocs.io/en/latest/arbitrary-ports-hosts.html for info.

  • Tests added / passed
  • Passes pre-commit run --all-files

@GPUtester
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 1, 2022

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       15 files  ±0         15 suites  ±0   6h 48m 0s ⏱️ + 5m 57s
  3 086 tests ±0    3 001 ✔️ ±0    84 💤 ±0  1 ±0 
22 834 runs  ±0  21 848 ✔️ ±0  985 💤 ±0  1 ±0 

For more details on these failures, see this check.

Results for commit 64410d9. ± Comparison against base commit bfc5cfe.

Copy link
Copy Markdown
Member

@jacobtomlinson jacobtomlinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am apprehensive to just set this to True and allow all proxying. This feature of jupyter-server-proxy is intended to avoid folks from using the proxy to access services they shouldn't.

Ideally we should just be adding the worker IPs to the allowlist.

cc @yuvipanda who may have opinions here

@jasonkena jasonkena requested a review from fjetter as a code owner January 23, 2024 10:57
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.

3 participants