zeitgeist team mailing list archive

[Mikkel Kamstrup Erlandsen <mikkel.kamstrup@xxxxxxxxx>]

2009/10/11 Siegfried-Angel Gevatter Pujals <siegfried@xxxxxxxxxxxx>:
> Hi,
>
> With the changes to the new event-item-separation model and having
> annotations exposed as such in the API (assuming
> http://live.gnome.org/GnomeZeitgeist/EngineAPIRevamp#Annotations will
> be used), the old FindEvents prototype doesn't make sense anymore.
> Further, it has some stuff which doesn't make sense and I want to use
> this chance to fix that (eg., why aren't the timestamps part of
> filters?).
>
> For reference, here is what we have in the 0.2 series:
>
> ================== CURRENT IMPLEMENTATION ============================
> FindEvents(min_timestamp, max_timestamp, limit, sorting_asc, mode, filters)
> # filters is a list of dicts, where each dict can have the following items:
> #   name: <list> of <str>    -- this is new in 0.2.1, was just "<str>"
> previously
> #   uri: <list> of <str>     -- this is new in 0.2.1, was just "<str>"
> previously
> #   tags: <list> of <str>
> #   mimetypes: <list> of <str>
> #   source: <list> of <str>
> #   content: <list> of <str>
> #   application: <list> of <str>
> #   bookmarked: <bool> (True means bookmarked items, and vice versa)
> ========================================================================
>
> I have though of a series of  changes to this. First of all, is moving
> min_timestamp and max_timestamp into the filters argument (which makes
> them optional arguments, by the way) and splitting filters into
> event_filters and item_filters to avoid confusion. The prototype would
> look like this:
>
>> FindEvents(limit, sorting_asc, event_filters, item_filters)
>
> The filter sets would now look like this:
>
> event_filters <list of dicts>:
>  min_timestamp <int>
>  max_timestamp <int>
>  source <list of str>
>  content <list of str>
>  application <list of str>
>  tags and bookmarked
> item_filters <list of dicts>:
>  name <list of str>
>  uri <list of str>
>  mimetypes <list of str>
>  source <list of str>
>  content <list of str>
>  tags and bookmarked

What do you mean with this last line "tags and bookmarked
"? It seems not to fit in with what you write below.

> For tags and bookmarked filters, I propose to replace them with
> "annotation" which would take dicts with one or more of "source",
> "content" and "text" (the remaining Annotation attributes, "uri" and
> "subject", are irrelevant here; this could be exposed in the Python
> module as AnnotationFilter). A user-defined tag called "foo" would be
> AnnotationFilter(content = Content.TAG, source = Source.USER_ACTIVITY,
> text = "foo"), while a bookmark would be AnnotationFilter(content =
> Content.BOOKMARK, source = Source.USER_ACTIVITY) and a filter for all
> tags of any sort (automatically defined, user-defined, whatever)
> starting with "a" would be AnnotationFilter(content = Content.TAG,
> text="a%").

Using a % for truncation seems very SQL-esque to me. Since we don't
really want to expose that we are SQL-based then maybe using * for
truncation (like the rest of the Unix world) would be more natural? No
biggie - just a notpick :-)

Otherwise - nice so far.

> The "annotation" entry in event_filters and item_filter would take a
> list of lists of such AnnotationFilters, to make OR'ing and AND'ing
> possible, eg: [[ann1, ann2], [ann3, ann4]] asks for stuff with, either
> "both ann1 AND ann2" OR "both ann3 AND ann4".
>
> Are you OK with this so far?

I think this sounds great - and also more cleanly cut than the olde
way of doing it.

> Now, another question I have is whether, taking advantage of the fact
> that we are using Python and doing funny stuff with objects is easy,
> we should make it possible to call FindEvents in a simpler way if not
> all functionality is needed. What I'm talking about is, eg., allowing
> {name: "foo"} instead of {name: ["foo"]} if you only have one value
> for "name" (this is, by the way, currently possible in 0.2.1 because
> in 0.2 "name" took just a string; 0.2.1 changed it to a list of
> strings, but still accepts a single string to remain
> backwards-compatible). In the same style, you could do "ann1" instead
> of "[[ann1]]", and "[ann1, ann2]" instead of "[[ann1, ann2]]".

I don't think we should allow for such things in the API. For two reasons:

 * We might not always be written in Python. I for one want to write a
C-based engine when everything is stable and we have specs cleanly in
shape.
 * Ambiguous APIs tend to lead to an unmaintainable code base. The
Unix philosophy of "fail early and fail loudly" is more my cup of tea.
If the user sends bad arguments throw a DBus error in their face (with
a helpful error msg of course).

> This would not only make simple calls to FindEvents more readable, but
> make the live easier to lower level languages app devs which may want
> to do simple calls but using the complex API.

+1, except for the Pythonish features :-) Great work!

-- 
Cheers,
Mikkel