Merge lp:~zeitgeist/zeitgeist/bug695311 into lp:zeitgeist/0.1

[Siegfried Gevatter]

[Seif Lotfy (seif)]

Changing to dict is a big improvement for me and makes it easier to comprehend. All in all its pretty straight forward and i like the unload method. I would suggest writing to disk every 5 minutes or every time a new application opens, instead of "on zeitgeist exit" but this is just a suggestion.
However given its current state a +1 from me since my suggestions could be discussed later.

[Seif Lotfy (seif)]

forgot the +1

review: Approve

[lp:~zeitgeist/zeitgeist/bug695311]

1660. By Siegfried Gevatter on 2010-12-29

Actually call the extension's unload method ^^.

[Markus Korn (thekorn)]

Siegfried, you added an additional handler for SIGTERM. Can you please elaborate when we receive such kind of signals. I mean we clearly documented SIGHUB as a tool for distributors, and I would like to see the same documentation for SIGTERM too.

review: Needs Information

[Siegfried Gevatter (rainct)]

I haven't checked, but Zeitgeist should be receiving a SIGTERM at system poweroff (and it's also the default signal sent by "kill"). This way we try to ensure that Zeitgeist always ends nicely (ie. mostly always other than if there's an uncached exception or a SIGKILL).

[Mikkel Kamstrup Erlandsen (kamstrup)]

I have done any test runs, but this looks sane to me

review: Approve

[Mikkel Kamstrup Erlandsen (kamstrup)]

Sorry i mean I have *not* done test runs... :-)

=== modified file '_zeitgeist/engine/extension.py'
--- _zeitgeist/engine/extension.py	2010-12-26 09:18:11 +0000
+++ _zeitgeist/engine/extension.py	2010-12-29 16:56:27 +0000
@@ -62,6 +62,15 @@
 	def __init__(self, engine):
 		self.engine = weakref.proxy(engine)
 	
