-
Notifications
You must be signed in to change notification settings - Fork 6.9k
Make Dashboard Port Configurable #8999
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
Changes from all commits
d994aca
78a388b
54d31ec
e993aa6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,11 +81,16 @@ def cli(logging_level, logging_format): | |
type=int, | ||
default=8265, | ||
help="The local port to forward to the dashboard") | ||
def dashboard(cluster_config_file, cluster_name, port): | ||
@click.option( | ||
"--remote-port", | ||
required=False, | ||
type=int, | ||
default=8265, | ||
help="The remote port your dashboard runs on") | ||
def dashboard(cluster_config_file, cluster_name, port, remote_port): | ||
"""Port-forward a Ray cluster's dashboard to the local machine.""" | ||
# Sleeping in a loop is preferable to `sleep infinity` because the latter | ||
# only works on linux. | ||
remote_port = 8265 | ||
if port: | ||
dashboard_port = port | ||
else: | ||
|
@@ -99,9 +104,9 @@ def dashboard(cluster_config_file, cluster_name, port): | |
port_forward = [ | ||
(dashboard_port, remote_port), | ||
] | ||
click.echo( | ||
"Attempting to establish dashboard at localhost:{}".format( | ||
port_forward[0][0])) | ||
click.echo(("Attempting to establish dashboard locally at" | ||
" localhost:{} connected to" | ||
" remote port {}").format(dashboard_port, remote_port)) | ||
# We want to probe with a no-op that returns quickly to avoid | ||
# exceptions caused by network errors. | ||
exec_cluster( | ||
|
@@ -233,14 +238,35 @@ def dashboard(cluster_config_file, cluster_name, port): | |
"--include-webui", | ||
default=None, | ||
type=bool, | ||
help="provide this argument if the UI should be started") | ||
help="provide this argument if the UI should be started " | ||
"(DEPRECATED: please use --include-dashboard.") | ||
@click.option( | ||
"--webui-host", | ||
required=False, | ||
default="localhost", | ||
help="The host to bind the web UI server to. Can either be localhost " | ||
"(127.0.0.1) or 0.0.0.0 (available from all interfaces). By default, this " | ||
"is set to localhost to prevent access from external machines.") | ||
help="the host to bind the dashboard server to, either localhost " | ||
"(127.0.0.1) or 0.0.0.0 (available from all interfaces). By default," | ||
" this is localhost." | ||
" (DEPRECATED: please use --dashboard-host)") | ||
@click.option( | ||
"--include-dashboard", | ||
default=None, | ||
type=bool, | ||
help="provide this argument to start the Ray dashboard GUI") | ||
@click.option( | ||
"--dashboard-host", | ||
required=False, | ||
default="localhost", | ||
help="the host to bind the dashboard server to, either localhost " | ||
"(127.0.0.1) or 0.0.0.0 (available from all interfaces). By default, this" | ||
"is localhost.") | ||
@click.option( | ||
"--dashboard-port", | ||
required=False, | ||
type=int, | ||
default=ray_constants.DEFAULT_DASHBOARD_PORT, | ||
help="the port to bind the dashboard server to--defaults to {}".format( | ||
ray_constants.DEFAULT_DASHBOARD_PORT)) | ||
@click.option( | ||
"--block", | ||
is_flag=True, | ||
|
@@ -309,7 +335,8 @@ def start(node_ip_address, redis_address, address, redis_port, port, | |
redis_shard_ports, object_manager_port, node_manager_port, | ||
min_worker_port, max_worker_port, memory, object_store_memory, | ||
redis_max_memory, num_cpus, num_gpus, resources, head, include_webui, | ||
webui_host, block, plasma_directory, huge_pages, autoscaling_config, | ||
webui_host, include_dashboard, dashboard_host, dashboard_port, block, | ||
plasma_directory, huge_pages, autoscaling_config, | ||
no_redirect_worker_output, no_redirect_output, | ||
plasma_store_socket_name, raylet_socket_name, temp_dir, include_java, | ||
java_worker_options, load_code_from_local, internal_config): | ||
|
@@ -323,6 +350,22 @@ def start(node_ip_address, redis_address, address, redis_port, port, | |
if port is not None and port != redis_port: | ||
raise ValueError("Cannot specify both --port and --redis-port " | ||
"as port is a rename of deprecated redis-port") | ||
if include_webui is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's not None we should probably either error or set the value for include_dashboard to what include_webui is set to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to what you have for the host flags below There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. |
||
logger.warn("The --include-webui argument will be deprecated soon" | ||
"Please use --include-dashboard instead.") | ||
if include_dashboard is not None: | ||
include_dashboard = include_webui | ||
|
||
dashboard_host_default = "localhost" | ||
if webui_host != dashboard_host_default: | ||
logger.warn("The --webui-host argument will be deprecated" | ||
" soon. Please use --dashboard-host instead.") | ||
if webui_host != dashboard_host and dashboard_host != "localhost": | ||
raise ValueError( | ||
"Cannot specify both --webui-host and --dashboard-host," | ||
" please specify only the latter") | ||
else: | ||
dashboard_host = webui_host | ||
|
||
# Convert hostnames to numerical IP address. | ||
if node_ip_address is not None: | ||
|
@@ -363,8 +406,9 @@ def start(node_ip_address, redis_address, address, redis_port, port, | |
raylet_socket_name=raylet_socket_name, | ||
temp_dir=temp_dir, | ||
include_java=include_java, | ||
include_webui=include_webui, | ||
webui_host=webui_host, | ||
include_dashboard=include_dashboard, | ||
dashboard_host=dashboard_host, | ||
dashboard_port=dashboard_port, | ||
java_worker_options=java_worker_options, | ||
load_code_from_local=load_code_from_local, | ||
_internal_config=internal_config) | ||
|
@@ -443,8 +487,12 @@ def start(node_ip_address, redis_address, address, redis_port, port, | |
raise Exception("If --head is not passed in, --redis-max-clients " | ||
"must not be provided.") | ||
if include_webui: | ||
raise Exception("If --head is not passed in, the --include-webui " | ||
raise Exception("If --head is not passed in, the --include-webui" | ||
"flag is not relevant.") | ||
if include_dashboard: | ||
raise ValueError( | ||
"If --head is not passed in, the --include-dashboard" | ||
"flag is not relevant.") | ||
if include_java is not None: | ||
raise ValueError("--include-java should only be set for the head " | ||
"node.") | ||
|
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.
Should we be starting it by default? A question for another PR
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 am confused. It must be started by default right? Why is i the question for another PR?
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.
We do start it by default already. In retrospect, my comment description here could've been more clear. I will rewrite. The default arg of None starts the dashboard.
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.
Oh, interesting. So you have to set
--include-dashboard=False
to not start it? I think I was confused by the flag name.