Code review comment for lp:~cerberus/nova/nova_notifications

Revision history for this message
Rick Harris (rconradharris) wrote :

Looks great.

Not really a nit, just an observation:

> 215 +class RabbitNotifier(object):
> 216 + """Sends notifications to a specific RabbitMQ server and topic"""
> 217 +
> 218 + def notify(self, message):

It looks like the Notifier /class/ is superflous since we define a single stateless function inside of it and we don't derive the implementations from a base-class.

Should we just make each driver's `notify` proc a function which is called by `api.notify`?

As a side benefit, we get to avoid the cost of instantiating an object for each notification sent (of which there will be a gazillion), as well as the small penalty of later having to gc them.

review: Approve

« Back to merge proposal