+	def unload(self):
+		"""
+		This method gets called before Zeitgeist stops.
+		
+		Execution of this method isn't guaranteed, and you shouldn't do
+		anything slow in there.
+		"""
+		pass
+	
 	def pre_insert_event(self, event, sender):
 		"""
 		Hook applied to all events before they are inserted into the
@@ -278,8 +287,9 @@
 				self.unload(ext)
 		else:
 			ext_name = get_extension_name(extension)
-			log.debug("Unloading extension '%s'" %ext_name)
+			log.debug("Unloading extension '%s'" % ext_name)
 			obj = self.__extensions[ext_name]
+			obj.unload()
 			for method in obj.PUBLIC_METHODS:
 				del self.methods[method]
 			del self.__extensions[ext_name]

=== modified file '_zeitgeist/engine/extensions/datasource_registry.py'
--- _zeitgeist/engine/extensions/datasource_registry.py	2010-10-19 13:54:12 +0000
+++ _zeitgeist/engine/extensions/datasource_registry.py	2010-12-29 16:56:27 +0000
@@ -72,17 +72,18 @@
 		dbus.service.Object.__init__(self, dbus.SessionBus(),
 			REGISTRY_DBUS_OBJECT_PATH)
 		
+		self._registry = {}
 		if os.path.exists(DATA_FILE):
 			try:
-				self._registry = [DataSource.from_list(
-					datasource) for datasource in pickle.load(open(DATA_FILE))]
+				with open(DATA_FILE) as data_file:
+					for datasource in pickle.load(data_file):
+						ds = DataSource.from_list(datasource)
+						self._registry[ds[DataSource.UniqueId]] = ds
 				log.debug("Loaded data-source data from %s" % DATA_FILE)
 			except Exception, e:
 				log.warn("Failed to load data file %s: %s" % (DATA_FILE, e))
-				self._registry = []
 		else:
 			log.debug("No existing data-source data found.")
-			self._registry = []
 		self._running = {}
 		
 		# Connect to client disconnection signals
@@ -93,49 +94,46 @@
 	    )
 	
 	def _write_to_disk(self):
-		data = [DataSource.get_plain(datasource) for datasource in self._registry]
+		data = [DataSource.get_plain(datasource) for datasource in
+			self._registry.itervalues()]
 		with open(DATA_FILE, "w") as data_file:
 			pickle.dump(data, data_file)
 		#log.debug("Data-source registry update written to disk.")
 	
-	def _get_data_source(self, unique_id):
-		for datasource in self._registry:
-			if datasource.unique_id == unique_id:
-				return datasource
-	
 	def pre_insert_event(self, event, sender):
 		for (unique_id, bus_names) in self._running.iteritems():
 			if sender in bus_names:
-				datasource = self._get_data_source(unique_id)
+				datasource = self._registry[unique_id]
 				# Update LastSeen time
 				datasource.last_seen = get_timestamp_for_now()
-				self._write_to_disk()
 				# Check whether the data-source is allowed to insert events
 				if not datasource.enabled:
 					return None
 		return event
 	
+	def unload(self):
+		self._write_to_disk()
+	
 	# PUBLIC
 	def register_data_source(self, unique_id, name, description, templates):
 		source = DataSource(str(unique_id), unicode(name), unicode(description),
 			map(Event.new_for_struct, templates))
-		for datasource in self._registry:
-			if datasource == source:
-				datasource.update_from_data_source(source)
-				self.DataSourceRegistered(datasource)
-				return datasource.enabled
-		self._registry.append(source)
+		if unique_id in self._registry:
+			datasource = self._registry[unique_id]
+			datasource.update_from_data_source(source)
+		else:
+			datasource = self._registry[unique_id] = source
 		self._write_to_disk()
-		self.DataSourceRegistered(source)
-		return True
+		self.DataSourceRegistered(datasource)
+		return datasource.enabled
 	
 	# PUBLIC
 	def get_data_sources(self):
-		return self._registry
+		return self._registry.values()
 	
 	# PUBLIC
 	def set_data_source_enabled(self, unique_id, enabled):
-		datasource = self._get_data_source(unique_id)
+		datasource = self._registry[unique_id]
 		if not datasource:
 			return False
 		if datasource.enabled != enabled:
@@ -244,11 +242,10 @@
 			return
 		uid = uid[0]
 
-		datasource = self._get_data_source(uid)
+		datasource = self._registry[uid]
 		
 		# Update LastSeen time
 		datasource.last_seen = get_timestamp_for_now()
-		self._write_to_disk()
 		
 		strid = "%s (%s)" % (uid, datasource.name)
 		log.debug("Client disconnected: %s" % strid)

=== modified file 'test/remote-test.py'
--- test/remote-test.py	2010-12-29 13:31:10 +0000
+++ test/remote-test.py	2010-12-29 16:56:27 +0000
@@ -455,6 +455,7 @@
 		# Verify that they have been inserted correctly
 		datasources = list(self.client._registry.GetDataSources())
 		self.assertEquals(len(datasources), 2)
+		datasources.sort(key=lambda x: x[DataSource.UniqueId])
 		self._assertDataSourceEquals(datasources[0], self._ds1)
 		self._assertDataSourceEquals(datasources[1], self._ds2)
 		
@@ -464,6 +465,7 @@
 		# Verify that it changed correctly
 		datasources = list(self.client._registry.GetDataSources())
 		self.assertEquals(len(datasources), 2)
+		datasources.sort(key=lambda x: x[DataSource.UniqueId])
 		self._assertDataSourceEquals(datasources[0], self._ds1)
 		self._assertDataSourceEquals(datasources[1], self._ds2b)
 	

=== modified file 'zeitgeist-daemon.py'
--- zeitgeist-daemon.py	2010-12-07 21:36:24 +0000
+++ zeitgeist-daemon.py	2010-12-29 16:56:27 +0000
@@ -118,12 +118,13 @@
 		# tell the user which datahub we are running
 		logging.debug("Running datahub (%s) with PID=%i" %(which(DATAHUB), p.pid))
 
-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")
+def setup_handle_exit(interface):
+	def handle_exit(signum=None, frame=None):
+		""" We want to end Zeitgeist in a clean way when we receive a
+		SIGTERM or SIGHUP signal, or the user pressed Ctrl+C. """
+		logging.info("Shutting down Zeitgeist interface...")
 		interface.Quit()
-	return handle_sighup
+	return handle_exit
 
 def setup_logger(log_level):
 	logger = logging.getLogger()
@@ -167,7 +168,13 @@
 		logging.info("Trying to start the datahub")
 		start_datahub()
 	
-	signal.signal(signal.SIGHUP, setup_handle_sighup(interface))
+	handle_exit = setup_handle_exit(interface)
+	
+	signal.signal(signal.SIGHUP, handle_exit)
+	signal.signal(signal.SIGTERM, handle_exit)
 	
 	logging.info("Starting Zeitgeist service...")
-	mainloop.run()
+	try:
+		mainloop.run()
+	except KeyboardInterrupt:
+		handle_exit()