Zeitgeist Framework

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

  1. fix-660415-improve-zeitgeist-daemon
  2. Merge into 0.8-python
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

lp:~thekorn/zeitgeist/fix-660415-improve-zeitgeist-daemon updated
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
1=== modified file '_zeitgeist/engine/__init__.py'
2--- _zeitgeist/engine/__init__.py 2010-09-19 14:15:21 +0000
3+++ _zeitgeist/engine/__init__.py 2010-10-20 10:26:03 +0000
4@@ -31,7 +31,6 @@
5 "constants"
6 ]
7
8-logging.basicConfig(level=logging.DEBUG)
9 log = logging.getLogger("zeitgeist.engine")
10
11 _engine = None
12
13=== modified file '_zeitgeist/engine/extension.py'
14--- _zeitgeist/engine/extension.py 2010-09-18 14:30:45 +0000
15+++ _zeitgeist/engine/extension.py 2010-10-20 10:26:03 +0000
16@@ -22,7 +22,6 @@
17 import logging
18 import weakref # avoid circular references as they confuse garbage collection
19
20-logging.basicConfig(level=logging.DEBUG)
21 log = logging.getLogger("zeitgeist.extension")
22
23 import zeitgeist
24
25=== modified file '_zeitgeist/engine/extensions/blacklist.py'
26--- _zeitgeist/engine/extensions/blacklist.py 2010-07-31 13:13:22 +0000
27+++ _zeitgeist/engine/extensions/blacklist.py 2010-10-20 10:26:03 +0000
28@@ -28,7 +28,6 @@
29 from _zeitgeist.engine.extension import Extension
30 from _zeitgeist.engine import constants
31
32-logging.basicConfig(level=logging.DEBUG)
33 log = logging.getLogger("zeitgeist.blacklist")
34
35 CONFIG_FILE = os.path.join(constants.DATA_PATH, "blacklist.pickle")
36
37=== modified file '_zeitgeist/engine/extensions/datasource_registry.py'
38--- _zeitgeist/engine/extensions/datasource_registry.py 2010-08-28 15:08:49 +0000
39+++ _zeitgeist/engine/extensions/datasource_registry.py 2010-10-20 10:26:03 +0000
40@@ -30,7 +30,6 @@
41 from _zeitgeist.engine.extension import Extension
42 from _zeitgeist.engine import constants
43
44-logging.basicConfig(level=logging.DEBUG)
45 log = logging.getLogger("zeitgeist.datasource_registry")
46
47 DATA_FILE = os.path.join(constants.DATA_PATH, "datasources.pickle")
48
49=== modified file '_zeitgeist/engine/main.py'
50--- _zeitgeist/engine/main.py 2010-10-19 10:25:33 +0000
51+++ _zeitgeist/engine/main.py 2010-10-20 10:26:03 +0000
52@@ -35,7 +35,6 @@
53 from _zeitgeist.engine.sql import get_default_cursor, unset_cursor, \
54 TableLookup, WhereClause
55
56-logging.basicConfig(level=logging.DEBUG)
57 log = logging.getLogger("zeitgeist.engine")
58
59 class NegationNotSupported(ValueError):
60
61=== modified file '_zeitgeist/engine/notify.py'
62--- _zeitgeist/engine/notify.py 2010-09-25 13:19:51 +0000
63+++ _zeitgeist/engine/notify.py 2010-10-20 10:26:03 +0000
64@@ -22,7 +22,6 @@
65
66 from zeitgeist.datamodel import TimeRange
67
68-logging.basicConfig(level=logging.DEBUG)
69 log = logging.getLogger("zeitgeist.notify")
70
71 class _MonitorProxy (dbus.Interface):
72
73=== modified file '_zeitgeist/engine/sql.py'
74--- _zeitgeist/engine/sql.py 2010-09-21 16:15:14 +0000
75+++ _zeitgeist/engine/sql.py 2010-10-20 10:26:03 +0000
76@@ -27,7 +27,6 @@
77
78 from _zeitgeist.engine import constants
79
80-logging.basicConfig(level=logging.DEBUG)
81 log = logging.getLogger("zeitgeist.sql")
82
83 TABLE_MAP = {
84
85=== modified file '_zeitgeist/engine/upgrades/core_0_1.py'
86--- _zeitgeist/engine/upgrades/core_0_1.py 2010-06-09 06:29:47 +0000
87+++ _zeitgeist/engine/upgrades/core_0_1.py 2010-10-20 10:26:03 +0000
88@@ -3,7 +3,6 @@
89 import logging
90 import sqlite3
91
92-logging.basicConfig(level=logging.DEBUG)
93 log = logging.getLogger("zeitgeist.sql")
94
95 INTERPRETATION_RENAMES = \
96
97=== modified file 'zeitgeist-daemon.py'
98--- zeitgeist-daemon.py 2010-10-14 09:42:08 +0000
99+++ zeitgeist-daemon.py 2010-10-20 10:26:03 +0000
100@@ -4,6 +4,7 @@
101 # Zeitgeist
102 #
103 # Copyright © 2009 Siegfried-Angel Gevatter Pujals <rainct@ubuntu.com>
104+# Copyright © 2010 Markus Korn <thekorn@gmx.de>
105 #
106 # This program is free software: you can redistribute it and/or modify
107 # it under the terms of the GNU Lesser General Public License as published by
108@@ -26,7 +27,6 @@
109 import logging
110 import optparse
111 import signal
112-from copy import copy
113 from subprocess import Popen, PIPE
114
115 # Make sure we can find the private _zeitgeist namespace
116@@ -38,75 +38,68 @@
117 from _zeitgeist.engine import constants
118 sys.path.insert(0, constants.USER_EXTENSION_PATH)
119
120-gettext.install("zeitgeist", _config.localedir, unicode=1)
121-DATAHUB = "zeitgeist-datahub"
122-
123-def check_loglevel(option, opt, value):
124- value = value.upper()
125- if value in Options.log_levels:
126- return value
127- raise optparse.OptionValueError(
128- "option %s: invalid value: %s" % (opt, value))
129-
130+gettext.install("zeitgeist", _config.localedir, unicode=True)
131+
132+class Options(optparse.Option):
133+ TYPES = optparse.Option.TYPES + ("log_levels",)
134+ TYPE_CHECKER = optparse.Option.TYPE_CHECKER.copy()
135+ log_levels = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL")
136+
137+ def check_loglevel(option, opt, value):
138+ value = value.upper()
139+ if value in Options.log_levels:
140+ return value
141+ raise optparse.OptionValueError(
142+ "option %s: invalid value: %s" % (opt, value))
143+ TYPE_CHECKER["log_levels"] = check_loglevel
144+
145+
146 def which(executable):
147+ """ helper to get the complete path to an executable """
148 p = Popen(["which", str(executable)], stderr=PIPE, stdout=PIPE)
149 p.wait()
150 return p.stdout.read().strip() or None
151
152-class Options(optparse.Option):
153-
154- TYPES = optparse.Option.TYPES + ("log_levels",)
155- TYPE_CHECKER = copy(optparse.Option.TYPE_CHECKER)
156- TYPE_CHECKER["log_levels"] = check_loglevel
157-
158- log_levels = ('DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL')
159-
160-parser = optparse.OptionParser(version = _config.VERSION, option_class=Options)
161-parser.add_option(
162- "-r", "--replace",
163- action = "store_true", default=False, dest = "replace",
164- help = _("if another Zeitgeist instance is already running, replace it"))
165-parser.add_option(
166- "--no-datahub", "--no-passive-loggers",
167- action = "store_false", default=True, dest = "start_datahub",
168- help = _("do not start zeitgeist-datahub automatically"))
169-parser.add_option(
170- "--log-level",
171- action = "store", type="log_levels", default="DEBUG", dest="log_level",
172- help = _("how much information should be printed; possible values:") + \
173- " %s" % ', '.join(Options.log_levels))
174-parser.add_option(
175- "--quit",
176- action = "store_true", default=False, dest = "quit",
177- help = _("if another Zeitgeist instance is already running, replace it"))
178-parser.add_option(
179- "--shell-completion",
180- action = "store_true", default=False, dest = "shell_completion",
181- help = optparse.SUPPRESS_HELP)
182-
183-(_config.options, _config.arguments) = parser.parse_args()
184-
185-if _config.options.shell_completion:
186+def parse_commandline():
187+ parser = optparse.OptionParser(version = _config.VERSION, option_class=Options)
188+ parser.add_option(
189+ "-r", "--replace",
190+ action="store_true", default=False, dest="replace",
191+ help=_("if another Zeitgeist instance is already running, replace it"))
192+ parser.add_option(
193+ "--no-datahub", "--no-passive-loggers",
194+ action="store_false", default=True, dest="start_datahub",
195+ help=_("do not start zeitgeist-datahub automatically"))
196+ parser.add_option(
197+ "--log-level",
198+ action="store", type="log_levels", default="DEBUG", dest="log_level",
199+ help=_("how much information should be printed; possible values:") + \
200+ " %s" % ", ".join(Options.log_levels))
201+ parser.add_option(
202+ "--quit",
203+ action="store_true", default=False, dest="quit",
204+ help=_("if another Zeitgeist instance is already running, replace it"))
205+ parser.add_option(
206+ "--shell-completion",
207+ action="store_true", default=False, dest="shell_completion",
208+ help=optparse.SUPPRESS_HELP)
209+ return parser
210+
211+def do_shell_completion(parser):
212 options = set()
213 for option in (str(option) for option in parser.option_list):
214 options.update(option.split("/"))
215- print ' '.join(options)
216- sys.exit(0)
217-
218-logging.basicConfig(level=getattr(logging, _config.options.log_level))
219-
220-from _zeitgeist.engine.remote import RemoteInterface
221-
222-dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
223-mainloop = gobject.MainLoop()
224-
225-try:
226- interface = RemoteInterface(mainloop = mainloop)
227-except RuntimeError, e:
228- logging.error(unicode(e))
229- sys.exit(1)
230-
231-if _config.options.start_datahub:
232+ print " ".join(options)
233+ return 0
234+
235+def setup_interface():
236+ from _zeitgeist.engine.remote import RemoteInterface
237+ dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
238+ mainloop = gobject.MainLoop()
239+ return mainloop, RemoteInterface(mainloop = mainloop)
240+
241+def start_datahub():
242+ DATAHUB = "zeitgeist-datahub"
243 # hide all output of the datahub for now,
244 # in the future we might want to be more verbose here to make
245 # debugging easier in case sth. goes wrong with the datahub
246@@ -122,11 +115,34 @@
247 # tell the user which datahub we are running
248 logging.debug("Running datahub (%s) with PID=%i" %(which(DATAHUB), p.pid))
249
250-def handle_sighup(signum, frame):
251- """We are using the SIGHUP signal to shutdown zeitgeist in a clean way"""
252- logging.info("got SIGHUP signal, shutting down zeitgeist interface")
253- interface.Quit()
254-signal.signal(signal.SIGHUP, handle_sighup)
255-
256-logging.info("Starting Zeitgeist service...")
257-mainloop.run()
258+def setup_handle_sighup(interface):
259+ def handle_sighup(signum, frame):
260+ """We are using the SIGHUP signal to shutdown zeitgeist in a clean way"""
261+ logging.info("got SIGHUP signal, shutting down zeitgeist interface")
262+ interface.Quit()
263+ return handle_sighup
264+
265+if __name__ == "__main__":
266+
267+ parser = parse_commandline()
268+
269+ _config.options, _config.arguments = parser.parse_args()
270+ if _config.options.shell_completion:
271+ sys.exit(do_shell_completion(parser))
272+
273+ logging.basicConfig(level=getattr(logging, _config.options.log_level))
274+
275+ try:
276+ mainloop, interface = setup_interface()
277+ except RuntimeError, e:
278+ logging.exception("Failed to setup the RemoteInterface")
279+ sys.exit(1)
280+
281+ if _config.options.start_datahub:
282+ logging.info("Trying to start the datahub")
283+ start_datahub()
284+
285+ signal.signal(signal.SIGHUP, setup_handle_sighup(interface))
286+
287+ logging.info("Starting Zeitgeist service...")
288+ mainloop.run()
289
290=== modified file 'zeitgeist/client.py'
291--- zeitgeist/client.py 2010-08-26 18:38:47 +0000
292+++ zeitgeist/client.py 2010-10-20 10:26:03 +0000
293@@ -37,7 +37,6 @@
294
295 SIG_EVENT = "asaasay"
296
297-logging.basicConfig(level=logging.DEBUG)
298 log = logging.getLogger("zeitgeist.client")
299
300 class _DBusInterface(object):

Subscribers

People subscribed via source and target branches