Blacklist API sucks

I suggest you post a full API draft before implementing this. We have the current (admittedly not-so-good) API because it was hard coming up with a nice API for this.

Thanks for raising this Michal, I agree completely with you on this.

Ok, as mentioned in the first post I'd suggest this:

[org.gnome.zeitgeist.Blacklist] inteface:
Methods:
 - GetTemplates() -> (Array of Events)
 - AddTemplate(Event)
 - RemoveTemplate(Event)
Signals:
 - Changed()

A few notes: templates must be unique, adding the same template twice will result in the second call being ignored (and will not emit Changed signal). Otherwise after every change Changed() signal is emitted and since it doesn't have any parameters the client is expected to call GetTemplates afterwards if it cares (maybe this should be changed?).

So if we have 10 clients monitoring the blacklist the daemon it will result in 10 GetTemplates() calls on the bus? ;-) How about putting the new list of templates in the Changed() signal? That way there'll be no extra noise on the buzz when the list changes.

Anyway, I am still not entirely convinced about this API. I think clients need a way to identify which blacklists come from where. I think maybe a naming scheme would provide a better API. Like fx:

Let 'E' define the event dbus signature.

Methods:
 - GetTemplates() -> ({sE})
 - AddTemplate(s, E)
 - RemoveTemplate(s)
Signals:
 - Changed({sE})

Apps would use their normal namespaced bus names to construct blacklist names. Like 'org.gnome.Epiphany.PrivateBrowsing' for the blacklist template epiphany install when you enter private mode.

2010/8/5 Mikkel Kamstrup Erlandsen <email address hidden>:
> Apps would use their normal namespaced bus names to construct blacklist
> names. Like 'org.gnome.Epiphany.PrivateBrowsing' for the blacklist
> template epiphany install when you enter private mode.

Why would it use blacklisting for that? The extension should just stop
sending events.

On 5 August 2010 23:04, Siegfried Gevatter <email address hidden> wrote:
> 2010/8/5 Mikkel Kamstrup Erlandsen <email address hidden>:
>> Apps would use their normal namespaced bus names to construct blacklist
>> names. Like 'org.gnome.Epiphany.PrivateBrowsing' for the blacklist
>> template epiphany install when you enter private mode.
>
> Why would it use blacklisting for that? The extension should just stop
> sending events.

Because it's mode complex that way? ;-)

The other reason would be that you could have third party apps easily
control the privacy mode.

(Note for people looking for ways to clear the blacklist with the currently released API: Just SetBlacklist([]))

Mikkel

Will this clear all the blacklists? I just want to remove one specific blacklist

2010/9/10 Manish Sinha <email address hidden>:
> Will this clear all the blacklists? I just want to remove one specific
> blacklist

Ask for the list of blacklisting rules, remove the one you don't want
from it, and set it again (with that rule removed).

@Manish: Right, that will clear all blacklists I am afraid. Which is also partially why we have this bug open to come up with something which is easier for apps to handle.

Go Manish! Go Manish! Its your birthday :P
(please ignore this comment)

If we had huge number of blacklists. This can be possible since people don't want fine grained privacy. We can set the default modes and give them option.
E.g. For chrome, we will log all the events in normal browsing and no logging in incognito mode. Additionally people sometimes watch pr0n in normal mode too and would like to block the logging based on a list of websites (app adult based).

So I recommend,one more methods
Search(E)

Additionally, does s stand for the application's D-Bus signature? I got a bit confused

Meh, type in last comment
scrap "This can be possible since people don't want fine grained privacy"

I meant
This can be possible since many people do want fine grained privacy

