Zeitgeist Framework

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

  1. bug580364
  2. Merge into 0.8-python
Proposed by Seif Lotfy
Status: Merged
Merge reported by: Seif Lotfy
Merged at revision: not available
Proposed branch: lp:~zeitgeist/zeitgeist/bug580364
Merge into: lp:zeitgeist/0.1
Diff against target: 73 lines (+25/-20)
2 files modified
_zeitgeist/engine/main.py (+4/-6)
test/engine-test.py (+21/-14)
To merge this branch: bzr merge lp:~zeitgeist/zeitgeist/bug580364
Reviewer Review Type Date Requested Status
Markus Korn Needs Fixing
Mikkel Kamstrup Erlandsen Needs Fixing
Review via email: mp+34141@code.launchpad.net

Description of the change

I fixed bug 580364
now we support searching for storage templates

To post a comment you must log in.
Revision history for this message
Markus Korn (thekorn) wrote :

Sorry, big NACK from me.
It makes no sense to allow this kind of queries right now, as we have no ways to set the storage state implemented. This is why we decided to throw an exception.

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

Well, I disagree with Markus :-) I think it makes perfect sense to include this - otherwise we have a chicken-and-egg problem.

In theory anyone could set the storage field upon item insertion. It just so happens that the DS we ship is not feature complete. On top of that, 3rd parties could provide their own storage monitor extension which update the storage table correctly (at the very least a networkmanager/connman extension is easy).

Regarding the code:

 1) It looks like testFindStorageNotExistant() is there twice?

 2) Docstrings in the unit tests are not right

 3) Apart from these small things it looks good to me

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

Ok, I cave in, fix the things mentioned by Mikkel, run the testsuite and if everything goes well merge it into lp:zeitgeist (but please don't *bzr pull*)

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

ok done :)

lp:~zeitgeist/zeitgeist/bug580364 updated
1562. By Seif Lotfy

fixed docstrings and removed duplicate test

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '_zeitgeist/engine/main.py'
--- _zeitgeist/engine/main.py 2010-08-02 10:13:12 +0000
+++ _zeitgeist/engine/main.py 2010-08-31 20:50:54 +0000
@@ -220,12 +220,6 @@
220 subject_templates = [Subject(data) for data in template[1]]220 subject_templates = [Subject(data) for data in template[1]]
221 else:221 else:
222 subject_templates = None222 subject_templates = None
223 # first of all we need to check if the query is supported at all
224 # we do not support searching by storage field for now
225 # see LP: #580364
226 if subject_templates is not None:
227 if any(data[Subject.Storage] for data in subject_templates):
228 raise ValueError("zeitgeist does not support searching by 'storage' field")
229 223
230 subwhere = WhereClause(WhereClause.AND)224 subwhere = WhereClause(WhereClause.AND)
231 225
@@ -292,6 +286,10 @@
292 if value:286 if value:
293 value, negation, wildcard = parse_operators(Subject, getattr(Subject, key.title()), value)287 value, negation, wildcard = parse_operators(Subject, getattr(Subject, key.title()), value)
294 subwhere.add_text_condition("subj_%s" %key, value, wildcard, negation)288 subwhere.add_text_condition("subj_%s" %key, value, wildcard, negation)
289
290 if subject_template.storage:
291 subwhere.add_text_condition("subj_storage", subject_template.storage)
292
295 except KeyError, e:293 except KeyError, e:
296 # Value not in DB294 # Value not in DB
297 log.debug("Unknown entity in query: %s" % e)295 log.debug("Unknown entity in query: %s" % e)
298296
=== modified file 'test/engine-test.py'
--- test/engine-test.py 2010-07-28 20:28:48 +0000
+++ test/engine-test.py 2010-08-31 20:50:54 +0000
@@ -866,20 +866,27 @@
866 )866 )
867 self.assertEquals(3, len(ids))867 self.assertEquals(3, len(ids))
868 868
869 def testBug580364(self):869 def testFindStorageNotExistant(self):
870 """ for now we raise a ValueError if someone wants to search870 events = [
871 by the storage field, this might change later on. (LP: #580364)"""871 Event.new_for_values(timestamp=1000, subject_storage="sometext"),
872 events = [872 Event.new_for_values(timestamp=2000, subject_storage="anotherplace")
873 Event.new_for_values(timestamp=1000, subject_storage="sometext"),873 ]
874 Event.new_for_values(timestamp=2000, subject_storage="anotherplace")874 ids_in = self.engine.insert_events(events)
875 ]875 template = Event.new_for_values(subject_storage="xxx")
876 ids_in = self.engine.insert_events(events)876 results = self.engine.find_eventids(TimeRange.always(), [template],
877 template = Event.new_for_values(subject_storage="xxxx")877 StorageState.Any, 10, ResultType.MostRecentEvents)
878 878 self.assertEquals(0, len(results))
879 self.assertRaises(ValueError, self.engine.find_eventids,879
880 TimeRange.always(), [template], StorageState.Any, 10,880 def testFindStorage(self):
881 ResultType.MostRecentEvents881 events = [
882 )882 Event.new_for_values(timestamp=1000, subject_storage="sometext"),
883 Event.new_for_values(timestamp=2000, subject_storage="anotherplace")
884 ]
885 ids_in = self.engine.insert_events(events)
886 template = Event.new_for_values(subject_storage="sometext")
887 results = self.engine.find_eventids(TimeRange.always(), [template],
888 StorageState.Any, 10, ResultType.MostRecentEvents)
889 self.assertEquals(1, len(results))
883 890
884 def testWildcard(self):891 def testWildcard(self):
885 import_events("test/data/five_events.js", self.engine)892 import_events("test/data/five_events.js", self.engine)