Skip to content

Commit

Permalink
Deprecate separate logging config file (apel#381)
Browse files Browse the repository at this point in the history
Remove separate logging file functionality as it's not generally used
and causes issues when accidentally included.

Much the same as apel#176 issue and apel#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
  • Loading branch information
Will-Cross1 authored Sep 12, 2024
1 parent d949f88 commit 6977feb
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 101 deletions.
17 changes: 9 additions & 8 deletions bin/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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))
Expand Down
22 changes: 11 additions & 11 deletions bin/dbloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)
18 changes: 9 additions & 9 deletions bin/dbunloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +82,23 @@ 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'})
cp.read([options.config])

# 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)
Expand Down
22 changes: 11 additions & 11 deletions bin/retrieve_dns.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
22 changes: 11 additions & 11 deletions bin/summariser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.'''

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
48 changes: 0 additions & 48 deletions conf/logging.cfg

This file was deleted.

4 changes: 2 additions & 2 deletions test/test_retrieve_dns.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def test_basics(self):
<SERVICE_ENDPOINT><HOSTDN>/banned/dn</HOSTDN></SERVICE_ENDPOINT>
<SERVICE_ENDPOINT><HOSTDN>invalid</HOSTDN></SERVICE_ENDPOINT>
</results>"""
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")

Expand All @@ -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")

Expand Down
2 changes: 1 addition & 1 deletion test/test_summariser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 6977feb

Please sign in to comment.