[oe] [Bitbake-dev] bitbake 'events' - How to fix the current problems?

[Richard Purdie]

On Mon, 2010-01-18 at 15:17 -0700, Chris Larson wrote: 
> We already have better semantics, we just don't use it.  isinstance()
> can be used for this.
[...]
> isinstance can operate against an iterable of classes, or a base
class: 
> 
> if isinstance(e, (TaskStarted, TaskSucceeded)):
> 
> works great.

Great, makes perfect sense. We should start doing this.
 
>         3. Inclusion of the data dictionary within every event
>         
>         This messes up the bitbake client/server code no end since if
>         you want
>         to pickle or otherwise transport the event via IPC, it has the
>         whole
>         data dictionary in it. We need the event objects to be
>         marshallable
>         ideally. 
> 
> 
> Is there a reason we can't customize the serialization process for
> these objects to exclude the reference to the datastore?

We could do that but it turns into a pain. For example we have the
situation where we want to communicate an event (say bb.error) from a
worker thread to the server process which then needs to send this to the
clients (event handlers in the metadata, UIs, whatever).

For worker -> server its not desirable to transfer the datastore. Event
handlers in the metadata happen in the server's process space just like
parsing so passing the datastore to them isn't an issue. Passing to the
UIs is a different process space and passing the datastore is not
desirable.

If we ditch the datastore, the core code is going to be a lot simpler as
the objects will just work with the standard serialization.

We could just make this an internal change by changing the parameters of
the fire() call to pass the datastore and then pass this on as
event.data to the metadata handlers, manually inserting it into the
objects when wanted.

>         4. The pointless return values
>         
>         These should just die, they're useless. 
> 
> 
> Agree wholeheartedly.

Return values were ripped out of bitbake master a while ago. If we do
another 1.8 release it should have them removed too, then we can remove
the references from the code. Having said that, I'm now planning a 1.10
branch based off master ASAP with the XML bits bypassed with some loops.
This assumes I can get worker -> server events working though.

> I don't really see the gain of separating the name from the data
> rather than using an object here, but I don't have a very strong
> opinion either way.

The isinstance calls buy us a lot. If I can fix the marshalling over
XMLRPC, I can fix a lot of the UI code to stop using the text strings
and everything should be nicer. It also looks like we don't need a flag
day as I think I could keep this bitbake internal only.

> The only other thing that might be nice as a feature add would be the
> ability to express the events you're interested in via a variable
> flag, rather than conditionals, just to reduce the boilerplate code in
> the various event handlers, but that could also reduce flexibility, so
> that would be both arguable, and something that could be visited /
> discussed at a later time, rather than as part of this effort.

Agreed, this would be nice to have and would save us some processor
time, particularly when we start using inter-process calls.

Thanks for the discussion, its cleared my head a bit :)

Cheers,

Richard