FIX(config) Respect log_level from settings.yaml and suppress premature debug output (#9)

* FIX(config) Respect log_level from settings.yaml and suppress premature debug output

Settings.yaml log_level was ignored because config was loaded after --log-level
argument default was set. Additionally, debug messages from config loading appeared
before the logger was properly configured.

Changes:
- Load config defaults before creating argparse arguments
- Use config file log_level as default instead of hardcoded "INFO"
- Remove loguru default handler before parsing args to suppress early debug output
- Add log_config_status() to log config info AFTER logger is configured
- Remove redundant --log-level from trigger/summary subcommands
- Note: --log-level must now come BEFORE subcommand (e.g., rptool --log-level DEBUG write)

Fixes: #6

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>


---------

Signed-off-by: Zdenek Kraus <zkraus@redhat.com>

Author: zkraus

src/reportportal/ap.py

@@ -45,6 +45,10 @@ def create_main_parser() -> argparse.ArgumentParser:
     except PackageNotFoundError:
         pkg_version = 'unknown (not installed)'
 
+    # Get configuration defaults (config file + env vars + built-in defaults)
+    # Must be loaded before creating arguments that use these defaults
+    defaults = _get_config_defaults()
+
     parser = argparse.ArgumentParser(
         prog='rptool',
         description='Unified command-line interface for ReportPortal tools',
@@ -58,11 +62,17 @@ def create_main_parser() -> argparse.ArgumentParser:
         version=f'rptool {pkg_version}'
     )
 
+    # Validation of config file log_level
+    valid_log_levels = {"DEBUG", "INFO", "WARNING", "ERROR"}
+    configured_log_level = str(defaults.get("log_level", "INFO")).upper()
+    if configured_log_level not in valid_log_levels:
+        raise ValueError(f'Invalid log level in config: {configured_log_level}. Must be one of {valid_log_levels}')
+
     parser.add_argument(
         "--log-level",
         choices=["DEBUG", "INFO", "WARNING", "ERROR"],
-        default="INFO",
-        help="Set the logging level (default: INFO)"
+        default=configured_log_level,
+        help="Set the logging level (default: from config or INFO)"
     )
 
     # Create subparsers for each command
@@ -74,9 +84,6 @@ def create_main_parser() -> argparse.ArgumentParser:
         required=True
     )
 
-    # Get configuration defaults (config file + env vars + built-in defaults)
-    defaults = _get_config_defaults()
-
     # Adding subparsers' arguments
     subparsers_hanlers = [
         _add_write_arguments,
@@ -124,14 +131,14 @@ def _add_write_arguments(subparsers: argparse.ArgumentParser, defaults: dict) ->
     _add_common_rp_args(parser, defaults)
 
     parser.add_argument(
-        "--launch-name", 
+        "--launch-name",
         help="Override Launch name that will be reported, otherwise filename will be used",
         default=defaults['rp_launch_name']
     )
     parser.add_argument(
-        "--launch-description", 
+        "--launch-description",
         help="Custom head section to launch description, passthrough description will be added from the junit if available",
-        # The empty string from defaults is necessary to enable additional description to be added on .finish_launch() 
+        # The empty string from defaults is necessary to enable additional description to be added on .finish_launch()
         default=defaults['rp_launch_description'],
     )
     parser.add_argument(
@@ -147,7 +154,7 @@ def _add_write_arguments(subparsers: argparse.ArgumentParser, defaults: dict) ->
         default=False
     )
     parser.add_argument("junits", nargs='+', help="path to all junit results, multiple files will be reportes as one launch")
-    
+
 
 def _add_query_arguments(subparsers: argparse.ArgumentParser, defaults: dict) -> None:
     """Add arguments for query command."""
@@ -240,13 +247,6 @@ def _add_trigger_arguments(subparsers: argparse.ArgumentParser, defaults: dict)
     )
     _add_common_rp_args(parser, defaults)
 
-    parser.add_argument(
-        "--log-level",
-        choices=["DEBUG", "INFO", "WARNING", "ERROR"],
-        default=defaults.get("log_level", "INFO"),
-        help="Set the logging level (default: INFO)"
-    )
-
 
 def _add_summary_arguments(subparsers: argparse.ArgumentParser, defaults: dict) -> None:
     """Add arguments for summary command."""
@@ -260,12 +260,6 @@ def _add_summary_arguments(subparsers: argparse.ArgumentParser, defaults: dict)
 
     _add_common_rp_args(parser, defaults)
 
