From 6977febb4ca7b1af0cd746576af3ce577805d3ab Mon Sep 17 00:00:00 2001
From: Will <115461530+Will-Cross1@users.noreply.github.com>
Date: Thu, 12 Sep 2024 15:39:53 +0100
Subject: [PATCH] Deprecate separate logging config file (#381)
Remove separate logging file functionality as it's not generally used
and causes issues when accidentally included.
Much the same as apel/ssm#176 issue and apel/ssm#203 PR
- Removed log_config_file and replaced with options.log_config where used
- Removed log_config_file uses in tests as they no longer are used by functions in the tested files
- Removed (optional) from the help messages
---
bin/client.py | 17 +++++++-------
bin/dbloader.py | 22 +++++++++---------
bin/dbunloader.py | 18 +++++++--------
bin/retrieve_dns.py | 22 +++++++++---------
bin/summariser.py | 22 +++++++++---------
conf/logging.cfg | 48 ---------------------------------------
test/test_retrieve_dns.py | 4 ++--
test/test_summariser.py | 2 +-
8 files changed, 54 insertions(+), 101 deletions(-)
delete mode 100644 conf/logging.cfg
diff --git a/bin/client.py b/bin/client.py
index 07f08511..ebfbf158 100644
--- a/bin/client.py
+++ b/bin/client.py
@@ -292,10 +292,15 @@ def main():
default='/etc/apel/sender.cfg')
opt_parser.add_option('-l', '--log_config',
- help='location of logging config file (optional)',
- default='/etc/apel/logging.cfg')
+ help='DEPRECATED - location of logging config file',
+ default=None)
options, unused_args = opt_parser.parse_args()
+
+ # Deprecating functionality.
+ if os.path.exists('/etc/apel/logging.cfg') or options.log_config is not None:
+ logging.warning('Separate logging config file option has been deprecated.')
+
ccp = ConfigParser.ConfigParser()
ccp.read(options.config)
@@ -304,12 +309,8 @@ def main():
# set up logging
try:
- if os.path.exists(options.log_config):
- logging.config.fileConfig(options.log_config)
- else:
- set_up_logging(ccp.get('logging', 'logfile'),
- ccp.get('logging', 'level'),
- ccp.getboolean('logging', 'console'))
+ set_up_logging(ccp.get('logging', 'logfile'), ccp.get('logging', 'level'),
+ ccp.getboolean('logging', 'console'))
log = logging.getLogger(LOGGER_ID)
except (ConfigParser.Error, ValueError, IOError) as err:
print('Error configuring logging: %s' % str(err))
diff --git a/bin/dbloader.py b/bin/dbloader.py
index f2eed4d3..30851376 100644
--- a/bin/dbloader.py
+++ b/bin/dbloader.py
@@ -42,7 +42,7 @@
log = None
-def runprocess(db_config_file, config_file, log_config_file):
+def runprocess(db_config_file, config_file):
'''Parse the configuration file and start the loader.'''
# Read configuration from file
@@ -54,12 +54,8 @@ def runprocess(db_config_file, config_file, log_config_file):
# set up logging
try:
- if os.path.exists(options.log_config):
- logging.config.fileConfig(options.log_config)
- else:
- set_up_logging(cp.get('logging', 'logfile'),
- cp.get('logging', 'level'),
- cp.getboolean('logging', 'console'))
+ set_up_logging(cp.get('logging', 'logfile'), cp.get('logging', 'level'),
+ cp.getboolean('logging', 'console'))
global log
log = logging.getLogger('dbloader')
except (ConfigParser.Error, ValueError, IOError) as err:
@@ -142,9 +138,13 @@ def run_as_daemon(loader, interval):
default='/etc/apel/db.cfg')
opt_parser.add_option('-c', '--config', help='Location of config file',
default='/etc/apel/loader.cfg')
- opt_parser.add_option('-l', '--log_config', help='Location of logging config file (optional)',
- default='/etc/apel/logging.cfg')
+ opt_parser.add_option('-l', '--log_config', help='DEPRECATED - location of logging config file',
+ default=None)
- (options, args) = opt_parser.parse_args()
+ options, args = opt_parser.parse_args()
- runprocess(options.db, options.config, options.log_config)
+ # Deprecating functionality.
+ if os.path.exists('/etc/apel/logging.cfg') or options.log_config is not None:
+ logging.warning('Separate logging config file option has been deprecated.')
+
+ runprocess(options.db, options.config)
diff --git a/bin/dbunloader.py b/bin/dbunloader.py
index 022ac398..4975800f 100644
--- a/bin/dbunloader.py
+++ b/bin/dbunloader.py
@@ -82,10 +82,14 @@ def _bounded_records_per_message(config_object, logger):
default='/etc/apel/db.cfg')
opt_parser.add_option('-c', '--config', help='Location of configuration file for dbunloader',
default='/etc/apel/unloader.cfg')
- opt_parser.add_option('-l', '--log_config', help='Location of logging configuration file for dbloader',
- default='/etc/apel/logging.cfg')
+ opt_parser.add_option('-l', '--log_config', help='DEPRECATED - location of logging config file',
+ default=None)
- (options, args) = opt_parser.parse_args()
+ options, args = opt_parser.parse_args()
+
+ # Deprecating functionality.
+ if os.path.exists('/etc/apel/logging.cfg') or options.log_config is not None:
+ logging.warning('Separate logging config file option has been deprecated.')
# Set default for 'interval' as it is a new option so may not be in config.
cp = ConfigParser.ConfigParser({'interval': 'latest'})
@@ -93,12 +97,8 @@ def _bounded_records_per_message(config_object, logger):
# set up logging
try:
- if os.path.exists(options.log_config):
- logging.config.fileConfig(options.log_config)
- else:
- set_up_logging(cp.get('logging', 'logfile'),
- cp.get('logging', 'level'),
- cp.getboolean('logging', 'console'))
+ set_up_logging(cp.get('logging', 'logfile'), cp.get('logging', 'level'),
+ cp.getboolean('logging', 'console'))
log = logging.getLogger('dbunloader')
except (ConfigParser.Error, ValueError, IOError) as err:
print('Error configuring logging: %s' % err)
diff --git a/bin/retrieve_dns.py b/bin/retrieve_dns.py
index acb65057..7d8be73e 100644
--- a/bin/retrieve_dns.py
+++ b/bin/retrieve_dns.py
@@ -107,12 +107,8 @@ def get_config(config_file):
# set up logging
try:
- if os.path.exists(options.log_config):
- logging.config.fileConfig(options.log_config)
- else:
- set_up_logging(cp.get('logging', 'logfile'),
- cp.get('logging', 'level'),
- cp.getboolean('logging', 'console'))
+ set_up_logging(cp.get('logging', 'logfile'), cp.get('logging', 'level'),
+ cp.getboolean('logging', 'console'))
except (ConfigParser.Error, ValueError, IOError) as err:
print('Error configuring logging: %s' % str(err))
print('The system will exit.')
@@ -256,7 +252,7 @@ def verify_dn(dn):
return True
-def runprocess(config_file, log_config_file):
+def runprocess(config_file):
'''Get DNs both from the URL and the additional file.'''
cfg = get_config(config_file)
@@ -362,8 +358,12 @@ def runprocess(config_file, log_config_file):
opt_parser = OptionParser(description=__doc__, version=ver)
opt_parser.add_option('-c', '--config', help='location of the config file',
default='/etc/apel/auth.cfg')
- opt_parser.add_option('-l', '--log_config', help='Location of logging config file (optional)',
- default='/etc/apel/logging.cfg')
- (options, args) = opt_parser.parse_args()
+ opt_parser.add_option('-l', '--log_config', help='DEPRECATED - location of logging config file',
+ default=None)
+ options, args = opt_parser.parse_args()
+
+ # Deprecating functionality.
+ if os.path.exists('/etc/apel/logging.cfg') or options.log_config is not None:
+ logging.warning('Separate logging config file option has been deprecated.')
- runprocess(options.config, options.log_config)
+ runprocess(options.config)
diff --git a/bin/summariser.py b/bin/summariser.py
index a79d8cfc..e0045c5f 100644
--- a/bin/summariser.py
+++ b/bin/summariser.py
@@ -40,7 +40,7 @@
from apel import __version__
-def runprocess(db_config_file, config_file, log_config_file):
+def runprocess(db_config_file, config_file):
'''Parse the configuration file, connect to the database and run the
summarising process.'''
@@ -73,12 +73,8 @@ def runprocess(db_config_file, config_file, log_config_file):
# set up logging
try:
- if os.path.exists(log_config_file):
- logging.config.fileConfig(log_config_file)
- else:
- set_up_logging(cp.get('logging', 'logfile'),
- cp.get('logging', 'level'),
- cp.getboolean('logging', 'console'))
+ set_up_logging(cp.get('logging', 'logfile'), cp.get('logging', 'level'),
+ cp.getboolean('logging', 'console'))
log = logging.getLogger('summariser')
except (ConfigParser.Error, ValueError, IOError) as err:
print('Error configuring logging: %s' % str(err))
@@ -186,8 +182,12 @@ def runprocess(db_config_file, config_file, log_config_file):
default='/etc/apel/db.cfg')
opt_parser.add_option('-c', '--config', help='the location of config file',
default='/etc/apel/summariser.cfg')
- opt_parser.add_option('-l', '--log_config', help='Location of logging config file (optional)',
- default='/etc/apel/logging.cfg')
- (options,args) = opt_parser.parse_args()
+ opt_parser.add_option('-l', '--log_config', help='DEPRECATED - location of logging config file',
+ default=None)
+ options,args = opt_parser.parse_args()
+
+ # Deprecating functionality.
+ if os.path.exists('/etc/apel/logging.cfg') or options.log_config is not None:
+ logging.warning('Separate logging config file option has been deprecated.')
- runprocess(options.db, options.config, options.log_config)
+ runprocess(options.db, options.config)
diff --git a/conf/logging.cfg b/conf/logging.cfg
deleted file mode 100644
index d9830569..00000000
--- a/conf/logging.cfg
+++ /dev/null
@@ -1,48 +0,0 @@
-[loggers]
-keys=root,loader,SSM,apeldb,apelldap
-
-[handlers]
-keys=consoleHandler,fileHandler
-
-[formatters]
-keys=simpleFormatter
-
-[logger_root]
-level=INFO
-handlers=consoleHandler,fileHandler
-
-[logger_loader]
-qualname=loader
-level=INFO
-handlers=consoleHandler,fileHandler
-
-[logger_apelldap]
-qualname=apelldap
-level=INFO
-handlers=consoleHandler,fileHandler
-
-[logger_SSM]
-qualname=SSM
-level=INFO
-handlers=consoleHandler,fileHandler
-
-[logger_apeldb]
-qualname=apeldb
-level=INFO
-handlers=consoleHandler,fileHandler
-
-[handler_fileHandler]
-class=FileHandler
-level=INFO
-formatter=simpleFormatter
-args=('/var/log/apel/apel.log', 'a')
-
-[handler_consoleHandler]
-class=StreamHandler
-level=INFO
-formatter=simpleFormatter
-args=(sys.stdout,)
-
-[formatter_simpleFormatter]
-format=%(asctime)s - %(name)s - %(levelname)s - %(message)s
-datefmt=
diff --git a/test/test_retrieve_dns.py b/test/test_retrieve_dns.py
index 150b4edc..5969e0b4 100644
--- a/test/test_retrieve_dns.py
+++ b/test/test_retrieve_dns.py
@@ -139,7 +139,7 @@ def test_basics(self):
/banned/dn
invalid
"""
- bin.retrieve_dns.runprocess("fake_config_file", "fake_log_config_file")
+ bin.retrieve_dns.runprocess("fake_config_file")
with open(self.files['dn']['path']) as dns:
self.assertEqual(dns.read(), "/basic/dn\n/extra/dn\n")
@@ -151,7 +151,7 @@ def test_gocdb_fail(self):
"""
self.mock_xml.return_value = ""
self.assertRaises(SystemExit, bin.retrieve_dns.runprocess,
- "fake_config_file", "fake_log_config_file")
+ "fake_config_file")
with open(self.files['dn']['path']) as dns:
self.assertEqual(dns.read(), "/dn/1\n/dn/2\n")
diff --git a/test/test_summariser.py b/test/test_summariser.py
index 6a79a9f0..306d1b77 100644
--- a/test/test_summariser.py
+++ b/test/test_summariser.py
@@ -60,7 +60,7 @@ def test_lock_file(self):
# Run the summariser with the temporary config files
try:
- summariser.runprocess(self.db_cfg_path, self.sum_cfg_path, '')
+ summariser.runprocess(self.db_cfg_path, self.sum_cfg_path)
except SystemExit as e:
self.assertEqual(e.code, 1, "Exit code '%s' is not 1." % e.code)
# A SystemExit is raised regardless of what happens