Merge lp:~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon into lp:zeitgeist/0.1

Proposed by Markus Korn
Status: Merged
Approved by: Seif Lotfy
Approved revision: 1623
Merged at revision: 1623
Proposed branch: lp:~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon
Merge into: lp:zeitgeist/0.1
Diff against target: 300 lines (+86/-79)
10 files modified
_zeitgeist/engine/__init__.py (+0/-1)
_zeitgeist/engine/extension.py (+0/-1)
_zeitgeist/engine/extensions/blacklist.py (+0/-1)
_zeitgeist/engine/extensions/datasource_registry.py (+0/-1)
_zeitgeist/engine/main.py (+0/-1)
_zeitgeist/engine/notify.py (+0/-1)
_zeitgeist/engine/sql.py (+0/-1)
_zeitgeist/engine/upgrades/core_0_1.py (+0/-1)
zeitgeist-daemon.py (+86/-70)
zeitgeist/client.py (+0/-1)
To merge this branch: bzr merge lp:~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon
Reviewer Review Type Date Requested Status
Seif Lotfy Approve
Review via email: mp+38914@code.launchpad.net

Description of the change

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/

To post a comment you must log in.
Revision history for this message
Siegfried Gevatter (rainct) wrote :

2010/10/20 Markus Korn <email address hidden>:
> +       logging.info("Setup RemoteInterface")
> +       logging.info("Connect to SIGHUB signal")

These messages are pretty useless, I don't really want to see them.

Looks good otherwise, great work.

--
Siegfried-Angel Gevatter Pujals (RainCT)
Free Software Developer       363DEAE3

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

review approve

I second Siegfrieds comment though. And please make sure the man page is
clear on the subject of debug logging

1623. By Markus Korn

zeitgeist-daemon.py: removed some redundant logging statements

Revision history for this message
Markus Korn (thekorn) wrote :

Ok, I've removed the logging statements you both mentioned.

> review approve
>
> I second Siegfrieds comment though. And please make sure the man page is
> clear on the subject of debug logging.

man zeitgeist-daemon says:

       --log-level=LOG_LEVEL
              Specifies how much information should be printed to the standard output. Possible values are: DEBUG,
              INFO, WARNING, ERROR, CRITICAL.

is it what you want, or is sth. missing?

Revision history for this message
Seif Lotfy (seif) wrote :

On Wed, Oct 20, 2010 at 12:27 PM, Markus Korn <email address hidden> wrote:

> Ok, I've removed the logging statements you both mentioned.
>
> > review approve
> >
> > I second Siegfrieds comment though. And please make sure the man page is
> > clear on the subject of debug logging.
>
> man zeitgeist-daemon says:
>
> --log-level=LOG_LEVEL
> Specifies how much information should be printed to the
> standard output. Possible values are: DEBUG,
> INFO, WARNING, ERROR, CRITICAL.
>
> is it what you want, or is sth. missing?
> --
>
> https://code.launchpad.net/~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon/+merge/38914
> You are subscribed to branch lp:zeitgeist.
>

Good Job! Approved!

Revision history for this message
Seif Lotfy (seif) wrote :

<seif_> RainCT, can u look at thekorn's https://code.edge.launchpad.net/~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon/+merge/38914
<seif_> so we can get it in
<seif_> its very very simple
<seif_> RainCT, please
<RainCT> seif_: it's already approved
<seif_> by me
<seif_> can u approve it to
<seif_> ?
<RainCT> and mikkel
<RainCT> and my comment also suggests that it's ok except for what he already fixed

