What to do on failed or blocked event insertions?

Bug #495179 reported by Mikkel Kamstrup Erlandsen on 2009-12-10
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zeitgeist Framework
Fix Released
Critical
Markus Korn

Bug Description

This bug is filed in relation to bug #495017: "AttributeError: 'NoneType' object has no attribute 'payload'"

Markus asks what to do with failed or blocked events in InsertEvents(). This is an important questions since we can't not raise an error or simply leave the event id out of the response. Clients depend on mapping the returned event_ids->events.

I propose to say that event id 0, indicates an error. I think this is a nice solution: easy to implement without breaking API, and easy to handle for clients because a boolean test "if not event_id : print 'eeeeeek!'" works.

Changed in zeitgeist:
milestone: none → 0.3.1
importance: Undecided → Critical
status: New → Confirmed
Markus Korn (thekorn) wrote :

I like this solution more than my proposal in my comment on the other bugreport. But I still think it is not optimal because there is no reason to distinguish between different reason of failure. The client will always get the same result back no matter if this event was blocked by an extension, blocked because the event already had an id, blocked because the client tried to add an event without a subject, and so on.
Is it possible to send negative integers over dbus, so we can define
  v >= 0 -> id of an event
  v < 0 -> error

We also predefine some error values as constants.

Markus Korn (thekorn) wrote :

I'm preparing a branch with the solution proposed by Mikkel, as this solution has the best (making sense)/(breaking API) ratio.

Changed in zeitgeist:
assignee: nobody → Markus Korn (thekorn)

We could use negative integers as error indicators, but that would require that we break DBus API since we use uint32 (signatire u) as event _ids now. The alternative is to use 64 bit ints (signature x). It's not a big break though - transpararent in Python actually...

Looking at your branch now.

If you rename _wrap_insert_event() to something like _insert_event_without_error() or something like that to indicate the purpose of the method I think the branch is fine to go in.

Markus Korn (thekorn) wrote :

thanks for your super fast code review.
I've changed the method name and fixed some engine-testcases (Exceptions are not raised anymore, 0 is returned instead)

If nobody complains until lunchtime I will merge this branch into lp:zeitgeist.
This branch fixes only half of the issue, we also need a way to handle GetEvents, which seems abit more complicated to me, but is a different bug, which I will file in a bit.

Markus Korn (thekorn) on 2009-12-11
Changed in zeitgeist:
status: Confirmed → Fix Committed
Changed in zeitgeist:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers