Issue #332 Code review, restore logging all log levels as default beh…

…avior

openeo/api/logs.py

@@ -2,6 +2,10 @@
 from typing import Optional, Union
 
 
+# To keep the helper functions for converting log levels consistent.
+_DEFAULT_LOG_LEVEL = logging.DEBUG
+
+
 class LogEntry(dict):
     """
     Log message and info for jobs and services
@@ -64,7 +68,7 @@ def normalize_log_level(log_level: Optional[Union[int, str]]) -> int:
         logging.ERROR, logging.WARNING, logging.INFO, or logging.DEBUG
     """
     if log_level is None:
-        return logging.ERROR
+        return _DEFAULT_LOG_LEVEL
 
     if isinstance(log_level, str):
         return string_to_log_level(log_level)
@@ -91,8 +95,12 @@ def string_to_log_level(log_level: str) -> int:
         raise TypeError(
             f"Parameter 'log_level' must be type str, but it is type {type(log_level)}. Value: log_level={log_level!r}"
         )
+    # We are not encouraging passing an empty string to log_level, because this function
+    # assumes this input is already valid, i.e. it is coming from code rather that from
+    # user input. But if we do get an empty string anyway, then the default should be
+    # consistent with `normalize_log_level`.
     if log_level == "":
-        return logging.ERROR
+        return _DEFAULT_LOG_LEVEL
 
     log_level = log_level.upper()
     if log_level in ["CRITICAL", "ERROR"]:

openeo/rest/job.py

@@ -158,13 +158,17 @@ def logs(
                 for level ``logging.INFO`` and above, i.e. also ``logging.WARNING`` and
                 ``logging.ERROR`` will be included.
 
-            Defaults to ``logging.ERROR`` (also when explicitly passing log_level=None or log_level="").
+            Default is to show all log levels, in other words ``logging.DEBUG``.
+            This is also the result when you explicitly pass log_level=None or log_level="".
 
         :return: A list containing the log entries for the batch job.
         """
         url = "/jobs/{}/logs".format(self.job_id)
         logs = self.connection.get(url, params={'offset': offset}, expected_status=200).json()["logs"]
-        log_level = normalize_log_level(log_level)
+
+        # Before we introduced the log_level param we printed all logs.
+        # So we explicitly keep that default behavior, making DEBUG the default log level.
+        log_level = normalize_log_level(log_level) if log_level else logging.DEBUG
         entries = [
             LogEntry(log)
             for log in logs

tests/api/test_logs.py

@@ -38,8 +38,8 @@ def test_log_entry_legacy():
 @pytest.mark.parametrize(
     ["log_level_in", "expected_log_level"],
     [
-        (None, logging.ERROR),
-        ("", logging.ERROR),
+        (None, logging.DEBUG),
+        ("", logging.DEBUG),
         (logging.ERROR, logging.ERROR),
         ("error", logging.ERROR),
         ("ERROR", logging.ERROR),
@@ -67,7 +67,7 @@ def test_normalize_log_level_raises_type_error(log_level):
 @pytest.mark.parametrize(
     ["log_level_in", "expected_log_level"],
     [
-        ("", logging.ERROR),
+        ("", logging.DEBUG),
         ("error", logging.ERROR),
         ("ERROR", logging.ERROR),
         ("warning", logging.WARNING),

tests/rest/test_job.py

@@ -266,7 +266,7 @@ def test_get_job_logs(session040, requests_mock):
 
 
 # TODO: do we keep testing on sesssion040, or should we switch to con100?
-def test_get_job_logs_returns_only_error_loglevel_by_default(session040, requests_mock):
+def test_get_job_logs_returns_debug_loglevel_by_default(session040, requests_mock):
     requests_mock.get(
         API_URL + "/jobs/f00ba5/logs",
         json={
@@ -297,14 +297,17 @@ def test_get_job_logs_returns_only_error_loglevel_by_default(session040, request
 
     log_entries = session040.job("f00ba5").logs()
 
-    assert len(log_entries) == 1
-    assert log_entries[0].message == "error processing batch job"
+    assert len(log_entries) == 4
+    assert log_entries[0].level == "error"
+    assert log_entries[1].level == "debug"
+    assert log_entries[2].level == "info"
+    assert log_entries[3].level == "warning"
 
 
 @pytest.mark.parametrize(
     ["log_level", "exp_num_messages"],
     [
-        (None, 1),
+        (None, 4),  # Default is DEBUG / show all log levels.
         (logging.ERROR, 1),
         ("error", 1),
         ("ERROR", 1),