Zeitgeist Framework

Merge lp://staging/~zeitgeist/zeitgeist/storagemonitor2 into lp://staging/zeitgeist/0.1

storagemonitor2
Merge into 0.8-python

Proposed by Mikkel Kamstrup Erlandsen on 2011-02-10

Status:

Merged

Merged at revision:

1699

Proposed branch:

lp://staging/~zeitgeist/zeitgeist/storagemonitor2

Merge into:

lp://staging/zeitgeist/0.1

Diff against target:

476 lines (+420/-5)

6 files modified

_zeitgeist/engine/__init__.py (+1/-1)
_zeitgeist/engine/extensions/Makefile.am (+2/-1)
_zeitgeist/engine/extensions/storagemonitor.py (+385/-0)
_zeitgeist/engine/sql.py (+3/-1)
_zeitgeist/engine/upgrades/core_3_4.py (+23/-2)
doc/zeitgeist/source/dbus_api.rst (+6/-0)

To merge this branch:

bzr merge lp://staging/~zeitgeist/zeitgeist/storagemonitor2

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Siegfried Gevatter		2011-02-10	Approve on 2011-03-14
Review via email: mp+49212@code.staging.launchpad.net

Description of the change

Woohoo, finally the storage monitor is ready. It supports uodating the storage table with the values from GIO volume monitors as well as the network state from Connman or NetworkManager which ever is available on the system.

The extension will also populate the 'storage' field of subjects that don't already have one.

So what's missing? Well ZG will raise NotImplementedError if you send it a StorageState that is different from StorageState.Any. So I'll propose another branch that properly supports this.

Revision history for this message

Siegfried Gevatter (rainct) wrote on 2011-02-10:

+logging.basicConfig(level=logging.DEBUG)
WHY?

And why is this and the imports there twice (before and after the big comments)?

+# storgaemonitor extension. This is actually backwards compatible.
Storgae? Is that like 'deine mudda' in Danish?

review: Needs Fixing

Revision history for this message

Siegfried Gevatter (rainct) wrote on 2011-02-10:

> "lambda : "
The space before the colons looks ugly :P.

> "except:"
Please make this "except sqlite3.foobar" (or, worst-case, "except Exception").
Also, why do you need the rollback there?

> "A storage medium is indetified by a key"
How can I indetify you? :)

You could move most the the NM/Connman code into a common base class.

Looks great otherwise. Good job!!!

review: Needs Fixing

Revision history for this message

Mikkel Kamstrup Erlandsen (kamstrup) wrote on 2011-02-11:

> > "lambda : "
> The space before the colons looks ugly :P.

Fixed

> > "except:"
> Please make this "except sqlite3.foobar" (or, worst-case, "except Exception").
> Also, why do you need the rollback there?

Since sqlite3 doesn't have a common error super class I am just catching Exception for now. The rollback() has been changed to 'return' instead.

> > "A storage medium is indetified by a key"
> How can I indetify you? :)

Fixed

> You could move most the the NM/Connman code into a common base class.

Yeah, I was thinking that, but there is so little code in them that I think a common base class would almost give us more net lines :-)

lp://staging/~zeitgeist/zeitgeist/storagemonitor2 updated on 2011-02-11

1676. By Mikkel Kamstrup Erlandsen on 2011-02-11: Fixes from review by Siegfried Gevatter
1677. By Mikkel Kamstrup Erlandsen on 2011-02-11: Typo in docstring
1678. By Mikkel Kamstrup Erlandsen on 2011-02-11: Remove line logging.basicConfig(level=logging.DEBUG) from storagemonitor.py

Revision history for this message

Siegfried Gevatter (rainct) wrote on 2011-02-11:

Haven't tried it yet, but the code looks good to me (although I still think a baseclass would be neat :P).

Revision history for this message

Siegfried Gevatter (rainct) wrote on 2011-03-04:

[2011-03-04 22:48:11,119] - ERROR - zeitgeist.extension - Failed loading the 'StorageMonitor' extension
Traceback (most recent call last):
  File "/home/rainct/Desenvolupament/Python/zeitgeist-project/storagemonitor2/zeitgeist/../_zeitgeist/engine/extension.py", line 265, in load
    obj = extension(self.__engine)
  File "/home/rainct/Desenvolupament/Python/zeitgeist-project/storagemonitor2/zeitgeist/../_zeitgeist/engine/extensions/storagemonitor.py", line 125, in __init__
    lambda: self.remove_storage_medium("net"))
  File "/home/rainct/Desenvolupament/Python/zeitgeist-project/storagemonitor2/zeitgeist/../_zeitgeist/engine/extensions/storagemonitor.py", line 326, in __init__
    proxy = dbus.SystemBus().get_object(NetworkMonitor.NM_BUS_NAME,
NameError: global name 'NetworkMonitor' is not defined

review: Needs Fixing

lp://staging/~zeitgeist/zeitgeist/storagemonitor2 updated on 2011-03-11

1679. By Mikkel Kamstrup Erlandsen on 2011-03-11: Fix ref to class variables NetworkMonitor.BLAH -> NMNetworkMonitor.BLAH

Revision history for this message

Siegfried Gevatter (rainct) wrote on 2011-03-14:

Alright, I've tested it with Network Manager and it seems to work fine. Awesome work!

Since I love nitpicking so much, "self._up ()" could be changed to "self._up()" :P. But anyway, I think this is ready to merge as soon as dbschema4 goes in (outstanding issues for that are deciding what to do with 0.7 compatibility, merging Seif's move tracking and deciding if we like Markus' cache deletion workaround - am I forgetting anything?).

review: Approve

lp://staging/~zeitgeist/zeitgeist/storagemonitor2 updated on 2011-04-04