Fixed some basic Flake8 errors, added Pylinter exception, Fixed some minor logging bugs.

This commit is contained in:
TheNetworkGuy 2025-04-28 15:44:45 +02:00
parent 819126ce36
commit 0c715d4f96
3 changed files with 19 additions and 19 deletions

View File

@ -57,8 +57,7 @@ def load_config_file(config_default, config_file="config.py"):
if hasattr(config_module, key): if hasattr(config_module, key):
dconf[key] = getattr(config_module, key) dconf[key] = getattr(config_module, key)
return dconf return dconf
else: logger.warning(
logger.warning( "Config file %s not found. Using default config "
"Config file %s not found. Using default config " "and environment variables.", config_file)
"and environment variables.", config_file)
return None return None

View File

@ -69,7 +69,7 @@ class PhysicalDevice():
if config["device_cf"] in self.nb.custom_fields: if config["device_cf"] in self.nb.custom_fields:
self.zabbix_id = self.nb.custom_fields[config["device_cf"]] self.zabbix_id = self.nb.custom_fields[config["device_cf"]]
else: else:
e = f"Host {self.name}: Custom field {config["device_cf"]} not present" e = f'Host {self.name}: Custom field {config["device_cf"]} not present'
self.logger.warning(e) self.logger.warning(e)
raise SyncInventoryError(e) raise SyncInventoryError(e)
@ -131,11 +131,13 @@ class PhysicalDevice():
# Set value to template # Set value to template
return [device_type_cfs[config["template_cf"]]] return [device_type_cfs[config["template_cf"]]]
# Custom field not found, return error # Custom field not found, return error
e = (f"Custom field {config["template_cf"]} not " e = (f'Custom field {config["template_cf"]} not '
f"found for {self.nb.device_type.manufacturer.name}" f"found for {self.nb.device_type.manufacturer.name}"
f" - {self.nb.device_type.display}.") f" - {self.nb.device_type.display}.")
raise TemplateError(e) raise TemplateError(e)
def get_templates_context(self): def get_templates_context(self):
""" Get Zabbix templates from the device context """ """ Get Zabbix templates from the device context """
if "zabbix" not in self.config_context: if "zabbix" not in self.config_context:
@ -165,8 +167,8 @@ class PhysicalDevice():
elif config["inventory_mode"] == "automatic": elif config["inventory_mode"] == "automatic":
self.inventory_mode = 1 self.inventory_mode = 1
else: else:
self.logger.error(f"Host {self.name}: Specified value for inventory mode in" self.logger.error(f"Host {self.name}: Specified value for inventory mode in "
f" config is not valid. Got value {config["inventory_mode"]}") f'config is not valid. Got value {config["inventory_mode"]}')
return False return False
self.inventory = {} self.inventory = {}
if config["inventory_sync"] and self.inventory_mode in [0, 1]: if config["inventory_sync"] and self.inventory_mode in [0, 1]:

View File

