Use logging instead of warnings

bbc2 · bbc2 · commit a8daa7c52663 · 2020-01-28T19:45:45.000+01:00
Problems with the `warnings` module:

* Undesirable output in tests, although it could be filtered out.
* Output unsuited for users: shows source code of Python-dotenv.  Users
  don't know what to do with that.
* Rather meant for programming issues (e.g. deprecation) rather than
  runtime issues (file not found).

Problems with the `logging` module:

* Slightly less easy to test, unlike warnings with
  `catch_warnings(record=True)`, because you need to use the `mock`
  package for compatibility with Python 2.

Despite this last issue, I think it makes more sense to use the
`logging` module as a base for warnings.

Of course, `logging` is not suited to all forms of output, so this
change doesn't mean `logging` has to be used everywhere.  For instance,
if we want to improve the output of the `dotenv` CLI later, we might
want to use `print`, `click.echo` or other printing facilities.
diff --git a/requirements.txt b/requirements.txt
@@ -3,6 +3,7 @@ typing; python_version<"3.5"
 click
 flake8>=2.2.3
 ipython
+mock
 pytest-cov
 pytest>=3.9
 sh>=1.09
diff --git a/src/dotenv/main.py b/src/dotenv/main.py
@@ -8,7 +8,6 @@
 import shutil
 import sys
 import tempfile
-import warnings
 from collections import OrderedDict
 from contextlib import contextmanager
 
@@ -64,7 +63,7 @@ def _get_stream(self):
                 yield stream
         else:
             if self.verbose:
-                warnings.warn("File doesn't exist {}".format(self.dotenv_path))  # type: ignore
+                logger.warning("File doesn't exist %s", self.dotenv_path)
             yield StringIO('')
 
     def dict(self):
@@ -107,7 +106,7 @@ def get(self, key):
             return data[key]
 
         if self.verbose:
-            warnings.warn("key %s not found in %s." % (key, self.dotenv_path))  # type: ignore
+            logger.warning("Key %s not found in %s.", key, self.dotenv_path)
 
         return None
 
@@ -147,7 +146,7 @@ def set_key(dotenv_path, key_to_set, value_to_set, quote_mode="always"):
     """
     value_to_set = value_to_set.strip("'").strip('"')
     if not os.path.exists(dotenv_path):
-        warnings.warn("can't write to %s - it doesn't exist." % dotenv_path)  # type: ignore
+        logger.warning("Can't write to %s - it doesn't exist.", dotenv_path)
         return None, key_to_set, value_to_set
 
     if " " in value_to_set:
@@ -179,7 +178,7 @@ def unset_key(dotenv_path, key_to_unset, quote_mode="always"):
     If the given key doesn't exist in the .env, fails
     """
     if not os.path.exists(dotenv_path):
-        warnings.warn("can't delete from %s - it doesn't exist." % dotenv_path)  # type: ignore
+        logger.warning("Can't delete from %s - it doesn't exist.", dotenv_path)
         return None, key_to_unset
 
     removed = False
@@ -191,7 +190,7 @@ def unset_key(dotenv_path, key_to_unset, quote_mode="always"):
                 dest.write(mapping.original.string)
 
     if not removed:
-        warnings.warn("key %s not removed from %s - key doesn't exist." % (key_to_unset, dotenv_path))  # type: ignore
+        logger.warning("Key %s not removed from %s - key doesn't exist.", key_to_unset, dotenv_path)
         return None, key_to_unset
 
     return removed, key_to_unset
diff --git a/tests/test_core.py b/tests/test_core.py
@@ -5,8 +5,9 @@
 import os
 import sys
 import textwrap
-import warnings
 
+import logging
+import mock
 import pytest
 import sh
 
@@ -25,12 +26,12 @@ def restore_os_environ():
 
 
 def test_warns_if_file_does_not_exist():
-    with warnings.catch_warnings(record=True) as w:
+    logger = logging.getLogger("dotenv.main")
+
+    with mock.patch.object(logger, "warning") as mock_warning:
         load_dotenv('.does_not_exist', verbose=True)
 
-        assert len(w) == 1
-        assert w[0].category is UserWarning
-        assert str(w[0].message) == "File doesn't exist .does_not_exist"
+    mock_warning.assert_called_once_with("File doesn't exist %s", ".does_not_exist")
 
 
 def test_find_dotenv(tmp_path):
diff --git a/tox.ini b/tox.ini
@@ -2,7 +2,8 @@
 envlist = lint,py{27,34,35,36,37,38,34-no-typing},pypy,pypy3,manifest,coverage-report
 
 [testenv]
-deps = 
+deps =
+  mock
   pytest
   coverage
   sh