-    parser.add_argument(
-        "--log-level",
-        choices=["DEBUG", "INFO", "WARNING", "ERROR"],
-        default=defaults.get("log_level", "INFO"),
-        help="Set the logging level (default: INFO)"
-    )
     parser.add_argument(
         "--attribute",
         action="append",

src/reportportal/config.py

@@ -37,26 +37,29 @@ def load_config_file() -> Dict[str, Any]:
 
     Returns:
         Dictionary with configuration values, empty dict if file doesn't exist
-        or can't be loaded
+        or is empty.
+
+    Raises:
+        ValueError: When config file cannot be parsed properly.
     """
 
     config_file = get_config_file_path()
 
     if not config_file.exists():
-        logger.debug(f"Config file not found: {config_file}")
+        logger.debug("No config file present, using defaults")
         return {}
 
     try:
         with open(config_file, 'r') as f:
             config = yaml.safe_load(f)
             if config is None:
-                logger.debug(f"Config file is empty: {config_file}")
+                logger.debug("Config file empty")
                 return {}
-            logger.debug(f"Loaded config from: {config_file}")
+            logger.info("Config file loaded successfully")
             return config
     except Exception as e:
-        logger.warning(f"Failed to load config file {config_file}: {e}")
-        return {}
+        # need to raise ValueError to indicate critical problem
+        raise ValueError(f"Error reading config file {config_file} {e}")
 
 
 def get_config_defaults() -> Dict[str, Any]:
@@ -162,6 +165,7 @@ def get_effective_defaults() -> Dict[str, Any]:
     # Inject REQUESTS_CA_BUNDLE into environment if configured but not already set
     if merged.get("requests_ca_bundle") and not os.environ.get("REQUESTS_CA_BUNDLE"):
         os.environ["REQUESTS_CA_BUNDLE"] = merged["requests_ca_bundle"]
-        logger.debug(f"Set REQUESTS_CA_BUNDLE from config: {merged['requests_ca_bundle']}")
+        logger.debug("Set REQUESTS_CA_BUNDLE from config: {}", merged['requests_ca_bundle'])
 
     return merged
+

src/reportportal/rp_dispatcher.py

@@ -164,16 +164,28 @@ def main(argv: Optional[List[str]] = None) -> int:
     Returns:
         Exit code (0 for success, 1 for error)
     """
-    parser = ap.create_main_parser()
+    # Remove default loguru handler immediately to prevent premature debug messages
+    # (e.g., during config file loading before log level is determined)
+    logger.remove()
+    # Setup intermittent WARNING logger for any configuration logs
+    # or set to environmnet variable LOG_LEVEL if defined
+    logger.add(sink=sys.stderr, level=os.environ.get('LOG_LEVEL', "").upper() or 'WARNING')
+
+    try:
+        parser = ap.create_main_parser()
+    except ValueError as e:
+        logger.error('Improper configuration {}', str(e))
+        return 1
+
 
     # Parse arguments
     try:
         args = parser.parse_args(argv)
     except SystemExit as e:
         return e.code if e.code is not None else 1
 
-    # setup logging handlers
-    logger.remove() # remove default one
+    # Setup logging handler with configured log level
+    logger.remove()
     logger.add(sink=sys.stderr, level=args.log_level)
 
     # Dispatch to appropriate command handler

tests/unit/test_ap.py

@@ -4,6 +4,7 @@
 
 import pytest
 import os
+from pathlib import Path
 from unittest.mock import patch
 
 from reportportal.ap import (
@@ -321,11 +322,11 @@ def test_release_parser_all_options(self):
                 parser = create_main_parser()
 
                 args = parser.parse_args([
+                    '--log-level', 'DEBUG',
                     'summary',
                     '--rp-project', 'test_project',
                     '--rp-url', 'https://test.reportportal.com',
                     '--rp-token', 'test_key',
-                    '--log-level', 'DEBUG',
                     '--attribute', 'kuadrant:v1.3.1',
                     '--attribute', 'platform:aws',
                     '--days', '7',
@@ -373,7 +374,7 @@ def test_release_parser_log_levels(self):
                 parser = create_main_parser()
 
                 for level in ['DEBUG', 'INFO', 'WARNING', 'ERROR']:
-                    args = parser.parse_args(['summary', '--attribute', 'kuadrant:v1.3.1', '--log-level', level])
+                    args = parser.parse_args(['--log-level', level, 'summary', '--attribute', 'kuadrant:v1.3.1'])
                     assert args.log_level == level
 
     def test_release_parser_days_parameter(self):
@@ -535,4 +536,136 @@ def test_multi_criteria_filtering(self):
                 assert 'platform:gcp' in args.attribute
                 assert 'component:controller' in args.attribute
                 assert 'env:staging' in args.attribute
-                assert args.days == 7
+                assert args.days == 7
+
+
+@pytest.mark.unit
+class TestLogLevelConfig:
+    """Test log level configuration (Issue #6)."""
+
+    def test_log_level_from_config_file(self):
+        """Test that log_level from config file is used as default."""
+        with patch.dict(os.environ, {}, clear=True):
+            with patch('reportportal.config.load_config_file') as mock_load:
+                # Simulate config file with DEBUG log level
+                mock_load.return_value = {'log_level': 'DEBUG'}
+                parser = create_main_parser()
+
+                # Parse without --log-level argument
+                args = parser.parse_args(['write', 'test.xml'])
+
+                # Should use config file value
+                assert args.log_level == 'DEBUG'
+
+    def test_log_level_cli_overrides_config(self):
+        """Test that CLI --log-level overrides config file."""
+        with patch.dict(os.environ, {}, clear=True):
+            with patch('reportportal.config.load_config_file') as mock_load:
+                # Config has DEBUG
+                mock_load.return_value = {'log_level': 'DEBUG'}
+                parser = create_main_parser()
+
+                # CLI specifies ERROR
+                args = parser.parse_args(['--log-level', 'ERROR', 'write', 'test.xml'])
+
+                # Should use CLI value
+                assert args.log_level == 'ERROR'
+
+    def test_log_level_default_when_no_config(self):
+        """Test that INFO is used when no config is provided."""
+        with patch.dict(os.environ, {}, clear=True):
+            with patch('reportportal.config.load_config_file') as mock_load:
+                # No log_level in config
+                mock_load.return_value = {}
+                parser = create_main_parser()
+
+                # Parse without --log-level argument
+                args = parser.parse_args(['write', 'test.xml'])
+
+                # Should use built-in default (INFO)
+                assert args.log_level == 'INFO'
+
+    def test_log_level_works_with_all_commands(self):
+        """Test that config log_level works for all commands."""
+        with patch.dict(os.environ, {}, clear=True):
+            with patch('reportportal.config.load_config_file') as mock_load:
+                mock_load.return_value = {'log_level': 'WARNING'}
+                parser = create_main_parser()
+
+                # Test write command
+                args = parser.parse_args(['write', 'test.xml'])
+                assert args.log_level == 'WARNING'
+
+                # Test query command
+                args = parser.parse_args(['query'])
+                assert args.log_level == 'WARNING'
+
+                # Test trigger command
+                args = parser.parse_args(['trigger'])
+                assert args.log_level == 'WARNING'
+
+                # Test summary command
+                args = parser.parse_args(['summary', '--attribute', 'test:v1'])
+                assert args.log_level == 'WARNING'
+
+    def test_log_level_invalid_in_config(self):
+        """Test that invalid log level in config raises ValueError."""
+        with patch.dict(os.environ, {}, clear=True):
+            with patch('reportportal.config.load_config_file') as mock_load:
+                # Simulate config file with invalid log level
+                mock_load.return_value = {'log_level': 'VERBOSE'}
+
+                # Creating parser should raise ValueError
+                with pytest.raises(ValueError) as exc_info:
+                    create_main_parser()
+
+                # Check error message contains the invalid value
+                error_msg = str(exc_info.value)
+                assert 'Invalid log level in config' in error_msg
+                assert 'VERBOSE' in error_msg
+                assert 'Must be one of' in error_msg
+
+    def test_log_level_invalid_in_config_case_insensitive(self):
+        """Test that invalid log level works with case normalization."""
+        with patch.dict(os.environ, {}, clear=True):
+            with patch('reportportal.config.load_config_file') as mock_load:
+                # Lowercase 'trace' should also be rejected
+                mock_load.return_value = {'log_level': 'trace'}
+
+                with pytest.raises(ValueError) as exc_info:
+                    create_main_parser()
+
+                error_msg = str(exc_info.value)
+                assert 'Invalid log level in config' in error_msg
+                # Should show uppercase version in error
+                assert 'TRACE' in error_msg
+
+    def test_no_premature_debug_messages_during_config_load(self):
+        """Test that debug messages during config loading are suppressed until log level is set."""
+        import io
+        from unittest.mock import patch
+        from reportportal.rp_dispatcher import main
+
+        # Capture stderr to check for debug messages
+        captured_stderr = io.StringIO()
+
+        with patch.dict(os.environ, {}, clear=True):
+            with patch('reportportal.config.load_config_file') as mock_load:
+                # Simulate config file exists and has values
+                mock_load.return_value = {'log_level': 'INFO', 'rp_url': 'http://test.com'}
+
+                # Redirect stderr
+                with patch('sys.stderr', captured_stderr):
+                    try:
+                        # Run with INFO level (default from config)
+                        # This will fail because we don't have valid args, but we just want to check logging
+                        main(['write', 'test.xml'])
+                    except SystemExit:
+                        pass
+
+                # Check that no "Loaded config from" or "Config file" debug messages appear
+                stderr_output = captured_stderr.getvalue()
+                assert "Loaded config from" not in stderr_output, \
+                    "Debug message 'Loaded config from' should not appear with INFO log level"
+                assert "Config file not found" not in stderr_output, \
+                    "Debug message 'Config file not found' should not appear with INFO log level"