From eace94d2dc51369dfb204320c9c5fa641cfd54ae Mon Sep 17 00:00:00 2001 From: Maksim Smolin Date: Thu, 6 Aug 2020 14:44:39 -0700 Subject: [PATCH] [cli] Fix issues with logging unescaped strings (#9960) --- python/ray/autoscaler/aws/utils.py | 4 ++-- python/ray/autoscaler/cli_logger.py | 16 +++++++++++----- python/ray/autoscaler/command_runner.py | 4 ++-- python/ray/autoscaler/commands.py | 3 ++- python/ray/autoscaler/updater.py | 9 ++++++--- 5 files changed, 23 insertions(+), 13 deletions(-) diff --git a/python/ray/autoscaler/aws/utils.py b/python/ray/autoscaler/aws/utils.py index 76b41cde3..312bfd8c0 100644 --- a/python/ray/autoscaler/aws/utils.py +++ b/python/ray/autoscaler/aws/utils.py @@ -102,8 +102,8 @@ def handle_boto_error(exc, msg, *args, **kwargs): cli_logger.error(*generic_message_args) cli_logger.newline() with cli_logger.verbatim_error_ctx("Boto3 error:"): - cli_logger.verbose(vars(exc)) - cli_logger.error(exc) + cli_logger.verbose("{}", str(vars(exc))) + cli_logger.error("{}", str(exc)) cli_logger.abort() diff --git a/python/ray/autoscaler/cli_logger.py b/python/ray/autoscaler/cli_logger.py index b686bfb9c..eb5cad224 100644 --- a/python/ray/autoscaler/cli_logger.py +++ b/python/ray/autoscaler/cli_logger.py @@ -234,7 +234,7 @@ class _CliLogger(): For arguments, see `_format_msg`. """ - self._print(_format_msg(cf.cornflowerBlue(msg), *args, **kwargs)) + self.print(cf.cornflowerBlue(msg), *args, **kwargs) return self.indented() @@ -252,7 +252,7 @@ class _CliLogger(): class VerbatimErorContextManager(): def __enter__(self): - cli_logger.error(cf.bold("!!! ") + msg, *args, **kwargs) + cli_logger.error(cf.bold("!!! ") + "{}", msg, *args, **kwargs) def __exit__(self, type, value, tb): cli_logger.error(cf.bold("!!!")) @@ -267,6 +267,9 @@ class _CliLogger(): For other arguments, see `_format_msg`. """ + if self.old_style: + return + self._print( cf.cyan(key) + ": " + _format_msg(cf.bold(msg), *args, **kwargs)) @@ -299,27 +302,30 @@ class _CliLogger(): For arguments, see `_format_msg`. """ - self._print(_format_msg(cf.green(msg), *args, **kwargs)) + self.print(cf.green(msg), *args, **kwargs) def warning(self, msg, *args, **kwargs): """Prints a formatted warning message. For arguments, see `_format_msg`. """ - self._print(_format_msg(cf.yellow(msg), *args, **kwargs)) + self.print(cf.yellow(msg), *args, **kwargs) def error(self, msg, *args, **kwargs): """Prints a formatted error message. For arguments, see `_format_msg`. """ - self._print(_format_msg(cf.red(msg), *args, **kwargs)) + self.print(cf.red(msg), *args, **kwargs) def print(self, msg, *args, **kwargs): """Prints a message. For arguments, see `_format_msg`. """ + if self.old_style: + return + self._print(_format_msg(msg, *args, **kwargs)) def abort(self, msg=None, *args, **kwargs): diff --git a/python/ray/autoscaler/command_runner.py b/python/ray/autoscaler/command_runner.py index efbd062c9..1b2ca4c12 100644 --- a/python/ray/autoscaler/command_runner.py +++ b/python/ray/autoscaler/command_runner.py @@ -324,8 +324,8 @@ class SSHCommandRunner(CommandRunnerInterface): try: os.makedirs(self.ssh_control_path, mode=0o700, exist_ok=True) except OSError as e: - cli_logger.warning(e) # todo: msg - cli_logger.old_warning(logger, e) + cli_logger.warning("{}", str(e)) # todo: msg + cli_logger.old_warning(logger, "{}", str(e)) def run(self, cmd, diff --git a/python/ray/autoscaler/commands.py b/python/ray/autoscaler/commands.py index 074903777..cfc425325 100644 --- a/python/ray/autoscaler/commands.py +++ b/python/ray/autoscaler/commands.py @@ -271,7 +271,8 @@ def teardown_cluster(config_file: str, yes: bool, workers_only: bool, port_forward=None, with_output=False) except Exception as e: - cli_logger.verbose_error(e) # todo: add better exception info + # todo: add better exception info + cli_logger.verbose_error("{}", str(e)) cli_logger.warning( "Exception occured when stopping the cluster Ray runtime " "(use -v to dump teardown exceptions).") diff --git a/python/ray/autoscaler/updater.py b/python/ray/autoscaler/updater.py index 0bd832c1e..b40678810 100644 --- a/python/ray/autoscaler/updater.py +++ b/python/ray/autoscaler/updater.py @@ -91,8 +91,9 @@ class NodeUpdater: "Setup command `{}` failed with exit code {}. stderr:", cf.bold(e.cmd), e.returncode) else: - cli_logger.verbose_error(vars(e), _no_format=True) - cli_logger.error(str(e)) # todo: handle this better somehow? + cli_logger.verbose_error("{}", str(vars(e))) + # todo: handle this better somehow? + cli_logger.error("{}", str(e)) # todo: print stderr here cli_logger.error("!!!") cli_logger.newline() @@ -282,7 +283,9 @@ class NodeUpdater: cmd_to_print = cf.bold(cmd) cli_logger.print( - cmd_to_print, _numbered=("()", i, total)) + "{}", + cmd_to_print, + _numbered=("()", i, total)) self.cmd_runner.run(cmd) else: