notifier: simply notifier_strategy compat support
In order to handle the deprecated notifier_strategy configuration option
we just need to translate the notifier_strategy value as follows:
default -> transport_url=None, driver='noop'
noop -> transport_url=None, driver='noop'
logging -> transport_url=None, driver='log'
rabbit -> transport_url=rabbit:///, driver='messaging'
qpid -> transport_url=qpid:///, driver='messaging'
None -> transport_url=None, driver=None
where the last bit simply means that the user hasn't specified any
notifier_strategy and we should just have oslo.messaging load a notifier
based on oslo.messaging's config options.
It really doesn't need to be any more complicated than that, and we
certainly shouldn't need to poke at what the user may have configured
for transport_url in their config file.
The reason this is important is that CONF.transport_url is a private
configuration option which soon will not be visible to users of the
oslo.messaging API. See I9c3bf74f13a820ce295aa77a2bbcd781778b5926.
Also, simplify the testing of this stuff by mocking the oslo.messaging
APIs used and using the test scenarios outlined above. This also means
we stop poking around in oslo.messaging internals like:
self.assertEqual(str(nfier._transport._driver._url),
'qpid:///')
Naughty, naughty!
Note: it's arguable we should just remove notifier_strategy in Juno
since we deprecated it in Icehouse, but this patch will be useful to
backport to stable/icehouse in any case.
Change-Id: I067248974baee50a2a0afb38bf022b65d1bea57a
This commit is contained in:
Mark McLoughlin
parent
00bc84b052
commit
4d6e55f40c
@ -26,7 +26,7 @@ import glance.openstack.common.log as logging
|
||||
from glance.openstack.common import timeutils
|
||||
|
||||
notifier_opts = [
|
||||
cfg.StrOpt('notifier_strategy', default='default',
|
||||
cfg.StrOpt('notifier_strategy',
|
||||
help=_('Notifications can be sent when images are create, '
|
||||
'updated or deleted. There are three methods of sending '
|
||||
'notifications, logging (via the log_file directive), '
|
||||
@ -61,57 +61,32 @@ _ALIASES = {
|
||||
class Notifier(object):
|
||||
"""Uses a notification strategy to send out messages about events."""
|
||||
|
||||
def __init__(self, strategy=None):
|
||||
def __init__(self):
|
||||
|
||||
_driver = None
|
||||
_strategy = strategy
|
||||
driver = None
|
||||
transport_url = None
|
||||
publisher_id = CONF.default_publisher_id
|
||||
|
||||
if CONF.notifier_strategy != 'default':
|
||||
if CONF.notifier_strategy:
|
||||
msg = _("notifier_strategy was deprecated in "
|
||||
"favor of `notification_driver`")
|
||||
LOG.warn(msg)
|
||||
|
||||
strategy = CONF.notifier_strategy
|
||||
|
||||
# NOTE(flaper87): Use this to keep backwards
|
||||
# compatibility. We'll try to get an oslo.messaging
|
||||
# driver from the specified strategy.
|
||||
_strategy = strategy or CONF.notifier_strategy
|
||||
_driver = _STRATEGY_ALIASES.get(_strategy)
|
||||
driver = _STRATEGY_ALIASES.get(strategy)
|
||||
if driver == 'messaging':
|
||||
transport_url = strategy + ':///'
|
||||
|
||||
publisher_id = CONF.default_publisher_id
|
||||
|
||||
try:
|
||||
# NOTE(flaper87): Assume the user has configured
|
||||
# the transport url.
|
||||
self._transport = messaging.get_transport(CONF,
|
||||
aliases=_ALIASES)
|
||||
except messaging.DriverLoadFailure:
|
||||
# NOTE(flaper87): Catch driver load failures and re-raise
|
||||
# them *just* if the `transport_url` option was set. This
|
||||
# step is intended to keep backwards compatibility and avoid
|
||||
# weird behaviors (like exceptions on missing dependencies)
|
||||
# when the old notifier options are used.
|
||||
if CONF.transport_url is not None:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.exception(_('Error loading the notifier'))
|
||||
|
||||
# NOTE(flaper87): This needs to be checked
|
||||
# here because the `get_transport` call
|
||||
# registers `transport_url` into ConfigOpts.
|
||||
if not CONF.transport_url:
|
||||
# NOTE(flaper87): The next 3 lines help
|
||||
# with the migration to oslo.messaging.
|
||||
# Without them, gate tests won't know
|
||||
# what driver should be loaded.
|
||||
# Once this patch lands, devstack will be
|
||||
# updated and then these lines will be removed.
|
||||
url = None
|
||||
if _strategy in ['rabbit', 'qpid']:
|
||||
url = _strategy + '://'
|
||||
self._transport = messaging.get_transport(CONF, url,
|
||||
aliases=_ALIASES)
|
||||
self._transport = messaging.get_transport(CONF,
|
||||
url=transport_url,
|
||||
aliases=_ALIASES)
|
||||
|
||||
self._notifier = messaging.Notifier(self._transport,
|
||||
driver=_driver,
|
||||
driver=driver,
|
||||
publisher_id=publisher_id)
|
||||
|
||||
def warn(self, event_type, payload):
|
||||
|
||||
@ -17,6 +17,8 @@
|
||||
import datetime
|
||||
|
||||
import mock
|
||||
from oslo.config import cfg
|
||||
from oslo import messaging
|
||||
import webob
|
||||
|
||||
from glance.common import exception
|
||||
@ -94,42 +96,50 @@ class TaskRepoStub(object):
|
||||
|
||||
class TestNotifier(utils.BaseTestCase):
|
||||
|
||||
def test_load_rabbit(self):
|
||||
nfier = notifier.Notifier('rabbit')
|
||||
self.assertIsNotNone(nfier._transport)
|
||||
|
||||
def test_load_qpid(self):
|
||||
nfier = notifier.Notifier('qpid')
|
||||
self.assertIsNotNone(nfier._transport)
|
||||
self.assertEqual(str(nfier._transport._driver._url),
|
||||
'qpid:///')
|
||||
|
||||
def test_notifier_strategy(self):
|
||||
self.config(notifier_strategy='qpid')
|
||||
@mock.patch.object(messaging, 'Notifier')
|
||||
@mock.patch.object(messaging, 'get_transport')
|
||||
def _test_load_strategy(self,
|
||||
mock_get_transport, mock_notifier,
|
||||
strategy, url, driver):
|
||||
if strategy is not None:
|
||||
self.config(notifier_strategy=strategy)
|
||||
nfier = notifier.Notifier()
|
||||
mock_get_transport.assert_called_with(cfg.CONF, url=url,
|
||||
aliases=notifier._ALIASES)
|
||||
self.assertIsNotNone(nfier._transport)
|
||||
self.assertEqual(str(nfier._transport._driver._url),
|
||||
'qpid:///')
|
||||
mock_notifier.assert_called_with(nfier._transport, driver=driver,
|
||||
publisher_id='image.localhost')
|
||||
self.assertIsNotNone(nfier._notifier)
|
||||
|
||||
def test_transport_url(self):
|
||||
transport_url = "qpid://superhost:5672/"
|
||||
self.config(transport_url=transport_url)
|
||||
notify = notifier.Notifier()
|
||||
self.assertEqual(str(notify._transport._driver._url),
|
||||
transport_url)
|
||||
def test_notifier_strategy_default(self):
|
||||
self._test_load_strategy(strategy='default',
|
||||
url=None,
|
||||
driver='noop')
|
||||
|
||||
def test_notification_driver_option(self):
|
||||
self.config(rpc_backend='qpid')
|
||||
self.config(notification_driver='messaging')
|
||||
self.config(notifier_strategy='rabbit')
|
||||
notify = notifier.Notifier()
|
||||
self.assertEqual(str(notify._transport._driver._url),
|
||||
'rabbit:///')
|
||||
def test_notifier_strategy_noop(self):
|
||||
self._test_load_strategy(strategy='noop',
|
||||
url=None,
|
||||
driver='noop')
|
||||
|
||||
self.config(notifier_strategy='default')
|
||||
notify = notifier.Notifier()
|
||||
self.assertEqual(str(notify._transport._driver._url),
|
||||
'qpid:///')
|
||||
def test_notifier_strategy_rabbit(self):
|
||||
self._test_load_strategy(strategy='rabbit',
|
||||
url='rabbit:///',
|
||||
driver='messaging')
|
||||
|
||||
def test_notifier_strategy_qpid(self):
|
||||
self._test_load_strategy(strategy='qpid',
|
||||
url='qpid:///',
|
||||
driver='messaging')
|
||||
|
||||
def test_notifier_strategy_logging(self):
|
||||
self._test_load_strategy(strategy='logging',
|
||||
url=None,
|
||||
driver='log')
|
||||
|
||||
def test_notifier_strategy_none(self):
|
||||
self._test_load_strategy(strategy=None,
|
||||
url=None,
|
||||
driver=None)
|
||||
|
||||
|
||||
class TestImageNotifications(utils.BaseTestCase):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user