From 825c823a37fe91c03b1bfcd21037ea0085d6348d Mon Sep 17 00:00:00 2001 From: skshetry <18718008+skshetry@users.noreply.github.com> Date: Thu, 19 Oct 2023 22:16:26 +0545 Subject: [PATCH] avoid failing on standard streams that are None. (#10041) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://docs.python.org/3/library/sys.html#sys.__stdin__. > [!Note] > Under some conditions stdin, stdout and stderr as well as the original values __stdin__, __stdout__ and __stderr__ can be None. It is usually the case for Windows GUI apps that aren’t connected to a console and Python apps started with pythonw. This is a best-effort solution. External libraries (eg: `tqdm`) and future code that is unaware of this stuff may still fail. As a console application, `stdout`/`stderr` being `None` is not really something that we should support in general. Regarding the failure of the daemon, it has been fixed so that it'll always have a stream attached (if not, it falls back to /dev/null). Today, this is a problem in daemon only if the main process is run with no streams attached, which is not something that we support or go on length to support. It is tricky to write tests for this case, and I could not come up with one. Also decided not to test daemon as it should no longer be a problem. --- dvc/cli/__init__.py | 6 +++--- dvc/daemon.py | 6 +++--- dvc/logger.py | 6 ++++-- dvc/prompt.py | 5 +++-- dvc/ui/__init__.py | 7 +++++-- dvc/ui/pager.py | 5 +++-- dvc/updater.py | 3 +-- dvc/utils/__init__.py | 8 ++++++++ 8 files changed, 30 insertions(+), 16 deletions(-) diff --git a/dvc/cli/__init__.py b/dvc/cli/__init__.py index 1fe146786..85c3e812e 100644 --- a/dvc/cli/__init__.py +++ b/dvc/cli/__init__.py @@ -165,9 +165,9 @@ def main(argv=None): # noqa: C901, PLR0912, PLR0915 # the copied parent's standard file descriptors. If we make any logging # calls in this state it will cause an exception due to writing to a closed # file descriptor. - if sys.stderr.closed: # pylint: disable=using-constant-test + if not sys.stderr or sys.stderr.closed: # pylint: disable=using-constant-test logging.disable() - elif sys.stdout.closed: # pylint: disable=using-constant-test + elif not sys.stdout or sys.stdout.closed: # pylint: disable=using-constant-test logging.disable(logging.INFO) args = None @@ -199,7 +199,7 @@ def main(argv=None): # noqa: C901, PLR0912, PLR0915 logger.trace(args) # type: ignore[attr-defined] - if not sys.stdout.closed and not args.quiet: + if sys.stdout and not sys.stdout.closed and not args.quiet: from dvc.ui import ui ui.enable() diff --git a/dvc/daemon.py b/dvc/daemon.py index 73d29a1a3..cc41cb0eb 100644 --- a/dvc/daemon.py +++ b/dvc/daemon.py @@ -129,12 +129,12 @@ def _spawn_posix( # disconnect from the terminal fd = os.open(os.devnull, os.O_RDWR) - os.dup2(fd, sys.stdin.fileno()) + os.dup2(fd, 0) os.close(fd) with open(output_file or os.devnull, "ab") as f: - os.dup2(f.fileno(), sys.stdout.fileno()) - os.dup2(f.fileno(), sys.stderr.fileno()) + os.dup2(f.fileno(), 1) + os.dup2(f.fileno(), 2) if platform.system() == "Darwin": # workaround for MacOS bug diff --git a/dvc/logger.py b/dvc/logger.py index b49468840..4abff4a75 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -182,9 +182,11 @@ def set_loggers_level(level: int = logging.INFO) -> None: def setup(level: int = logging.INFO, log_colors: bool = True) -> None: + from dvc.utils import isatty + colorama.init() - formatter = ColorFormatter(log_colors=log_colors and sys.stdout.isatty()) + formatter = ColorFormatter(log_colors=log_colors and isatty(sys.stdout)) console_info = LoggerHandler(sys.stdout) console_info.setLevel(logging.INFO) @@ -205,7 +207,7 @@ def setup(level: int = logging.INFO, log_colors: bool = True) -> None: show_traceback = bool(os.environ.get(DVC_SHOW_TRACEBACK)) err_formatter = ColorFormatter( - log_colors=log_colors and sys.stderr.isatty(), show_traceback=show_traceback + log_colors=log_colors and isatty(sys.stderr), show_traceback=show_traceback ) console_errors = LoggerHandler(sys.stderr) console_errors.setLevel(logging.WARNING) diff --git a/dvc/prompt.py b/dvc/prompt.py index 8c358b504..64606448f 100644 --- a/dvc/prompt.py +++ b/dvc/prompt.py @@ -1,7 +1,6 @@ """Manages user prompts.""" import logging -import sys from getpass import getpass from typing import Collection, Optional @@ -9,7 +8,9 @@ logger = logging.getLogger(__name__) def ask(prompt: str, limited_to: Optional[Collection[str]] = None): - if not sys.stdout.isatty(): + from dvc.ui import Console + + if not Console.isatty(): return None while True: diff --git a/dvc/ui/__init__.py b/dvc/ui/__init__.py index 07f64a88a..992d080bc 100644 --- a/dvc/ui/__init__.py +++ b/dvc/ui/__init__.py @@ -340,10 +340,13 @@ class Console: def status(self, status: str, **kwargs: Any) -> "Status": return self.error_console.status(status, **kwargs) - def isatty(self) -> bool: + @staticmethod + def isatty() -> bool: import sys - return sys.stdout.isatty() + from dvc import utils + + return utils.isatty(sys.stdout) def open_browser(self, file: "StrPath") -> int: import webbrowser diff --git a/dvc/ui/pager.py b/dvc/ui/pager.py index 6aa00de78..c4b92ec69 100644 --- a/dvc/ui/pager.py +++ b/dvc/ui/pager.py @@ -3,7 +3,6 @@ import logging import os import pydoc -import sys from rich.pager import Pager @@ -51,7 +50,9 @@ def make_pager(cmd=None): def find_pager(): - if not sys.stdout.isatty(): + from . import Console + + if not Console.isatty(): return None # pylint: disable=redefined-outer-name diff --git a/dvc/updater.py b/dvc/updater.py index 906372e21..a047ede6c 100644 --- a/dvc/updater.py +++ b/dvc/updater.py @@ -1,6 +1,5 @@ import logging import os -import sys import time from typing import TYPE_CHECKING, Optional @@ -123,7 +122,7 @@ class Updater: def _notify(self, latest: str, pkg: Optional[str] = PKG) -> None: from dvc.ui import ui - if not sys.stdout.isatty(): + if not ui.isatty(): return message = self._get_message(latest, pkg=pkg) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 9a0c2d137..2a65cc115 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -11,6 +11,8 @@ from typing import TYPE_CHECKING, Dict, Iterable, Iterator, List, Optional, Tupl import colorama if TYPE_CHECKING: + from typing import TextIO + from dvc.fs import FileSystem logger = logging.getLogger(__name__) @@ -424,3 +426,9 @@ def expand_paths(fs: "FileSystem", paths: Iterable[str]) -> Iterator[str]: yield from fs.find(path) else: yield path + + +def isatty(stream: "Optional[TextIO]") -> bool: + if stream is None: + return False + return stream.isatty()