From 94494c43d6ef4e567b1e60a9ea93fb05129ccf9d Mon Sep 17 00:00:00 2001 From: Maksim Smolin Date: Tue, 29 Sep 2020 16:55:07 -0700 Subject: [PATCH] [cli] Updates (#10777) --- python/ray/autoscaler/_private/aws/config.py | 4 +- .../autoscaler/_private/aws/node_provider.py | 3 +- python/ray/autoscaler/_private/aws/utils.py | 3 +- python/ray/autoscaler/_private/cli_logger.py | 82 +++++++++++++++---- .../autoscaler/_private/cli_logger_demoall.py | 9 +- .../ray/autoscaler/_private/command_runner.py | 5 +- python/ray/autoscaler/_private/commands.py | 12 ++- .../_private/subprocess_output_util.py | 3 +- python/ray/autoscaler/_private/updater.py | 3 +- python/ray/scripts/scripts.py | 3 +- 10 files changed, 83 insertions(+), 44 deletions(-) diff --git a/python/ray/autoscaler/_private/aws/config.py b/python/ray/autoscaler/_private/aws/config.py index 7831b9634..afa63b949 100644 --- a/python/ray/autoscaler/_private/aws/config.py +++ b/python/ray/autoscaler/_private/aws/config.py @@ -16,9 +16,7 @@ from ray.autoscaler.tags import NODE_KIND_WORKER, NODE_KIND_HEAD from ray.autoscaler.node_provider import _PROVIDER_PRETTY_NAMES from ray.autoscaler._private.aws.utils import LazyDefaultDict, \ handle_boto_error -from ray.autoscaler._private.cli_logger import cli_logger - -import colorful as cf +from ray.autoscaler._private.cli_logger import cli_logger, cf logger = logging.getLogger(__name__) diff --git a/python/ray/autoscaler/_private/aws/node_provider.py b/python/ray/autoscaler/_private/aws/node_provider.py index 4a7301100..874521f37 100644 --- a/python/ray/autoscaler/_private/aws/node_provider.py +++ b/python/ray/autoscaler/_private/aws/node_provider.py @@ -16,8 +16,7 @@ from ray.autoscaler._private.aws.config import bootstrap_aws from ray.autoscaler._private.log_timer import LogTimer from ray.autoscaler._private.aws.utils import boto_exception_handler -from ray.autoscaler._private.cli_logger import cli_logger -import colorful as cf +from ray.autoscaler._private.cli_logger import cli_logger, cf logger = logging.getLogger(__name__) diff --git a/python/ray/autoscaler/_private/aws/utils.py b/python/ray/autoscaler/_private/aws/utils.py index b75075a4e..d97d0afa5 100644 --- a/python/ray/autoscaler/_private/aws/utils.py +++ b/python/ray/autoscaler/_private/aws/utils.py @@ -1,7 +1,6 @@ from collections import defaultdict -from ray.autoscaler._private.cli_logger import cli_logger -import colorful as cf +from ray.autoscaler._private.cli_logger import cli_logger, cf class LazyDefaultDict(defaultdict): diff --git a/python/ray/autoscaler/_private/cli_logger.py b/python/ray/autoscaler/_private/cli_logger.py index 7d1cb4370..57aea7fec 100644 --- a/python/ray/autoscaler/_private/cli_logger.py +++ b/python/ray/autoscaler/_private/cli_logger.py @@ -18,8 +18,73 @@ from typing import Any, Dict, Tuple, Optional, List import click import colorama -from colorful.core import ColorfulString -import colorful as cf +try: + import colorful as _cf + from colorful.core import ColorfulString +except ModuleNotFoundError: + # We mock Colorful to restrict the colors used for consistency + # anyway, so we also allow for not having colorful at all. + # If the Ray Core dependency on colorful is ever removed, + # the CliLogger code will still work. + class ColorfulString: + pass + + class _ColorfulMock: + def __init__(self): + # do not do any color work + self.identity = lambda x: x + + self.colorful = self + self.colormode = None + + self.NO_COLORS = None + self.ANSI_8_COLORS = None + + def disable(self): + pass + + def __getattr__(self, name): + return self.identity + + _cf = _ColorfulMock() + + +# We want to only allow specific formatting +# to prevent people from accidentally making bad looking color schemes. +# +# This is especially important since most will look bad on either light +# or dark themes. +class _ColorfulProxy: + _proxy_whitelist = [ + "disable", + "reset", + "bold", + "italic", + "underlined", + "with_style", + + # used instead of `gray` as `dimmed` adapts to + # both light and dark themes + "dimmed", + "dodgerBlue", # group + "limeGreen", # success + "red", # error + "orange", # warning + "skyBlue" # label + ] + + def __getattr__(self, name): + res = getattr(_cf, name) + if callable(res) and name not in _ColorfulProxy._proxy_whitelist: + raise ValueError("Usage of the colorful method '" + name + + "' is forbidden " + "by the proxy to keep a consistent color scheme. " + "Check `cli_logger.py` for allowed methods") + return res + + +cf = _ColorfulProxy() + colorama.init(strip=False) @@ -152,7 +217,7 @@ def _format_msg(msg: str, if _no_format: # todo: throw if given args/kwargs? return numbering_str + msg + tags_str - return numbering_str + cf.format(msg, *args, **kwargs) + tags_str + return numbering_str + msg.format(*args, **kwargs) + tags_str if kwargs: raise ValueError("We do not support printing kwargs yet.") @@ -373,17 +438,6 @@ class _CliLogger(): return IndentedContextManager() - def timed(self, msg: str, *args: Any, **kwargs: Any): - """ - TODO: Unimplemented special type of output grouping that displays - a timer for its execution. The method was not removed so we - can mark places where this might be useful in case we ever - implement this. - - For arguments, see `_format_msg`. - """ - return self.group(msg, *args, **kwargs) - def group(self, msg: str, *args: Any, **kwargs: Any): """Print a group title in a special color and start an indented block. diff --git a/python/ray/autoscaler/_private/cli_logger_demoall.py b/python/ray/autoscaler/_private/cli_logger_demoall.py index 1f7c1e68b..76c3554c0 100755 --- a/python/ray/autoscaler/_private/cli_logger_demoall.py +++ b/python/ray/autoscaler/_private/cli_logger_demoall.py @@ -4,12 +4,9 @@ # function for demonstration purposes. Primarily useful for tuning color and # other formatting. -from ray.autoscaler._private.cli_logger import cli_logger -import colorful as cf +from ray.autoscaler._private.cli_logger import cli_logger, cf -cli_logger.old_style = False -cli_logger.verbosity = 999 -cli_logger.detect_colors() +cli_logger.configure(log_style="auto", verbosity=999) cli_logger.print( cf.bold("Bold ") + cf.italic("Italic ") + cf.underlined("Underlined")) @@ -40,7 +37,5 @@ with cli_logger.indented(): cli_logger.print("Indented") with cli_logger.group("Group"): cli_logger.print("Group contents") -with cli_logger.timed("Timed (unimplemented)"): - cli_logger.print("Timed contents") with cli_logger.verbatim_error_ctx("Verbtaim error"): cli_logger.print("Error contents") diff --git a/python/ray/autoscaler/_private/command_runner.py b/python/ray/autoscaler/_private/command_runner.py index f72c136f7..9234e8dd0 100644 --- a/python/ray/autoscaler/_private/command_runner.py +++ b/python/ray/autoscaler/_private/command_runner.py @@ -23,8 +23,7 @@ from ray.autoscaler._private.log_timer import LogTimer from ray.autoscaler._private.subprocess_output_util import ( run_cmd_redirected, ProcessRunnerError, is_output_redirected) -from ray.autoscaler._private.cli_logger import cli_logger -import colorful as cf +from ray.autoscaler._private.cli_logger import cli_logger, cf logger = logging.getLogger(__name__) @@ -312,7 +311,7 @@ class SSHCommandRunner(CommandRunnerInterface): return ip interval = 10 - with cli_logger.timed("Waiting for IP"): + with cli_logger.group("Waiting for IP"): while time.time() < deadline and \ not self.provider.is_terminated(self.node_id): cli_logger.old_info(logger, "{}Waiting for IP...", diff --git a/python/ray/autoscaler/_private/commands.py b/python/ray/autoscaler/_private/commands.py index d889ca57a..dfc240315 100644 --- a/python/ray/autoscaler/_private/commands.py +++ b/python/ray/autoscaler/_private/commands.py @@ -1,4 +1,3 @@ -import colorful as cf import copy import hashlib import json @@ -28,7 +27,7 @@ from ray.autoscaler.node_provider import _get_node_provider, \ _NODE_PROVIDERS, _PROVIDER_PRETTY_NAMES from ray.autoscaler.tags import TAG_RAY_NODE_KIND, TAG_RAY_LAUNCH_CONFIG, \ TAG_RAY_NODE_NAME, NODE_KIND_WORKER, NODE_KIND_HEAD, TAG_RAY_USER_NODE_TYPE -from ray.autoscaler._private.cli_logger import cli_logger +from ray.autoscaler._private.cli_logger import cli_logger, cf from ray.autoscaler._private.updater import NodeUpdaterThread from ray.autoscaler._private.command_runner import set_using_login_shells, \ set_rsync_silent @@ -260,10 +259,9 @@ def _bootstrap_config(config: Dict[str, Any], provider_cls = importer(config["provider"]) - with cli_logger.timed( - "Checking {} environment settings", - _PROVIDER_PRETTY_NAMES.get(config["provider"]["type"])): - resolved_config = provider_cls.bootstrap_config(config) + cli_logger.print("Checking {} environment settings", + _PROVIDER_PRETTY_NAMES.get(config["provider"]["type"])) + resolved_config = provider_cls.bootstrap_config(config) if not no_config_cache: with open(cache_key, "w") as f: @@ -604,7 +602,7 @@ def get_or_create_head_node(config, start = time.time() head_node = None - with cli_logger.timed("Fetching the new head node"): + with cli_logger.group("Fetching the new head node"): while True: if time.time() - start > 50: cli_logger.abort( diff --git a/python/ray/autoscaler/_private/subprocess_output_util.py b/python/ray/autoscaler/_private/subprocess_output_util.py index b22fbdc4b..22bc981e5 100644 --- a/python/ray/autoscaler/_private/subprocess_output_util.py +++ b/python/ray/autoscaler/_private/subprocess_output_util.py @@ -5,8 +5,7 @@ import tempfile import time import sys -from ray.autoscaler._private.cli_logger import cli_logger -import colorful as cf +from ray.autoscaler._private.cli_logger import cli_logger, cf CONN_REFUSED_PATIENCE = 30 # how long to wait for sshd to run diff --git a/python/ray/autoscaler/_private/updater.py b/python/ray/autoscaler/_private/updater.py index 337d373e3..f305f3b51 100644 --- a/python/ray/autoscaler/_private/updater.py +++ b/python/ray/autoscaler/_private/updater.py @@ -13,11 +13,10 @@ from ray.autoscaler.tags import TAG_RAY_NODE_STATUS, TAG_RAY_RUNTIME_CONFIG, \ from ray.autoscaler._private.command_runner import NODE_START_WAIT_S, \ ProcessRunnerError from ray.autoscaler._private.log_timer import LogTimer -from ray.autoscaler._private.cli_logger import cli_logger +from ray.autoscaler._private.cli_logger import cli_logger, cf import ray.autoscaler._private.subprocess_output_util as cmd_output_util from ray import ray_constants -import colorful as cf logger = logging.getLogger(__name__) diff --git a/python/ray/scripts/scripts.py b/python/ray/scripts/scripts.py index 1fa884490..ddb0c3cb8 100644 --- a/python/ray/scripts/scripts.py +++ b/python/ray/scripts/scripts.py @@ -22,8 +22,7 @@ from ray.autoscaler._private.commands import ( import ray.ray_constants as ray_constants import ray.utils -from ray.autoscaler._private.cli_logger import cli_logger -import colorful as cf +from ray.autoscaler._private.cli_logger import cli_logger, cf logger = logging.getLogger(__name__)