Address Mitar's comments

Author: mfeurer

openml/config.py

@@ -9,7 +9,6 @@
 import os
 from pathlib import Path
 from typing import Tuple, cast
-import warnings
 
 from io import StringIO
 import configparser
@@ -21,7 +20,7 @@
 file_handler = None
 
 
-def _create_log_handlers():
+def _create_log_handlers(create_file_handler=True):
     """ Creates but does not attach the log handlers. """
     global console_handler, file_handler
     if console_handler is not None or file_handler is not None:
@@ -34,12 +33,13 @@ def _create_log_handlers():
     console_handler = logging.StreamHandler()
     console_handler.setFormatter(output_formatter)
 
-    one_mb = 2 ** 20
-    log_path = os.path.join(cache_directory, "openml_python.log")
-    file_handler = logging.handlers.RotatingFileHandler(
-        log_path, maxBytes=one_mb, backupCount=1, delay=True
-    )
-    file_handler.setFormatter(output_formatter)
+    if create_file_handler:
+        one_mb = 2 ** 20
+        log_path = os.path.join(cache_directory, "openml_python.log")
+        file_handler = logging.handlers.RotatingFileHandler(
+            log_path, maxBytes=one_mb, backupCount=1, delay=True
+        )
+        file_handler.setFormatter(output_formatter)
 
 
 def _convert_log_levels(log_level: int) -> Tuple[int, int]:
@@ -194,12 +194,20 @@ def _setup():
     if not os.path.exists(expanded_openml_dir):
         try:
             os.mkdir(expanded_openml_dir)
+            cache_exists = True
         except PermissionError:
-            warnings.warn(
-                "No permission to create openml directory at %s! This can result in OpenML-Python "
-                "not working properly." % expanded_openml_dir
-            )
-            pass
+            cache_exists = False
+    else:
+        cache_exists = True
+
+    if cache_exists:
+        _create_log_handlers()
+    else:
+        _create_log_handlers(create_file_handler=False)
+        openml_logger.warning(
+            "No permission to create openml directory at %s! This can result in OpenML-Python "
+            "not working properly." % expanded_openml_dir
+        )
 
     config = _parse_config()
     apikey = config.get("FAKE_SECTION", "apikey")
@@ -213,14 +221,13 @@ def _setup():
         try:
             os.mkdir(cache_directory)
         except PermissionError:
-            warnings.warn(
+            openml_logger.warning(
                 "No permission to create openml cache directory at %s! This can result in "
                 "OpenML-Python not working properly." % cache_directory
             )
-            pass
 
     avoid_duplicate_runs = config.getboolean("FAKE_SECTION", "avoid_duplicate_runs")
-    connection_n_retries = config.get("FAKE_SECTION", "connection_n_retries")
+    connection_n_retries = int(config.get("FAKE_SECTION", "connection_n_retries"))
     if connection_n_retries > 20:
         raise ValueError(
             "A higher number of retries than 20 is not allowed to keep the "
@@ -296,4 +303,3 @@ def set_cache_directory(cachedir):
 ]
 
 _setup()
-_create_log_handlers()

openml/utils.py

@@ -279,7 +279,7 @@ def _create_cache_directory(key):
         os.makedirs(cache_dir, exist_ok=True)
     except Exception as e:
         raise openml.exceptions.OpenMLCacheException(
-            f"Cannot create cache directory {cache_dir} due to exception {e}"
+            f"Cannot create cache directory {cache_dir}."
         ) from e
     return cache_dir
 

tests/test_openml/test_config.py

@@ -10,8 +10,9 @@
 
 class TestConfig(openml.testing.TestBase):
     @unittest.mock.patch("os.path.expanduser")
-    @unittest.mock.patch("warnings.warn")
-    def test_non_writable_home(self, warnings_mock, expanduser_mock):
+    @unittest.mock.patch("openml.config.openml_logger.warning")
+    @unittest.mock.patch("openml.config._create_log_handlers")
+    def test_non_writable_home(self, log_handler_mock, warnings_mock, expanduser_mock):
         with tempfile.TemporaryDirectory(dir=self.workdir) as td:
             expanduser_mock.side_effect = (
                 os.path.join(td, "openmldir"),
@@ -21,6 +22,8 @@ def test_non_writable_home(self, warnings_mock, expanduser_mock):
             openml.config._setup()
 
         self.assertEqual(warnings_mock.call_count, 2)
+        self.assertEqual(log_handler_mock.call_count, 1)
+        self.assertFalse(log_handler_mock.call_args_list[0][1]["create_file_handler"])
 
 
 class TestConfigurationForExamples(openml.testing.TestBase):

tests/test_utils/test_utils.py

@@ -97,7 +97,6 @@ def test__create_cache_directory(self, config_mock):
             os.chmod(subdir, 0o444)
             config_mock.return_value = subdir
             with self.assertRaisesRegex(
-                openml.exceptions.OpenMLCacheException,
-                r"due to exception \[Errno 13\] Permission denied",
+                openml.exceptions.OpenMLCacheException, r"Cannot create cache directory",
             ):
                 openml.utils._create_cache_directory("ghi")