delete_event_hook for extensions

Bug #595919 reported by Mikkel Kamstrup Erlandsen
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zeitgeist Framework
Fix Released
Medium
Seif Lotfy

Bug Description

Right now zeitgeist extensions have no way of knowing when events are deleted. I propose adding a delete_event_hook() method on the Extension class that extensions can override. The delete_event_hook() method should take an event_id as its sole argument.

I need this functionality in the FTS extension.

Related branches

Revision history for this message
Siegfried Gevatter (rainct) wrote : Re: [Bug 595919] [NEW] delete_event_hook for extensions

2010/6/18 Mikkel Kamstrup Erlandsen <email address hidden>:
> Right now zeitgeist extensions have no way of knowing when events are
> deleted. I propose adding a delete_event_hook() method on the Extension
> class that extensions can override.
+1, this should definitelly be available.

> The delete_event_hook() method
> should take an event_id as its sole argument.
Wouldn't a list of events be more useful?

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

I agree, we can add this delete hook to extensions, but I'm not sure what this hook should be able to do.
Is it to just notify an extension that a certain event is deleted, or is about to be deleted? Or can this hook also block the removal of an event?
For notification purposes we have the notification interface.

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

I proposed one event_id because the insert hook also only receives one event.

The idea was simply as a notification mechanism, I am not sure it's a good idea to allow extensions to bar events from deletion, although I can't fully see the ramifications of this... We can't use the current notification API because that would require a full dbus roundtrip (back to the same process!).

I don't know if the hooks should be called before or after the deletion... Any thoughts?

Revision history for this message
Siegfried Gevatter (rainct) wrote : Re: [Bug 595919] Re: delete_event_hook for extensions

I'd say: Call the hook before the deletion with the full event list
(or event ids only, not sure *) and have them return true or false to
indicate whether the event should be deleted or the operation aborted.

This may serve to create an evil extension which won't let you delete
anything? Well, yes, but why the hell would you install that
extension? And if you do, it's your fault :P.

(* Full events may be needed by some extension, but require us to do a
query to fetch the event if there is at least on hook. To avoid that
we could just send the event ids, but then there may be two extensions
fetching the info by themselves so doing two queries. Not sure what is
worse. I think I tend to 'only IDs' to KISS).

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

On Fri, Jun 18, 2010 at 9:47 PM, Siegfried Gevatter <email address hidden>wrote:

> I'd say: Call the hook before the deletion with the full event list
> (or event ids only, not sure *) and have them return true or false to
> indicate whether the event should be deleted or the operation aborted.
>
> This may serve to create an evil extension which won't let you delete
> anything? Well, yes, but why the hell would you install that
> extension? And if you do, it's your fault :P.
>
> (* Full events may be needed by some extension, but require us to do a
> query to fetch the event if there is at least on hook. To avoid that
> we could just send the event ids, but then there may be two extensions
> fetching the info by themselves so doing two queries. Not sure what is
> worse. I think I tend to 'only IDs' to KISS).
>
> --
> delete_event_hook for extensions
> https://bugs.launchpad.net/bugs/595919
> You received this bug notification because you are subscribed to The
> Zeitgeist Project.
>
> Status in Zeitgeist Framework: New
>
> Bug description:
> Right now zeitgeist extensions have no way of knowing when events are
> deleted. I propose adding a delete_event_hook() method on the Extension
> class that extensions can override. The delete_event_hook() method should
> take an event_id as its sole argument.
>
> I need this functionality in the FTS extension.
>
>
>
I don't know if I agree or disagree with Siggi on this one. I think the hook
should be after the event has been deleted. An extension should
not interfere with the process of deletion. But I do see a purpose of
hooking in before the event is deleted.

It is more interesting to know if the event has been successfully deleted
and THEN hooking in with the extension.

Maybe a possibility is to add to hooks delete_event_hook and
delete_event_hook_after

Revision history for this message
Siegfried Gevatter (rainct) wrote :

2010/6/18 Seif Lotfy <email address hidden>:
> Maybe a possibility is to add to hooks delete_event_hook and
> delete_event_hook_after
+1, so that if another extension decides a particular thing shouldn't
be deleted FTS's hook won't be called.

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

As there is rouch consensus that we want something like this, I am tagetting 0.4.1 as it shouldn't be a big deal to implement

Changed in zeitgeist:
importance: Undecided → Medium
milestone: none → 0.4.1
status: New → Triaged
Revision history for this message
Seif Lotfy (seif) wrote :

On second thought i think a delete hook must be after.. since its the only way of knowing something was successfully deleted

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

BTW what should the delete_event_hook return?
the whole event or the id

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

Does the extension need to know the sender of the delete ?

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

Ok here is my frist draft of a delete_event_hook that takes in the ids of deleted events and it hooks in AFTER the events are deleted. Take a look the test case is in there too

Changed in zeitgeist:
status: Triaged → Fix Committed
Seif Lotfy (seif)
Changed in zeitgeist:
assignee: nobody → Seif Lotfy (seif)
status: Fix Committed → Fix Released
Changed in zeitgeist:
milestone: 0.4.1 → 0.5.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.