zeitgeist team mailing list archive

[Markus Korn <thekorn@xxxxxx>]

Markus Korn has proposed merging lp:~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon into lp:zeitgeist.

Requested reviews:
  Zeitgeist Framework Team (zeitgeist)
Related bugs:
  #660415 zeitgeist-daemon.py has a bad code structure
  https://bugs.launchpad.net/bugs/660415


zeitgeist-daemon.py has now a much more readable code structure (LP: #660415)
While I was on it I also fixed the `--log-level` option, by removing logging.basicConfig() from all modules in _zeitgeist/ and zeitgeist/
-- 
https://code.launchpad.net/~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon/+merge/38914
Your team Zeitgeist Framework Team is requested to review the proposed merge of lp:~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon into lp:zeitgeist.

=== modified file '_zeitgeist/engine/__init__.py'
--- _zeitgeist/engine/__init__.py	2010-09-19 14:15:21 +0000
+++ _zeitgeist/engine/__init__.py	2010-10-20 07:31:07 +0000
@@ -31,7 +31,6 @@
 	"constants"
 ]
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.engine")
 
 _engine = None

=== modified file '_zeitgeist/engine/extension.py'
--- _zeitgeist/engine/extension.py	2010-09-18 14:30:45 +0000
+++ _zeitgeist/engine/extension.py	2010-10-20 07:31:07 +0000
@@ -22,7 +22,6 @@
 import logging
 import weakref # avoid circular references as they confuse garbage collection
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.extension")
 
 import zeitgeist

=== modified file '_zeitgeist/engine/extensions/blacklist.py'
--- _zeitgeist/engine/extensions/blacklist.py	2010-07-31 13:13:22 +0000
+++ _zeitgeist/engine/extensions/blacklist.py	2010-10-20 07:31:07 +0000
@@ -28,7 +28,6 @@
 from _zeitgeist.engine.extension import Extension
 from _zeitgeist.engine import constants
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.blacklist")
 
 CONFIG_FILE = os.path.join(constants.DATA_PATH, "blacklist.pickle")

=== modified file '_zeitgeist/engine/extensions/datasource_registry.py'
--- _zeitgeist/engine/extensions/datasource_registry.py	2010-08-28 15:08:49 +0000
+++ _zeitgeist/engine/extensions/datasource_registry.py	2010-10-20 07:31:07 +0000
@@ -30,7 +30,6 @@
 from _zeitgeist.engine.extension import Extension
 from _zeitgeist.engine import constants
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.datasource_registry")
 
 DATA_FILE = os.path.join(constants.DATA_PATH, "datasources.pickle")

=== modified file '_zeitgeist/engine/main.py'
--- _zeitgeist/engine/main.py	2010-10-19 10:25:33 +0000
+++ _zeitgeist/engine/main.py	2010-10-20 07:31:07 +0000
@@ -35,7 +35,6 @@
 from _zeitgeist.engine.sql import get_default_cursor, unset_cursor, \
 	TableLookup, WhereClause
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.engine")
 
 class NegationNotSupported(ValueError):

=== modified file '_zeitgeist/engine/notify.py'
--- _zeitgeist/engine/notify.py	2010-09-25 13:19:51 +0000
+++ _zeitgeist/engine/notify.py	2010-10-20 07:31:07 +0000
@@ -22,7 +22,6 @@
 
 from zeitgeist.datamodel import TimeRange
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.notify")
 
 class _MonitorProxy (dbus.Interface):

=== modified file '_zeitgeist/engine/sql.py'
--- _zeitgeist/engine/sql.py	2010-09-21 16:15:14 +0000
+++ _zeitgeist/engine/sql.py	2010-10-20 07:31:07 +0000
@@ -27,7 +27,6 @@
 
 from _zeitgeist.engine import constants
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.sql")
 
 TABLE_MAP = {

=== modified file '_zeitgeist/engine/upgrades/core_0_1.py'
--- _zeitgeist/engine/upgrades/core_0_1.py	2010-06-09 06:29:47 +0000
+++ _zeitgeist/engine/upgrades/core_0_1.py	2010-10-20 07:31:07 +0000
@@ -3,7 +3,6 @@
 import logging
 import sqlite3
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.sql")
 
 INTERPRETATION_RENAMES = \

=== modified file 'zeitgeist-daemon.py'
--- zeitgeist-daemon.py	2010-10-14 09:42:08 +0000
+++ zeitgeist-daemon.py	2010-10-20 07:31:07 +0000
@@ -4,6 +4,7 @@
 # Zeitgeist
 #
 # Copyright © 2009 Siegfried-Angel Gevatter Pujals <rainct@xxxxxxxxxx>
+# Copyright © 2010 Markus Korn <thekorn@xxxxxx>
 #
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Lesser General Public License as published by
@@ -26,7 +27,6 @@
 import logging
 import optparse
 import signal
-from copy import copy
 from subprocess import Popen, PIPE
 
 # Make sure we can find the private _zeitgeist namespace
@@ -38,75 +38,68 @@
 from _zeitgeist.engine import constants
 sys.path.insert(0, constants.USER_EXTENSION_PATH)
 
-gettext.install("zeitgeist", _config.localedir, unicode=1)
-DATAHUB = "zeitgeist-datahub"
-
-def check_loglevel(option, opt, value):
-	value = value.upper()
-	if value in Options.log_levels:
-		return value
-	raise optparse.OptionValueError(
-		"option %s: invalid value: %s" % (opt, value))
-		
+gettext.install("zeitgeist", _config.localedir, unicode=True)
+
+class Options(optparse.Option):
+	TYPES = optparse.Option.TYPES + ("log_levels",)
+	TYPE_CHECKER = optparse.Option.TYPE_CHECKER.copy()
+	log_levels = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL")
+
+	def check_loglevel(option, opt, value):
+		value = value.upper()
+		if value in Options.log_levels:
+			return value
+		raise optparse.OptionValueError(
+			"option %s: invalid value: %s" % (opt, value))
+	TYPE_CHECKER["log_levels"] = check_loglevel
+
+
 def which(executable):
+	""" helper to get the complete path to an executable """
 	p = Popen(["which", str(executable)], stderr=PIPE, stdout=PIPE)
 	p.wait()
 	return p.stdout.read().strip() or None
 
-class Options(optparse.Option):
-
-	TYPES = optparse.Option.TYPES + ("log_levels",)
-	TYPE_CHECKER = copy(optparse.Option.TYPE_CHECKER)
-	TYPE_CHECKER["log_levels"] = check_loglevel
-
-	log_levels = ('DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL')
-
-parser = optparse.OptionParser(version = _config.VERSION, option_class=Options)
-parser.add_option(
-	"-r", "--replace",
-	action = "store_true", default=False, dest = "replace",
-	help = _("if another Zeitgeist instance is already running, replace it"))
-parser.add_option(
-	"--no-datahub", "--no-passive-loggers",
-	action = "store_false", default=True, dest = "start_datahub",
-	help = _("do not start zeitgeist-datahub automatically"))
-parser.add_option(
-	"--log-level",
-	action = "store", type="log_levels", default="DEBUG", dest="log_level",
-	help = _("how much information should be printed; possible values:") + \
-		" %s" % ', '.join(Options.log_levels))
-parser.add_option(
-	"--quit",
-	action = "store_true", default=False, dest = "quit",
-	help = _("if another Zeitgeist instance is already running, replace it"))
-parser.add_option(
-	"--shell-completion",
-	action = "store_true", default=False, dest = "shell_completion",
-	help = optparse.SUPPRESS_HELP)
-
-(_config.options, _config.arguments) = parser.parse_args()
-
-if _config.options.shell_completion:
+def parse_commandline():
+	parser = optparse.OptionParser(version = _config.VERSION, option_class=Options)
+	parser.add_option(
+		"-r", "--replace",
+		action="store_true", default=False, dest="replace",
+		help=_("if another Zeitgeist instance is already running, replace it"))
+	parser.add_option(
+		"--no-datahub", "--no-passive-loggers",
+		action="store_false", default=True, dest="start_datahub",
+		help=_("do not start zeitgeist-datahub automatically"))
+	parser.add_option(
+		"--log-level",
+		action="store", type="log_levels", default="DEBUG", dest="log_level",
+		help=_("how much information should be printed; possible values:") + \
+			" %s" % ", ".join(Options.log_levels))
+	parser.add_option(
+		"--quit",
+		action="store_true", default=False, dest="quit",
+		help=_("if another Zeitgeist instance is already running, replace it"))
+	parser.add_option(
+		"--shell-completion",
+		action="store_true", default=False, dest="shell_completion",
+		help=optparse.SUPPRESS_HELP)
+	return parser
+
+def do_shell_completion(parser):
 	options = set()
 	for option in (str(option) for option in parser.option_list):
 		options.update(option.split("/"))
-	print ' '.join(options)
-	sys.exit(0)
-
-logging.basicConfig(level=getattr(logging, _config.options.log_level))
-
-from _zeitgeist.engine.remote import RemoteInterface
-
-dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
-mainloop = gobject.MainLoop()
-
-try:
-	interface = RemoteInterface(mainloop = mainloop)
-except RuntimeError, e:
-	logging.error(unicode(e))
-	sys.exit(1)
-
-if _config.options.start_datahub:
+	print " ".join(options)
+	return 0
+
+def setup_interface():
+	from _zeitgeist.engine.remote import RemoteInterface
+	dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+	mainloop = gobject.MainLoop()
+	return mainloop, RemoteInterface(mainloop = mainloop)
+
+def start_datahub():
+	DATAHUB = "zeitgeist-datahub"
 	# hide all output of the datahub for now,
 	# in the future we might want to be more verbose here to make
 	# debugging easier in case sth. goes wrong with the datahub
@@ -122,11 +115,36 @@
 		# tell the user which datahub we are running
 		logging.debug("Running datahub (%s) with PID=%i" %(which(DATAHUB), p.pid))
 
-def handle_sighup(signum, frame):
-	"""We are using the SIGHUP signal to shutdown zeitgeist in a clean way"""
-	logging.info("got SIGHUP signal, shutting down zeitgeist interface")
-	interface.Quit()
-signal.signal(signal.SIGHUP, handle_sighup)
-
-logging.info("Starting Zeitgeist service...")
-mainloop.run()
+def setup_handle_sighup(interface):
+	def handle_sighup(signum, frame):
+		"""We are using the SIGHUP signal to shutdown zeitgeist in a clean way"""
+		logging.info("got SIGHUP signal, shutting down zeitgeist interface")
+		interface.Quit()
+	return handle_sighup
+	
+if __name__ == "__main__":
+	
+	parser = parse_commandline()
+	
+	_config.options, _config.arguments = parser.parse_args()
+	if _config.options.shell_completion:
+		sys.exit(do_shell_completion(parser))
+	
+	logging.basicConfig(level=getattr(logging, _config.options.log_level))
+	
+	logging.info("Setup RemoteInterface")
+	try:
+		mainloop, interface = setup_interface()
+	except RuntimeError, e:
+		logging.exception("Failed to setup the RemoteInterface")
+		sys.exit(1)
+	
+	if _config.options.start_datahub:
+		logging.info("Trying to start the datahub")
+		start_datahub()
+	
+	logging.info("Connect to SIGHUB signal")
+	signal.signal(signal.SIGHUP, setup_handle_sighup(interface))
+	
+	logging.info("Starting Zeitgeist service...")
+	mainloop.run()

=== modified file 'zeitgeist/client.py'
--- zeitgeist/client.py	2010-08-26 18:38:47 +0000
+++ zeitgeist/client.py	2010-10-20 07:31:07 +0000
@@ -37,7 +37,6 @@
 
 SIG_EVENT = "asaasay"
 
-logging.basicConfig(level=logging.DEBUG)
 log = logging.getLogger("zeitgeist.client")
 
 class _DBusInterface(object):