thekorn> it sucks, maybe. but do we need a new API for this, that's what I'm not sure about
<seiflotfy> thekorn, u can see mhr3 and kamstrup got into the details of it
<seiflotfy> they wrote the whoel api actually
<m4n1sh_> seiflotfy: look into #ubuntu-classroom
* mvo has quit (Quit: Ex-Chat)
<RainCT> thekorn: what we should really add is AddBlacklistRule and DeleteBlacklistRule
<thekorn> RainCT: yeah,
<thekorn> after reading this bug, I tend to agree that we need some changes
<thekorn> mikkel's proposal seems close to perfect to me
<thekorn> so, whoever what's to work on it, should start with some paperwork,
<thekorn> we are not ready to do the implementation yet
<seiflotfy> thekorn, AFAIK m4n1sh is on it
<seiflotfy> thekorn, also m4n1sh will do the implementation of it and propose it for merge
<seiflotfy> he is very good with python
<seiflotfy> maybe better than you :P
<m4n1sh_> seiflotfy: dont set the expectations too high
<thekorn> I'm sure he is.
<seiflotfy> m4n1sh, its marketing :P
<m4n1sh_> yeah. I was going through the code. The progress is slow as I am also on zg-sharp packaging and fixing the issues with meebey
<thekorn> but I would like to design it properly first,
<thekorn> to avoid extra roubndtrips of work
<m4n1sh_> thekorn: sure!
<m4n1sh_> seiflotfy: fuck man. OMG question on #ubuntu-classroom
<seiflotfy> thekorn, lets use the first draft from mikkel
<m4n1sh_> seiflotfy: over to classroom
<thekorn> seiflotfy: why?
<thekorn> does it cover all possible usecases of the blacklist feature
* RainCT doesn't like kamstrup's proposal of having names for them
<seiflotfy> RainCT, why not
<thekorn> haha, that's actually the only thing I *like* about this proposal ;)
<seiflotfy> thekorn, +1
<RainCT> because it's not necessary and may cause conflicts between apps who have the same rule with different names (so one says it's enabled and another one it isn't)
<m4n1sh_> thekorn: RainCT seiflotfy I like kamstrup idea
<seiflotfy> thekorn, whats up we have been getting along much better lately
<seiflotfy> we tend to agree on stuff
<seiflotfy> that is new to me
<RainCT> what could be nice is a free form string for users to write their own name/description for the rule and maybe a boolean to enable/disable it (although i'm not sure if this later thing should really be in ZG or be a GUI thing)
<thekorn> RainCT: if we set a naming convention, which is clearly defined somewhere, than it should be perfectly ok
<RainCT> no, there is no naming convention for blacklists
<thekorn> say who?
<thekorn> says
<RainCT> Me.
<thekorn> ah ok.
<RainCT> The Blacklist Management GUI would just resort to using unique IDs for user-defined stuff, which would be completely meaningless.
<thekorn> in fact, in mikkels proposal the names are free form strings
* meebey leaves work
<RainCT> And the private browsing example isn't something which should be handled by blacklist but by the epiphany plugin itself
<RainCT> and if you want to disable it centralized you can do it over the DataSourceRegistry and then the epiphany plugin is told it's been disabled
<thekorn> ok, as you can all see, there needs to be alot of discussion
<RainCT> ^^
<thekorn> so I don't expect we can do a ch...

with only a week left and no decision on the development method and API I postponed the bug to 0.7

At some point people will need to blacklist over Unity. I think that should be taken in consideration in the new design and development cycle.
Example: What if I don't want my .py files to show up in Unity

> Let 'E' define the event dbus signature.
>
> Methods:
> - GetTemplates() -> ({sE})
> - AddTemplate(s, E)
> - RemoveTemplate(s)
> Signals:
> - Changed({sE})
> Apps would use their normal namespaced bus names to construct blacklist names.
> Like 'org.gnome.Epiphany.PrivateBrowsing' for the blacklist template epiphany install when you enter private mode.

I was looking at this bug today and wanted to clarify some points.

1) s = 'org.gnome.Epiphany.PrivateBrowsing' ??

2) So ({sE}) means each blacklist template associated with the application which set it

3) RemoveTemplate(s) would clear all the blacklists set by an application. Should not it be RemoveTemplate(s, E) or RemoveTemplate(s, {E}) meaning clearing more than one event template at a time

4) Similarly, how about AddTemplate(s, {E}) meaning adding more than one template at a time

Have some more doubts on Changed() but will ask them after these 4 doubts are cleared.

So the final consensus is that the parameter s is any randomly chosen identifier for an application.

* GetBlacklists() -> ({sE})
* AddBlacklist(sE) --> Changed(sE)
* RemoveBlacklist(sE) --> Changed(sE)

I changed the name to Blacklist instead of Template for the method names as it sounds better and is more clear

After re-reading this bugreport, the discussion it includes and the merge proposal, I'm still not confident that the API which is proposed here is good enough. This is why I started working on a blacklist API spec.
It is still work in progress, and I'm not sure if this points in the right direction (comments are welcome) but I at least have a good feeling.

current version is here: http://paste.ubuntu.com/551001/

On 6 January 2011 10:43, Markus Korn <email address hidden> wrote:
> After re-reading this bugreport, the discussion it includes and the merge proposal, I'm still not confident that the API which is proposed here is good enough. This is why I started working on a blacklist API spec.
> It is still work in progress, and I'm not sure if this points in the right direction (comments are welcome) but I at least have a good feeling.
>
> current version is here: http://paste.ubuntu.com/551001/

I think there is some good stuff in there Markus, however I'm not
entirely sold on the regex idea for lookups. I'd rather that we just
support the same globbing semantics as we do for event templates; ie
prefix-! for negation and postfix-* for truncation. I guess * will be
the most common case. Not allowing general regexes also motivates app
authors to choose more compatible blacklist ids. Ie, we'll be unlikely
to see:

 org.gnome.bookmarksepiphany
 org.gnome.downloadsepiphany

but we encourage something like:

org.gnome.epiphany.bookmarks
org.gnome.epiphany.downloads

One thing I am missing here and maybe its too new for the proposal is
temporary blacklists. Such as:
Don't log for the next 20 minutes. Don't log Documents from 19.7 - 19.8
etc... I don't see this option at all in our current API ideas.

While I actually like Markus's proposal alot (kudos on the awesome work) I
am kind of hesitant on the BlacklistTemplateIdentifier. I feel more
comfortable using an identifier like "actor/#uid" as in
"application://firefox.desktop/porn". This allows us to easily trace where
the blacklist came from.

These are just random thoughts.
Cheers
Seif

On Thu, Jan 6, 2011 at 5:29 PM, Seif Lotfy <email address hidden>wrote:

> While I actually like Markus's proposal alot (kudos on the awesome work) I
> am kind of hesitant on the BlacklistTemplateIdentifier. I feel more
> comfortable using an identifier like "actor/#uid" as in
> "application://firefox.desktop/porn". This allows us to easily trace where
> the blacklist came from.
>

I guess BlacklistTemplateIdentifier identifies a blacklist template instead
of where it came from. I don't agree on this, ANy blacklist template is a
event at the end of the day, so will have ID set which can be used as
identifier.

Tracing which applications set the application makes sense. We don't need
"application://firefox.desktop/porn" Why? Identifiers should be atomic if
possible.

My first thought was also "Why regexes, glob patterns would do the trick", so I'm with Mikkel on this one. Overall I think it's what we agreed on with some polishing (the enum in the signal, great formalization of everything...), therefore I'm fine with it.

Please let me explain how I see the whole 'identifier' story:
I don't think we need a identifier which tells us which client set an certain blacklist, simply because the client which sets a blacklist might not be related to the actual purpose of this blacklist, let me give you an example:

A calendar daemon is setting a blacklist for media files played by MyMusicPlayer every workday's morning. So in the app-naming proposal the name would look like "calendar-daemon" (and maybe some addiotions of any kind). IMHO it makes more sense to use the purpose as a first class object for description, so something like "MyMusicPlayer.DuringWorkHours".

Also, blacklists are not owned by a certain client (e.g. there is no client which has exclusive add/removal rights for a special kind of blacklist). So names which bind a blacklist to the "blacklister"-client might lead people on the wrong track.

@Seif, this is an interresting usecase, which was never raised so far. But honstly I'm not a big fan of this idea, basically because it is hard to do it right. One thing which will always fail is: let's say you have a blacklist which is *somehow* active for the next 5 minutes. Can this blacklist be removed once it gets deactivated? What if a client adds an event with a timestamp in this period later on (e.g. from a history)? - So potentially we will always get users complaining: "hey I didn't want zg to log events during XYZ, but there are still events in my log from during this timeperiod." And we have to tell this user: "well, this event was inserted afterwards" - So for the sake simplisity we should move thi kind of blacklisting to the client side.

My first idea related to RegEx based matches was that I thought we need something very powerful there. But maybe I'm wrong, So I'm happy to change my proposal there and only allow prefix-search and negation. And if we need something more powerful later on we can always change it.

I still don't see why blacklist templates would need a
manually-constructed identifier...

@Siegfried, how do you want to identify a template in the collection of all templates, for example, what do you propose as an argument for the RemoveTemplate() method?

Do you want automatically generated identifier (e.g. auto-integer-id, maybe in combination with sender string)? or do you have a system in mind which does not use identifiers at all?

2011/1/6 Markus Korn <email address hidden>:
> @Siegfried, how do you want to identify a template in the collection of
> all templates, for example, what do you propose as an argument for the
> RemoveTemplate() method?

Sending the template (like we're doing now) would be an option. If
you're not happy with that, we could do something like Zeitgeist
itself uses: automatically assigned unique identifiers (be it a
number, or whatever).

The problem I see with clients supplying a name is that it just seems
too random to me. How are they supposed to choose a name? If I was to
write some application using the proposed API, I'd probably end up
just using "<my-application's-name>-<current timestamp>" or something
like that and hope it isn't already in use.