From 1e01ce3ac97293304c48cd28d213c8c248ad332f Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 19 Oct 2019 22:54:06 +0100 Subject: [PATCH] isatty (#2550) * suppress Tqdm when isatty - fixes #2549 * fix tests * unconditionally disable Tqdm if not isatty - fixes https://github.com/iterative/dvc/pull/2550#discussion_r329386950 * add TQDM_DISABLE env var - update tests - add documentation * TQDM_DISABLE => TQDM_ISATTY * tests: mock.patch sys.stderr * TQDM_ISATTY => DVC_ISATTY as per https://github.com/iterative/dvc/pull/2550#discussion_r334565120 * dvc.progress.Tqdm file kwarg - fixes https://github.com/iterative/dvc/pull/2550/files/ad1f245107c405697c4b61fe58a92fc0265c6588#r335706955 * dynamic env DVC_ISATTY - fixes https://github.com/iterative/dvc/pull/2550/files/ad1f245107c405697c4b61fe58a92fc0265c6588#r335811454 * DVC_ISATTY in tests - fixes https://github.com/iterative/dvc/pull/2550/files/ad1f245107c405697c4b61fe58a92fc0265c6588#r335811099 * re-add and abstract `env2bool` to `dvc.utils` - partially reverts commit 524a0df188f566fb3e103f6965f1b6ac9c530886 - fixes https://github.com/iterative/dvc/pull/2550/files/ad1f245107c405697c4b61fe58a92fc0265c6588#r335811639 * env2bool tidy * rename DVC_ISATTY => DVC_IGNORE_ISATTY - fixes https://github.com/iterative/dvc/pull/2550#discussion_r335947382 --- dvc/analytics.py | 3 ++- dvc/progress.py | 16 ++++++++++++++++ dvc/updater.py | 4 ++-- dvc/utils/__init__.py | 10 ++++++++++ tests/__main__.py | 2 +- tests/conftest.py | 2 ++ tests/func/test_data_cloud.py | 33 ++++++++++++++++----------------- tests/unit/test_progress.py | 24 ++++++++++++++++++++++-- 8 files changed, 71 insertions(+), 23 deletions(-) diff --git a/dvc/analytics.py b/dvc/analytics.py index e4b0dc425..95fda9b9a 100644 --- a/dvc/analytics.py +++ b/dvc/analytics.py @@ -10,6 +10,7 @@ import errno import logging from dvc import __version__ +from dvc.utils import env2bool logger = logging.getLogger(__name__) @@ -229,7 +230,7 @@ class Analytics(object): def is_enabled(cmd=None): from dvc.command.daemon import CmdDaemonBase - if os.getenv("DVC_TEST"): + if env2bool("DVC_TEST"): return False if isinstance(cmd, CmdDaemonBase): diff --git a/dvc/progress.py b/dvc/progress.py index f64459601..588211eb8 100644 --- a/dvc/progress.py +++ b/dvc/progress.py @@ -1,9 +1,11 @@ """Manages progress bars for dvc repo.""" from __future__ import print_function import logging +import sys from tqdm import tqdm from concurrent.futures import ThreadPoolExecutor from funcy import merge +from dvc.utils import env2bool logger = logging.getLogger(__name__) @@ -54,6 +56,7 @@ class Tqdm(tqdm): leave=False, bar_format=None, bytes=False, # pylint: disable=W0622 + file=None, **kwargs ): """ @@ -62,6 +65,9 @@ class Tqdm(tqdm): desc : persists after `close()` level : effective logging level for determining `disable`; used only if `disable` is unspecified + disable : If (default: None), will be determined by logging level. + May be overridden to `True` due to non-TTY status. + Skip override by specifying env var `DVC_IGNORE_ISATTY`. kwargs : anything accepted by `tqdm.tqdm()` """ kwargs = kwargs.copy() @@ -71,9 +77,19 @@ class Tqdm(tqdm): unit="B", unit_scale=True, unit_divisor=1024, miniters=1 ) kwargs = merge(bytes_defaults, kwargs) + if file is None: + file = sys.stderr self.desc_persist = desc + # auto-disable based on `logger.level` if disable is None: disable = logger.getEffectiveLevel() > level + # auto-disable based on TTY + if ( + not disable + and not env2bool("DVC_IGNORE_ISATTY") + and hasattr(file, "isatty") + ): + disable = file.isatty() super(Tqdm, self).__init__( iterable=iterable, disable=disable, diff --git a/dvc/updater.py b/dvc/updater.py index fc1e8d58e..21251e4da 100644 --- a/dvc/updater.py +++ b/dvc/updater.py @@ -9,7 +9,7 @@ from packaging import version from dvc import __version__ from dvc.lock import Lock, LockError -from dvc.utils import is_binary, boxify +from dvc.utils import is_binary, boxify, env2bool logger = logging.getLogger(__name__) @@ -45,7 +45,7 @@ class Updater(object): # pragma: no cover logger.debug(msg.format(self.lock.lockfile, action)) def check(self): - if os.getenv("CI") or os.getenv("DVC_TEST"): + if os.getenv("CI") or env2bool("DVC_TEST"): return self._with_lock(self._check, "checking") diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 594d6cce6..538b7a97e 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -462,6 +462,16 @@ def relpath(path, start=os.curdir): return os.path.relpath(path, start) +def env2bool(var, undefined=False): + """ + undefined: return value if env var is unset + """ + var = os.getenv(var, None) + if var is None: + return undefined + return bool(re.search("1|y|yes|true", var, flags=re.I)) + + def resolve_output(inp, out): from dvc.utils.compat import urlparse diff --git a/tests/__main__.py b/tests/__main__.py index 2068d7f7d..ba115ae68 100644 --- a/tests/__main__.py +++ b/tests/__main__.py @@ -7,7 +7,7 @@ REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) os.chdir(REPO_ROOT) os.putenv( - "PATH", "{}:{}".format(os.path.join(REPO_ROOT, "bin"), os.getenv("PATH")) + "PATH", ":".join([os.path.join(REPO_ROOT, "bin"), os.getenv("PATH")]) ) os.putenv("DVC_HOME", REPO_ROOT) diff --git a/tests/conftest.py b/tests/conftest.py index e9855056d..9ca00335f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,6 +13,8 @@ from .basic_env import TestDirFixture, TestDvcGitFixture, TestGitFixture # Prevent updater and analytics from running their processes os.environ[cast_bytes_py2("DVC_TEST")] = cast_bytes_py2("true") +# Ensure progress output even when not outputting to raw sys.stderr console +os.environ[cast_bytes_py2("DVC_IGNORE_ISATTY")] = cast_bytes_py2("true") @pytest.fixture(autouse=True) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 11c9eceed..6633a4f81 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -13,6 +13,7 @@ import pytest from mock import patch from dvc.utils.compat import str +from dvc.utils import env2bool from dvc.main import main from dvc.config import Config from dvc.data_cloud import DataCloud @@ -57,11 +58,9 @@ os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = TEST_GCP_CREDS_FILE def _should_test_aws(): - dvc_test_aws = os.getenv("DVC_TEST_AWS") - if dvc_test_aws == "true": - return True - elif dvc_test_aws == "false": - return False + do_test = env2bool("DVC_TEST_AWS", undefined=None) + if do_test is not None: + return do_test if os.getenv("AWS_ACCESS_KEY_ID") and os.getenv("AWS_SECRET_ACCESS_KEY"): return True @@ -70,8 +69,9 @@ def _should_test_aws(): def _should_test_gcp(): - if os.getenv("DVC_TEST_GCP") == "true": - return True + do_test = env2bool("DVC_TEST_GCP", undefined=None) + if do_test is not None: + return do_test if not os.path.exists(TEST_GCP_CREDS_FILE): return False @@ -92,10 +92,9 @@ def _should_test_gcp(): def _should_test_azure(): - if os.getenv("DVC_TEST_AZURE") == "true": - return True - elif os.getenv("DVC_TEST_AZURE") == "false": - return False + do_test = env2bool("DVC_TEST_AZURE", undefined=None) + if do_test is not None: + return do_test return os.getenv("AZURE_STORAGE_CONTAINER_NAME") and os.getenv( "AZURE_STORAGE_CONNECTION_STRING" @@ -103,10 +102,9 @@ def _should_test_azure(): def _should_test_oss(): - if os.getenv("DVC_TEST_OSS") == "true": - return True - elif os.getenv("DVC_TEST_OSS") == "false": - return False + do_test = env2bool("DVC_TEST_OSS", undefined=None) + if do_test is not None: + return do_test return ( os.getenv("OSS_ENDPOINT") @@ -116,8 +114,9 @@ def _should_test_oss(): def _should_test_ssh(): - if os.getenv("DVC_TEST_SSH") == "true": - return True + do_test = env2bool("DVC_TEST_SSH", undefined=None) + if do_test is not None: + return do_test # FIXME: enable on windows if os.name == "nt": diff --git a/tests/unit/test_progress.py b/tests/unit/test_progress.py index 8149300c0..36ba35e44 100644 --- a/tests/unit/test_progress.py +++ b/tests/unit/test_progress.py @@ -1,17 +1,37 @@ import logging from dvc.progress import Tqdm +from dvc.utils import env2bool +import sys +import mock -def test_quiet(caplog, capsys): +def test_quiet_logging(caplog, capsys): with caplog.at_level(logging.CRITICAL, logger="dvc"): for _ in Tqdm(range(10)): pass out_err = capsys.readouterr() assert out_err.out == "" assert out_err.err == "" + + +def test_quiet_notty(caplog, capsys): with caplog.at_level(logging.INFO, logger="dvc"): for _ in Tqdm(range(10)): pass out_err = capsys.readouterr() assert out_err.out == "" - assert "0/10" in out_err.err + if env2bool("DVC_IGNORE_ISATTY"): + assert "0/10" in out_err.err + else: + assert out_err.err == "" + + +def test_default(caplog, capsys): + with caplog.at_level(logging.INFO, logger="dvc"): + # simulate interactive terminal + with mock.patch.object(sys.stderr, "isatty", return_value=True): + for _ in Tqdm(range(10)): + pass + out_err = capsys.readouterr() + assert out_err.out == "" + assert "0/10" in out_err.err