[cli] New logging for the rest of the ray commands (#9984)

Co-authored-by: Richard Liaw <rliaw@berkeley.edu>
This commit is contained in:
Maksim Smolin
2020-08-11 09:58:23 -07:00
committed by GitHub
co-authored by Richard Liaw
parent 4f8fef134e
commit 40b8e35d61
8 changed files with 508 additions and 116 deletions
+69 -7
View File
@@ -9,6 +9,9 @@ as well as indentation and other structured output.
"""
import sys
import logging
import inspect
import os
import click
@@ -18,6 +21,60 @@ import colorful as cf
colorama.init()
def _patched_makeRecord(self,
name,
level,
fn,
lno,
msg,
args,
exc_info,
func=None,
extra=None,
sinfo=None):
"""Monkey-patched version of logging.Logger.makeRecord
We have to patch default loggers so they use the proper frame for
line numbers and function names (otherwise everything shows up as
e.g. cli_logger:info() instead of as where it was called from).
In Python 3.8 we could just use stacklevel=2, but we have to support
Python 3.6 and 3.7 as well.
The solution is this Python magic superhack.
The default makeRecord will deliberately check that we don't override
any existing property on the LogRecord using `extra`,
so we remove that check.
This patched version is otherwise identical to the one in the standard
library.
"""
rv = logging.LogRecord(name, level, fn, lno, msg, args, exc_info, func,
sinfo)
if extra is not None:
rv.__dict__.update(extra)
return rv
logging.Logger.makeRecord = _patched_makeRecord
def _parent_frame_info():
"""Get the info from the caller frame.
Used to override the logging function and line number with the correct
ones. See the comment on _patched_makeRecord for more info.
"""
frame = inspect.currentframe()
# we are also in a function, so must go 2 levels up
caller = frame.f_back.f_back
return {
"lineno": caller.f_lineno,
"filename": os.path.basename(caller.f_code.co_filename),
}
def _format_msg(msg,
*args,
_tags=None,
@@ -93,7 +150,7 @@ def _format_msg(msg,
if _no_format:
# todo: throw if given args/kwargs?
return numbering_str + msg + tags_str
return numbering_str + msg.format(*args, **kwargs) + tags_str
return numbering_str + cf.format(msg, *args, **kwargs) + tags_str
if kwargs:
raise ValueError("We do not support printing kwargs yet.")
@@ -180,7 +237,7 @@ class _CliLogger():
def newline(self):
"""Print a line feed.
"""
self._print("")
self.print("")
def _print(self, msg, linefeed=True):
"""Proxy for printing messages.
@@ -377,7 +434,8 @@ class _CliLogger():
For other arguments, see `_format_msg`.
"""
if self.old_style:
logger.debug(_format_msg(msg, *args, **kwargs))
logger.debug(
_format_msg(msg, *args, **kwargs), extra=_parent_frame_info())
return
def old_info(self, logger, msg, *args, **kwargs):
@@ -393,7 +451,8 @@ class _CliLogger():
For other arguments, see `_format_msg`.
"""
if self.old_style:
logger.info(_format_msg(msg, *args, **kwargs))
logger.info(
_format_msg(msg, *args, **kwargs), extra=_parent_frame_info())
return
def old_warning(self, logger, msg, *args, **kwargs):
@@ -409,7 +468,8 @@ class _CliLogger():
For other arguments, see `_format_msg`.
"""
if self.old_style:
logger.warning(_format_msg(msg, *args, **kwargs))
logger.warning(
_format_msg(msg, *args, **kwargs), extra=_parent_frame_info())
return
def old_error(self, logger, msg, *args, **kwargs):
@@ -425,7 +485,8 @@ class _CliLogger():
For other arguments, see `_format_msg`.
"""
if self.old_style:
logger.error(_format_msg(msg, *args, **kwargs))
logger.error(
_format_msg(msg, *args, **kwargs), extra=_parent_frame_info())
return
def old_exception(self, logger, msg, *args, **kwargs):
@@ -441,7 +502,8 @@ class _CliLogger():
For other arguments, see `_format_msg`.
"""
if self.old_style:
logger.exception(_format_msg(msg, *args, **kwargs))
logger.exception(
_format_msg(msg, *args, **kwargs), extra=_parent_frame_info())
return
def render_list(self, xs, separator=cf.reset(", ")):
+16 -3
View File
@@ -26,7 +26,20 @@ HASH_MAX_LENGTH = 10
KUBECTL_RSYNC = os.path.join(
os.path.dirname(os.path.abspath(__file__)), "kubernetes/kubectl-rsync.sh")
_config = {"use_login_shells": True}
_config = {"use_login_shells": True, "silent_rsync": True}
def is_rsync_silent():
return _config["silent_rsync"]
def set_rsync_silent(val):
"""Choose whether to silence rsync output.
Most commands will want to list rsync'd files themselves rather than
print the default rsync spew.
"""
_config["silent_rsync"] = val
def is_using_login_shells():
@@ -460,7 +473,7 @@ class SSHCommandRunner(CommandRunnerInterface):
target)
]
cli_logger.verbose("Running `{}`", cf.bold(" ".join(command)))
self._run_helper(command, silent=True)
self._run_helper(command, silent=is_rsync_silent())
def run_rsync_down(self, source, target):
self._set_ssh_ip_if_required()
@@ -473,7 +486,7 @@ class SSHCommandRunner(CommandRunnerInterface):
source), target
]
cli_logger.verbose("Running `{}`", cf.bold(" ".join(command)))
self._run_helper(command, silent=True)
self._run_helper(command, silent=is_rsync_silent())
def remote_shell_command_str(self):
if self.ssh_private_key:
+23 -20
View File
@@ -29,7 +29,8 @@ from ray.autoscaler.tags import TAG_RAY_NODE_TYPE, TAG_RAY_LAUNCH_CONFIG, \
from ray.ray_constants import AUTOSCALER_RESOURCE_REQUEST_CHANNEL
from ray.autoscaler.updater import NodeUpdaterThread
from ray.autoscaler.command_runner import set_using_login_shells
from ray.autoscaler.command_runner import set_using_login_shells, \
set_rsync_silent
from ray.autoscaler.command_runner import DockerCommandRunner
from ray.autoscaler.log_timer import LogTimer
from ray.worker import global_worker
@@ -97,14 +98,9 @@ def create_or_update_cluster(
config_file: str, override_min_workers: Optional[int],
override_max_workers: Optional[int], no_restart: bool,
restart_only: bool, yes: bool, override_cluster_name: Optional[str],
no_config_cache: bool, log_old_style: bool, log_color: str,
dump_command_output: bool, use_login_shells: bool,
verbose: int) -> None:
no_config_cache: bool, dump_command_output: bool,
use_login_shells: bool) -> None:
"""Create or updates an autoscaling Ray cluster from a config json."""
cli_logger.old_style = log_old_style
cli_logger.color_mode = log_color
cli_logger.verbosity = verbose
set_using_login_shells(use_login_shells)
cmd_output_util.set_output_redirected(not dump_command_output)
@@ -184,6 +180,7 @@ def create_or_update_cluster(
# because it only supports aws
if config["provider"]["type"] != "aws":
cli_logger.old_style = True
cli_logger.newline()
config = _bootstrap_config(config, no_config_cache)
if config["provider"]["type"] != "aws":
cli_logger.old_style = False
@@ -217,7 +214,6 @@ def _bootstrap_config(config: Dict[str, Any],
try_reload_log_state(config_cache["config"]["provider"],
config_cache.get("provider_log_info"))
cli_logger.newline()
cli_logger.verbose_warning(
"Loaded cached provider configuration "
"from " + cf.bold("{}"), cache_key)
@@ -264,14 +260,8 @@ def _bootstrap_config(config: Dict[str, Any],
def teardown_cluster(config_file: str, yes: bool, workers_only: bool,
override_cluster_name: Optional[str],
keep_min_workers: bool, log_old_style: bool,
log_color: str, verbose: int):
keep_min_workers: bool):
"""Destroys all nodes of a Ray cluster described by a config json."""
cli_logger.old_style = log_old_style
cli_logger.color_mode = log_color
cli_logger.verbosity = verbose
cli_logger.dump_command_output = verbose == 3 # todo: add a separate flag?
config = yaml.safe_load(open(config_file).read())
if override_cluster_name is not None:
config["cluster_name"] = override_cluster_name
@@ -375,7 +365,8 @@ def kill_node(config_file, yes, hard, override_cluster_name):
config["cluster_name"] = override_cluster_name
config = _bootstrap_config(config)
confirm("This will kill a node in your cluster", yes)
cli_logger.confirm(yes, "A random node will be killed.")
cli_logger.old_confirm("This will kill a node in your cluster", yes)
provider = get_node_provider(config["provider"], config["cluster_name"])
try:
@@ -383,7 +374,8 @@ def kill_node(config_file, yes, hard, override_cluster_name):
TAG_RAY_NODE_TYPE: NODE_TYPE_WORKER
})
node = random.choice(nodes)
logger.info("kill_node: Shutdown worker {}".format(node))
cli_logger.print("Shutdown " + cf.bold("{}"), node)
cli_logger.old_info(logger, "kill_node: Shutdown worker {}", node)
if hard:
provider.terminate_node(node)
else:
@@ -682,7 +674,7 @@ def get_or_create_head_node(config, config_file, no_restart, restart_only, yes,
cli_logger.newline()
with cli_logger.group("Useful commands"):
cli_logger.print("Monitor auto-scailng with")
cli_logger.print("Monitor autoscaling with")
cli_logger.print(
cf.bold(" ray exec {}{} {}"), raw_config_file, modifiers,
quote(monitor_str))
@@ -820,9 +812,12 @@ def exec_cluster(config_file: str,
attach_command_parts.append("--screen")
attach_command = " ".join(attach_command_parts)
cli_logger.print("Run `{}` to check command status.",
cf.bold(attach_command))
attach_info = "Use `{}` to check on command status.".format(
attach_command)
logger.info(attach_info)
cli_logger.old_info(logger, attach_info)
return result
finally:
provider.cleanup()
@@ -873,6 +868,10 @@ def rsync(config_file: str,
down: whether we're syncing remote -> local
all_nodes: whether to sync worker nodes in addition to the head node
"""
if bool(source) != bool(target):
cli_logger.abort(
"Expected either both a source and a target, or neither.")
assert bool(source) == bool(target), (
"Must either provide both or neither source and target.")
@@ -918,6 +917,10 @@ def rsync(config_file: str,
rsync = updater.rsync_up
if source and target:
# print rsync progress for single file rsync
cmd_output_util.set_output_redirected(False)
set_rsync_silent(False)
rsync(source, target)
else:
updater.sync_file_mounts(rsync)
@@ -14,7 +14,7 @@ _config = {"redirect_output": True}
def is_output_redirected():
return _config["_redirect_output"]
return _config["redirect_output"]
def set_output_redirected(val):
@@ -26,7 +26,7 @@ def set_output_redirected(val):
val (bool): If true, subprocess output will be redirected to
a temporary file.
"""
_config["_redirect_output"] = val
_config["redirect_output"] = val
class ProcessRunnerError(Exception):
+15 -6
View File
@@ -119,7 +119,10 @@ class NodeUpdater:
self.exitcode = 0
def sync_file_mounts(self, sync_cmd):
def sync_file_mounts(self, sync_cmd, step_numbers=(0, 2)):
# step_numbers is (# of previous steps, total steps)
previous_steps, total_steps = step_numbers
nolog_paths = []
if cli_logger.verbosity == 0:
nolog_paths = [
@@ -154,18 +157,21 @@ class NodeUpdater:
# Rsync file mounts
with cli_logger.group(
"Processing file mounts", _numbered=("[]", 2, 6)):
"Processing file mounts",
_numbered=("[]", previous_steps + 1, total_steps)):
for remote_path, local_path in self.file_mounts.items():
do_sync(remote_path, local_path)
if self.cluster_synced_files:
with cli_logger.group(
"Processing worker file mounts", _numbered=("[]", 3, 6)):
"Processing worker file mounts",
_numbered=("[]", previous_steps + 2, total_steps)):
for path in self.cluster_synced_files:
do_sync(path, path, allow_non_existing_paths=True)
else:
cli_logger.print(
"No worker file mounts to sync", _numbered=("[]", 3, 6))
"No worker file mounts to sync",
_numbered=("[]", previous_steps + 2, total_steps))
def wait_ready(self, deadline):
with cli_logger.group(
@@ -239,7 +245,8 @@ class NodeUpdater:
# full setup might be cancelled here
cli_logger.print(
"Configuration already up to date, "
"skipping file mounts, initalization and setup commands.")
"skipping file mounts, initalization and setup commands.",
_numbered=("[]", "2-5", 6))
cli_logger.old_info(logger,
"{}{} already up-to-date, skip to ray start",
self.log_prefix, self.node_id)
@@ -252,7 +259,7 @@ class NodeUpdater:
self.provider.set_node_tags(
self.node_id, {TAG_RAY_NODE_STATUS: STATUS_SYNCING_FILES})
cli_logger.labeled_value("New status", STATUS_SYNCING_FILES)
self.sync_file_mounts(self.rsync_up)
self.sync_file_mounts(self.rsync_up, step_numbers=(2, 6))
# Only run setup commands if runtime_hash has changed because
# we don't want to run setup_commands every time the head node
@@ -331,7 +338,9 @@ class NodeUpdater:
self.log_prefix + "Ray start commands", show_status=True):
for cmd in self.ray_start_commands:
try:
cmd_output_util.set_output_redirected(False)
self.cmd_runner.run(cmd)
cmd_output_util.set_output_redirected(True)
except ProcessRunnerError as e:
if e.msg_type == "ssh_command_failed":
cli_logger.error("Failed.")