Revision history for this message
Seif Lotfy (seif) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '_zeitgeist/engine/__init__.py'
--- _zeitgeist/engine/__init__.py 2010-09-19 14:15:21 +0000
+++ _zeitgeist/engine/__init__.py 2010-10-20 10:26:03 +0000
@@ -31,7 +31,6 @@
31 "constants"31 "constants"
32]32]
3333
34logging.basicConfig(level=logging.DEBUG)
35log = logging.getLogger("zeitgeist.engine")34log = logging.getLogger("zeitgeist.engine")
3635
37_engine = None36_engine = None
3837
=== modified file '_zeitgeist/engine/extension.py'
--- _zeitgeist/engine/extension.py 2010-09-18 14:30:45 +0000
+++ _zeitgeist/engine/extension.py 2010-10-20 10:26:03 +0000
@@ -22,7 +22,6 @@
22import logging22import logging
23import weakref # avoid circular references as they confuse garbage collection23import weakref # avoid circular references as they confuse garbage collection
2424
25logging.basicConfig(level=logging.DEBUG)
26log = logging.getLogger("zeitgeist.extension")25log = logging.getLogger("zeitgeist.extension")
2726
28import zeitgeist27import zeitgeist
2928
=== 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 10:26:03 +0000
@@ -28,7 +28,6 @@
28from _zeitgeist.engine.extension import Extension28from _zeitgeist.engine.extension import Extension
29from _zeitgeist.engine import constants29from _zeitgeist.engine import constants
3030
31logging.basicConfig(level=logging.DEBUG)
32log = logging.getLogger("zeitgeist.blacklist")31log = logging.getLogger("zeitgeist.blacklist")
3332
34CONFIG_FILE = os.path.join(constants.DATA_PATH, "blacklist.pickle")33CONFIG_FILE = os.path.join(constants.DATA_PATH, "blacklist.pickle")
3534
=== 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 10:26:03 +0000
@@ -30,7 +30,6 @@
30from _zeitgeist.engine.extension import Extension30from _zeitgeist.engine.extension import Extension
31from _zeitgeist.engine import constants31from _zeitgeist.engine import constants
3232
33logging.basicConfig(level=logging.DEBUG)
34log = logging.getLogger("zeitgeist.datasource_registry")33log = logging.getLogger("zeitgeist.datasource_registry")
3534
36DATA_FILE = os.path.join(constants.DATA_PATH, "datasources.pickle")35DATA_FILE = os.path.join(constants.DATA_PATH, "datasources.pickle")
3736
=== modified file '_zeitgeist/engine/main.py'
--- _zeitgeist/engine/main.py 2010-10-19 10:25:33 +0000
+++ _zeitgeist/engine/main.py 2010-10-20 10:26:03 +0000
@@ -35,7 +35,6 @@
35from _zeitgeist.engine.sql import get_default_cursor, unset_cursor, \35from _zeitgeist.engine.sql import get_default_cursor, unset_cursor, \
36 TableLookup, WhereClause36 TableLookup, WhereClause
3737
38logging.basicConfig(level=logging.DEBUG)
39log = logging.getLogger("zeitgeist.engine")38log = logging.getLogger("zeitgeist.engine")
4039
41class NegationNotSupported(ValueError):40class NegationNotSupported(ValueError):
4241
=== modified file '_zeitgeist/engine/notify.py'
--- _zeitgeist/engine/notify.py 2010-09-25 13:19:51 +0000
+++ _zeitgeist/engine/notify.py 2010-10-20 10:26:03 +0000
@@ -22,7 +22,6 @@
2222
23from zeitgeist.datamodel import TimeRange23from zeitgeist.datamodel import TimeRange
2424
25logging.basicConfig(level=logging.DEBUG)
26log = logging.getLogger("zeitgeist.notify")25log = logging.getLogger("zeitgeist.notify")
2726
28class _MonitorProxy (dbus.Interface):27class _MonitorProxy (dbus.Interface):
2928
=== modified file '_zeitgeist/engine/sql.py'
--- _zeitgeist/engine/sql.py 2010-09-21 16:15:14 +0000
+++ _zeitgeist/engine/sql.py 2010-10-20 10:26:03 +0000
@@ -27,7 +27,6 @@
2727
28from _zeitgeist.engine import constants28from _zeitgeist.engine import constants
2929
30logging.basicConfig(level=logging.DEBUG)
31log = logging.getLogger("zeitgeist.sql")30log = logging.getLogger("zeitgeist.sql")
3231
33TABLE_MAP = {32TABLE_MAP = {
3433
=== 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 10:26:03 +0000
@@ -3,7 +3,6 @@
3import logging3import logging
4import sqlite34import sqlite3
55
6logging.basicConfig(level=logging.DEBUG)
7log = logging.getLogger("zeitgeist.sql")6log = logging.getLogger("zeitgeist.sql")
87
9INTERPRETATION_RENAMES = \8INTERPRETATION_RENAMES = \
109
=== modified file 'zeitgeist-daemon.py'
--- zeitgeist-daemon.py 2010-10-14 09:42:08 +0000
+++ zeitgeist-daemon.py 2010-10-20 10:26:03 +0000
@@ -4,6 +4,7 @@
4# Zeitgeist4# Zeitgeist
5#5#
6# Copyright © 2009 Siegfried-Angel Gevatter Pujals <rainct@ubuntu.com>6# Copyright © 2009 Siegfried-Angel Gevatter Pujals <rainct@ubuntu.com>
7# Copyright © 2010 Markus Korn <thekorn@gmx.de>
7#8#
8# This program is free software: you can redistribute it and/or modify9# This program is free software: you can redistribute it and/or modify
9# it under the terms of the GNU Lesser General Public License as published by10# it under the terms of the GNU Lesser General Public License as published by
@@ -26,7 +27,6 @@
26import logging27import logging
27import optparse28import optparse
28import signal29import signal
29from copy import copy
30from subprocess import Popen, PIPE30from subprocess import Popen, PIPE
3131
32# Make sure we can find the private _zeitgeist namespace32# Make sure we can find the private _zeitgeist namespace
@@ -38,75 +38,68 @@
38from _zeitgeist.engine import constants38from _zeitgeist.engine import constants
39sys.path.insert(0, constants.USER_EXTENSION_PATH)39sys.path.insert(0, constants.USER_EXTENSION_PATH)
4040
41gettext.install("zeitgeist", _config.localedir, unicode=1)41gettext.install("zeitgeist", _config.localedir, unicode=True)
42DATAHUB = "zeitgeist-datahub"42
4343class Options(optparse.Option):
44def check_loglevel(option, opt, value):44 TYPES = optparse.Option.TYPES + ("log_levels",)
45 value = value.upper()45 TYPE_CHECKER = optparse.Option.TYPE_CHECKER.copy()
46 if value in Options.log_levels:46 log_levels = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL")
47 return value47
48 raise optparse.OptionValueError(48 def check_loglevel(option, opt, value):
49 "option %s: invalid value: %s" % (opt, value))49 value = value.upper()
50 50 if value in Options.log_levels:
51 return value
52 raise optparse.OptionValueError(
53 "option %s: invalid value: %s" % (opt, value))
54 TYPE_CHECKER["log_levels"] = check_loglevel
55
56
51def which(executable):57def which(executable):
58 """ helper to get the complete path to an executable """
52 p = Popen(["which", str(executable)], stderr=PIPE, stdout=PIPE)59 p = Popen(["which", str(executable)], stderr=PIPE, stdout=PIPE)
53 p.wait()60 p.wait()
54 return p.stdout.read().strip() or None61 return p.stdout.read().strip() or None
5562
56class Options(optparse.Option):63def parse_commandline():
5764 parser = optparse.OptionParser(version = _config.VERSION, option_class=Options)
58 TYPES = optparse.Option.TYPES + ("log_levels",)65 parser.add_option(
59 TYPE_CHECKER = copy(optparse.Option.TYPE_CHECKER)66 "-r", "--replace",
60 TYPE_CHECKER["log_levels"] = check_loglevel67 action="store_true", default=False, dest="replace",
6168 help=_("if another Zeitgeist instance is already running, replace it"))
62 log_levels = ('DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL')69 parser.add_option(
6370 "--no-datahub", "--no-passive-loggers",
64parser = optparse.OptionParser(version = _config.VERSION, option_class=Options)71 action="store_false", default=True, dest="start_datahub",
65parser.add_option(72 help=_("do not start zeitgeist-datahub automatically"))
66 "-r", "--replace",73 parser.add_option(
67 action = "store_true", default=False, dest = "replace",74 "--log-level",
68 help = _("if another Zeitgeist instance is already running, replace it"))75 action="store", type="log_levels", default="DEBUG", dest="log_level",
69parser.add_option(76 help=_("how much information should be printed; possible values:") + \
70 "--no-datahub", "--no-passive-loggers",77 " %s" % ", ".join(Options.log_levels))
71 action = "store_false", default=True, dest = "start_datahub",78 parser.add_option(
72 help = _("do not start zeitgeist-datahub automatically"))79 "--quit",
73parser.add_option(80 action="store_true", default=False, dest="quit",
74 "--log-level",81 help=_("if another Zeitgeist instance is already running, replace it"))
75 action = "store", type="log_levels", default="DEBUG", dest="log_level",82 parser.add_option(
76 help = _("how much information should be printed; possible values:") + \83 "--shell-completion",
77 " %s" % ', '.join(Options.log_levels))84 action="store_true", default=False, dest="shell_completion",
78parser.add_option(85 help=optparse.SUPPRESS_HELP)
79 "--quit",86 return parser
80 action = "store_true", default=False, dest = "quit",87
81 help = _("if another Zeitgeist instance is already running, replace it"))88def do_shell_completion(parser):
82parser.add_option(
83 "--shell-completion",
84 action = "store_true", default=False, dest = "shell_completion",
85 help = optparse.SUPPRESS_HELP)
86
87(_config.options, _config.arguments) = parser.parse_args()
88
89if _config.options.shell_completion:
90 options = set()89 options = set()
91 for option in (str(option) for option in parser.option_list):90 for option in (str(option) for option in parser.option_list):
92 options.update(option.split("/"))91 options.update(option.split("/"))
93 print ' '.join(options)92 print " ".join(options)
94 sys.exit(0)93 return 0
9594
96logging.basicConfig(level=getattr(logging, _config.options.log_level))95def setup_interface():
9796 from _zeitgeist.engine.remote import RemoteInterface
98from _zeitgeist.engine.remote import RemoteInterface97 dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
9998 mainloop = gobject.MainLoop()
100dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)99 return mainloop, RemoteInterface(mainloop = mainloop)
101mainloop = gobject.MainLoop()100
102101def start_datahub():
103try:102 DATAHUB = "zeitgeist-datahub"
104 interface = RemoteInterface(mainloop = mainloop)
105except RuntimeError, e:
106 logging.error(unicode(e))
107 sys.exit(1)
108
109if _config.options.start_datahub:
110 # hide all output of the datahub for now,103 # hide all output of the datahub for now,
111 # in the future we might want to be more verbose here to make104 # in the future we might want to be more verbose here to make
112 # debugging easier in case sth. goes wrong with the datahub105 # debugging easier in case sth. goes wrong with the datahub
@@ -122,11 +115,34 @@
122 # tell the user which datahub we are running115 # tell the user which datahub we are running
123 logging.debug("Running datahub (%s) with PID=%i" %(which(DATAHUB), p.pid))116 logging.debug("Running datahub (%s) with PID=%i" %(which(DATAHUB), p.pid))
124117
125def handle_sighup(signum, frame):118def setup_handle_sighup(interface):
126 """We are using the SIGHUP signal to shutdown zeitgeist in a clean way"""119 def handle_sighup(signum, frame):
127 logging.info("got SIGHUP signal, shutting down zeitgeist interface")120 """We are using the SIGHUP signal to shutdown zeitgeist in a clean way"""
128 interface.Quit()121 logging.info("got SIGHUP signal, shutting down zeitgeist interface")
129signal.signal(signal.SIGHUP, handle_sighup)122 interface.Quit()
130123 return handle_sighup
131logging.info("Starting Zeitgeist service...")124
132mainloop.run()125if __name__ == "__main__":
126
127 parser = parse_commandline()
128
129 _config.options, _config.arguments = parser.parse_args()
130 if _config.options.shell_completion:
131 sys.exit(do_shell_completion(parser))
132
133 logging.basicConfig(level=getattr(logging, _config.options.log_level))
134
135 try:
136 mainloop, interface = setup_interface()
137 except RuntimeError, e:
138 logging.exception("Failed to setup the RemoteInterface")
139 sys.exit(1)
140
141 if _config.options.start_datahub:
142 logging.info("Trying to start the datahub")
143 start_datahub()
144
145 signal.signal(signal.SIGHUP, setup_handle_sighup(interface))
146
147 logging.info("Starting Zeitgeist service...")
148 mainloop.run()
133149
=== modified file 'zeitgeist/client.py'
--- zeitgeist/client.py 2010-08-26 18:38:47 +0000
+++ zeitgeist/client.py 2010-10-20 10:26:03 +0000
@@ -37,7 +37,6 @@
3737
38SIG_EVENT = "asaasay"38SIG_EVENT = "asaasay"
3939
40logging.basicConfig(level=logging.DEBUG)
41log = logging.getLogger("zeitgeist.client")40log = logging.getLogger("zeitgeist.client")
4241
43class _DBusInterface(object):42class _DBusInterface(object):

Subscribers

People subscribed via source and target branches