@ -1,6 +1,5 @@
"""Tests for configuration parsing in the modules.config module.""" """Tests for configuration parsing in the modules.config module."""
from unittest.mock import patch, MagicMock from unittest.mock import patch, MagicMock
from pathlib import Path
import os import os
from modules.config import load_config, DEFAULT_CONFIG, load_config_file, load_env_variable from modules.config import load_config, DEFAULT_CONFIG, load_config_file, load_env_variable
@ -20,7 +19,7 @@ def test_load_config_file():
mock_config = DEFAULT_CONFIG.copy() mock_config = DEFAULT_CONFIG.copy()
mock_config["templates_config_context"] = True mock_config["templates_config_context"] = True
mock_config["sync_vms"] = True mock_config["sync_vms"] = True
with patch('modules.config.load_config_file', return_value=mock_config), \ with patch('modules.config.load_config_file', return_value=mock_config), \
patch('modules.config.load_env_variable', return_value=None): patch('modules.config.load_env_variable', return_value=None):
config = load_config() config = load_config()
@ -39,7 +38,7 @@ def test_load_env_variables():
if key == "create_journal": if key == "create_journal":
return True return True
return None return None
with patch('modules.config.load_config_file', return_value=DEFAULT_CONFIG.copy()), \ with patch('modules.config.load_config_file', return_value=DEFAULT_CONFIG.copy()), \
patch('modules.config.load_env_variable', side_effect=mock_load_env): patch('modules.config.load_env_variable', side_effect=mock_load_env):
config = load_config() config = load_config()
@ -54,13 +53,13 @@ def test_env_vars_override_config_file():
mock_config = DEFAULT_CONFIG.copy() mock_config = DEFAULT_CONFIG.copy()
mock_config["templates_config_context"] = True mock_config["templates_config_context"] = True
mock_config["sync_vms"] = False mock_config["sync_vms"] = False
# Mock env variable that will override the config file value # Mock env variable that will override the config file value
def mock_load_env(key): def mock_load_env(key):
if key == "sync_vms": if key == "sync_vms":
return True return True
return None return None
with patch('modules.config.load_config_file', return_value=mock_config), \ with patch('modules.config.load_config_file', return_value=mock_config), \
patch('modules.config.load_env_variable', side_effect=mock_load_env): patch('modules.config.load_env_variable', side_effect=mock_load_env):
config = load_config() config = load_config()
@ -79,12 +78,12 @@ def test_load_config_file_function():
mock_module = MagicMock() mock_module = MagicMock()
mock_module.templates_config_context = True mock_module.templates_config_context = True
mock_module.sync_vms = True mock_module.sync_vms = True
# Setup the mock spec # Setup the mock spec
mock_spec_instance = MagicMock() mock_spec_instance = MagicMock()
mock_spec.return_value = mock_spec_instance mock_spec.return_value = mock_spec_instance
mock_spec_instance.loader.exec_module = lambda x: None mock_spec_instance.loader.exec_module = lambda x: None
# Patch module_from_spec to return our mock module # Patch module_from_spec to return our mock module
with patch('importlib.util.module_from_spec', return_value=mock_module): with patch('importlib.util.module_from_spec', return_value=mock_module):
config = load_config_file(DEFAULT_CONFIG.copy()) config = load_config_file(DEFAULT_CONFIG.copy())
@ -94,7 +93,7 @@ def test_load_config_file_function():
def test_load_config_file_not_found(): def test_load_config_file_not_found():
"""Test load_config_file when the config file doesn't exist""" """Test load_config_file when the config file doesn't exist"""
# Instead of trying to assert on the logger call, we'll just check the return value # Instead of trying to assert on the logger call, we'll just check the return value
# and verify the function works as expected in this case # and verify the function works as expected in this case
with patch('pathlib.Path.exists', return_value=False): with patch('pathlib.Path.exists', return_value=False):
result = load_config_file(DEFAULT_CONFIG.copy()) result = load_config_file(DEFAULT_CONFIG.copy())
@ -107,7 +106,7 @@ def test_load_env_variable_function():
with patch.dict(os.environ, {"templates_config_context": "True"}): with patch.dict(os.environ, {"templates_config_context": "True"}):
value = load_env_variable("templates_config_context") value = load_env_variable("templates_config_context")
assert value == "True" assert value == "True"
# Test when the environment variable doesn't exist # Test when the environment variable doesn't exist
with patch.dict(os.environ, {}, clear=True): with patch.dict(os.environ, {}, clear=True):
value = load_env_variable("nonexistent_variable") value = load_env_variable("nonexistent_variable")
@ -123,8 +122,8 @@ def test_load_config_file_exception_handling():
# Since the current implementation doesn't handle exceptions, we should # Since the current implementation doesn't handle exceptions, we should
# expect an exception to be raised # expect an exception to be raised
try: try:
result = load_config_file(DEFAULT_CONFIG.copy()) load_config_file(DEFAULT_CONFIG.copy())
assert False, "An exception should have been raised" assert False, "An exception should have been raised"
except Exception: except Exception: # pylint: disable=broad-except
# This is expected # This is expected
pass pass