From f2dbd3096c7a454fa0c8afefaf2774541e57d18c Mon Sep 17 00:00:00 2001 From: Si-Yuan Date: Wed, 3 Oct 2018 21:08:20 -0700 Subject: [PATCH] Minor improvements and fixes in Python code. (#3022) This commit fix some small defects. 1. Remove a comment that should have been removed in #3003 2. Remove `redis_protected_mode` that is never used in `ray.init()` 3. Fix `object_id_seed` that is forgotten to be passed into `ray._init()` 4. Remove several redundant brackets. --- python/ray/scripts/scripts.py | 40 +++++++++++++++++------------------ python/ray/services.py | 9 +++++--- python/ray/worker.py | 12 +++-------- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/python/ray/scripts/scripts.py b/python/ray/scripts/scripts.py index d3e9417c1..f8e0c5484 100644 --- a/python/ray/scripts/scripts.py +++ b/python/ray/scripts/scripts.py @@ -432,24 +432,24 @@ def stop(): "--min-workers", required=False, type=int, - help=("Override the configured min worker node count for the cluster.")) + help="Override the configured min worker node count for the cluster.") @click.option( "--max-workers", required=False, type=int, - help=("Override the configured max worker node count for the cluster.")) + help="Override the configured max worker node count for the cluster.") @click.option( "--cluster-name", "-n", required=False, type=str, - help=("Override the configured cluster name.")) + help="Override the configured cluster name.") @click.option( "--yes", "-y", is_flag=True, default=False, - help=("Don't ask for confirmation.")) + help="Don't ask for confirmation.") def create_or_update(cluster_config_file, min_workers, max_workers, no_restart, restart_only, yes, cluster_name): if restart_only or no_restart: @@ -465,19 +465,19 @@ def create_or_update(cluster_config_file, min_workers, max_workers, no_restart, "--workers-only", is_flag=True, default=False, - help=("Only destroy the workers.")) + help="Only destroy the workers.") @click.option( "--yes", "-y", is_flag=True, default=False, - help=("Don't ask for confirmation.")) + help="Don't ask for confirmation.") @click.option( "--cluster-name", "-n", required=False, type=str, - help=("Override the configured cluster name.")) + help="Override the configured cluster name.") def teardown(cluster_config_file, yes, workers_only, cluster_name): teardown_cluster(cluster_config_file, yes, workers_only, cluster_name) @@ -488,17 +488,17 @@ def teardown(cluster_config_file, yes, workers_only, cluster_name): "--start", is_flag=True, default=False, - help=("Start the cluster if needed.")) + help="Start the cluster if needed.") @click.option( - "--tmux", is_flag=True, default=False, help=("Run the command in tmux.")) + "--tmux", is_flag=True, default=False, help="Run the command in tmux.") @click.option( "--cluster-name", "-n", required=False, type=str, - help=("Override the configured cluster name.")) + help="Override the configured cluster name.") @click.option( - "--new", "-N", is_flag=True, help=("Force creation of a new screen.")) + "--new", "-N", is_flag=True, help="Force creation of a new screen.") def attach(cluster_config_file, start, tmux, cluster_name, new): attach_cluster(cluster_config_file, start, tmux, cluster_name, new) @@ -512,7 +512,7 @@ def attach(cluster_config_file, start, tmux, cluster_name, new): "-n", required=False, type=str, - help=("Override the configured cluster name.")) + help="Override the configured cluster name.") def rsync_down(cluster_config_file, source, target, cluster_name): rsync(cluster_config_file, source, target, cluster_name, down=True) @@ -526,7 +526,7 @@ def rsync_down(cluster_config_file, source, target, cluster_name): "-n", required=False, type=str, - help=("Override the configured cluster name.")) + help="Override the configured cluster name.") def rsync_up(cluster_config_file, source, target, cluster_name): rsync(cluster_config_file, source, target, cluster_name, down=False) @@ -538,27 +538,27 @@ def rsync_up(cluster_config_file, source, target, cluster_name): "--stop", is_flag=True, default=False, - help=("Stop the cluster after the command finishes running.")) + help="Stop the cluster after the command finishes running.") @click.option( "--start", is_flag=True, default=False, - help=("Start the cluster if needed.")) + help="Start the cluster if needed.") @click.option( "--screen", is_flag=True, default=False, - help=("Run the command in a screen.")) + help="Run the command in a screen.") @click.option( - "--tmux", is_flag=True, default=False, help=("Run the command in tmux.")) + "--tmux", is_flag=True, default=False, help="Run the command in tmux.") @click.option( "--cluster-name", "-n", required=False, type=str, - help=("Override the configured cluster name.")) + help="Override the configured cluster name.") @click.option( - "--port-forward", required=False, type=int, help=("Port to forward.")) + "--port-forward", required=False, type=int, help="Port to forward.") def exec_cmd(cluster_config_file, cmd, screen, tmux, stop, start, cluster_name, port_forward): assert not (screen and tmux), "Can specify only one of `screen` or `tmux`." @@ -576,7 +576,7 @@ def exec_cmd(cluster_config_file, cmd, screen, tmux, stop, start, cluster_name, "-n", required=False, type=str, - help=("Override the configured cluster name.")) + help="Override the configured cluster name.") def get_head_ip(cluster_config_file, cluster_name): click.echo(get_head_node_ip(cluster_config_file, cluster_name)) diff --git a/python/ray/services.py b/python/ray/services.py index ab632354f..f66001ba6 100644 --- a/python/ray/services.py +++ b/python/ray/services.py @@ -187,18 +187,21 @@ def cleanup(): logger.warning("Ray did not shut down properly.") -def all_processes_alive(exclude=[]): +def all_processes_alive(exclude=None): """Check if all of the processes are still alive. Args: exclude: Don't check the processes whose types are in this list. """ + + if exclude is None: + exclude = [] for process_type, processes in all_processes.items(): # Note that p.poll() returns the exit code that the process exited # with, so an exit code of None indicates that the process is still # alive. processes_alive = [p.poll() is None for p in processes] - if (not all(processes_alive) and process_type not in exclude): + if not all(processes_alive) and process_type not in exclude: logger.warning( "A process of type {} has died.".format(process_type)) return False @@ -358,7 +361,7 @@ def _compute_version_info(): ray_version = ray.__version__ python_version = ".".join(map(str, sys.version_info[:3])) pyarrow_version = pyarrow.__version__ - return (ray_version, python_version, pyarrow_version) + return ray_version, python_version, pyarrow_version def _put_version_info_in_redis(redis_client): diff --git a/python/ray/worker.py b/python/ray/worker.py index 5299c2d07..a88503bf0 100644 --- a/python/ray/worker.py +++ b/python/ray/worker.py @@ -201,12 +201,6 @@ class Worker(object): def __init__(self): """Initialize a Worker object.""" - # This is a dictionary mapping driver ID to a dictionary that maps - # remote function IDs for that driver to a counter of the number of - # times that remote function has been executed on this worker. The - # counter is incremented every time the function is executed on this - # worker. When the counter reaches the maximum number of executions - # allowed for a particular function, the worker is killed. self.connected = False self.mode = None self.cached_functions_to_run = [] @@ -1645,7 +1639,6 @@ def init(redis_address=None, ignore_reinit_error=False, num_redis_shards=None, redis_max_clients=None, - redis_protected_mode=True, plasma_directory=None, huge_pages=False, include_webui=True, @@ -1761,6 +1754,7 @@ def init(redis_address=None, address_info=info, start_ray_local=(redis_address is None), num_workers=num_workers, + object_id_seed=object_id_seed, local_mode=local_mode, driver_mode=driver_mode, redirect_worker_output=redirect_worker_output, @@ -1823,9 +1817,9 @@ def shutdown(worker=global_worker): # besides possibly the worker itself. for process_type, processes in services.all_processes.items(): if process_type == services.PROCESS_TYPE_WORKER: - assert (len(processes)) <= 1 + assert len(processes) <= 1 else: - assert (len(processes) == 0) + assert len(processes) == 0 worker.set_mode(None)