221 Commits

Author SHA1 Message Date
Mark McLoughlin
f7cf85333c Don't include msg_id or reply_q in casts
On the server side, we only send replies if the request included a
_msg_id key. Also, the _reply_q key is only used when we wish to send a
reply.

So, in order to retain the exact same on-the-wire behaviour and ensure
servers aren't sending replies where none is needed, only include these
keys if we're doing a call (i.e. wait_for_reply=True).

Change-Id: Iac329493252be7d94b1ebe24f00e4d3f5c61d269
2013-08-26 10:23:06 +01:00
Mark McLoughlin
361092a488 Add a Notifier.prepare() method
Nova sends notifications with a bunch of different publisher_ids, so we
instantiate quite a lot of Notifier objects. Loading the noification
drivers for each of these is a substantial amount of overhead.

One obvious answer would be to make publisher_id an argument to the
error(), info(), etc. methods, but I think it's nice to encapsulate the
publisher_id in a notifier instance.

Instead, add a prepare() method which mirrors the approach in RPCClient.
You use this method to create a specialized notifier instance with a new
publisher_id.

Change-Id: Ia45fda3164086bb7a9ef6dee0587f726ab8b1a97
2013-08-21 07:30:49 +01:00
Davanum Srinivas
0555e939be Fix dictionary changed size during iteration
Make a copy of the keys before we operate on it

Fixes LP# : 1212854

Change-Id: I431ffb3878863e9be6d1a35078f7d7c3edf4b133
2013-08-17 22:04:05 -04:00
Mark McLoughlin
2564795108 Fix transport URL parsing bug
Handle e.g. foo://u:p@/bar. Right now we get:

 IndexError: string index out of range

from:

     if hostname[0] == '[':

Change-Id: I0bccebb14ad1d37862955e8988d160240bd1cf6d
2013-08-18 01:49:25 +01:00
Mark McLoughlin
c79bd1f24c Fix rabbit driver handling of None, etc. replies
Just like the bug in the fake driver, we have a similar rather
embarassing and obvious bug - we're currently not allowing endpoint
methods to send replies of None, '', False, [], {}, etc.

Change-Id: Icf0abdfcf122c5757dd3737f66130b3a53769ef6
2013-08-17 21:40:02 +01:00
Jenkins
f0bb1dac2e Merge "Bumps hacking to 0.7.0" 2013-08-17 17:10:26 +00:00
Jenkins
26722f92d9 Merge "Remove ConfFixture from toplevel public API" 2013-08-17 16:59:51 +00:00
Jenkins
42c324293d Merge "Fix fake driver handling of failure replies" 2013-08-17 16:58:33 +00:00
Mark McLoughlin
a2f99ad35e Remove ConfFixture from toplevel public API
There's no need to make the fixtures, testtools, etc. libraries a
runtime requirement, so let's move it from the toplevel oslo.messaging
API so you need to explicitly import it.

Change-Id: I9e2f32a898d78489f2d8d9c218c81f35cda14e34
2013-08-17 17:49:23 +01:00
Mark McLoughlin
001d66e6e5 Fix fake driver handling of failure replies
The driver reply() method is actually passed a full sys.exc_info()
tuple.

This was masked in the unit tests because the driver ended up basically
doing:

  raise (ValueError, ValueError, ...)

which caused a new ValueError to be instantiated and the test was
satisified. However, if an exception type has some required arguments,
you'll get a TypeError when this statement attempts to instantiate it
with no arguments.

Change-Id: I4af9c5084954d7b9c5f02cdae3387d17c206985b
2013-08-16 11:08:50 +01:00
Zhongyue Luo
f4e4f45433 Bumps hacking to 0.7.0
Change-Id: Ife77d8c6a2a479fce0a7879fb72c0b44e1287324
2013-08-16 14:36:42 +08:00
Mark McLoughlin
a3ffdd1b16 Fix transport URL ipv6 parsing support
Just copies code from Nova's cells code.

See I6b18f7643ab694f5ff34206b80865c40b1ec2680 for where this code was
first introduced.

Change-Id: If9b1503ed47f5910c0ab44edfbe3f42fcce9bf18
2013-08-16 06:31:03 +01:00
Mark McLoughlin
747e1a4099 Fix handling of None, etc. replies
A rather embarassing and obvious bug - we're currently not allowing
endpoint methods to send replies of None, '', False, [], {}, etc.

Change-Id: Ifc5ff8f308f526197559a4df7bed244cff6ed3c1
2013-08-15 21:11:38 +01:00
Jenkins
f95c5bb6ed Merge "Add a unit testing configuration fixture" 2013-08-13 14:02:29 +00:00
Mark McLoughlin
fbe3192d9a Add a unit testing configuration fixture
The configuration options registered by oslo.messaging should not be
directly relied upon by users of the library, since those config option
names could change in future.

Add an API which allows API users to override specific configuration
options e.g.

    self.messaging_conf = self.useFixture(messaging.ConfFixture(cfg.CONF))
    self.messaging_conf.transport_driver = 'fake'

Change-Id: If0d837e1b86e3b04237fde522551cfb81505a543
2013-08-13 13:52:42 +01:00
Mark McLoughlin
c846cf35b8 Add a TransportURL class to the public API
Nova's cells/rpc_driver.py has some code which allows user of the REST
API to update elements of a cell's transport URL (say, the host name of
the message broker) stored in the database. To achieve this, it has
a parse_transport_url() method which breaks the URL into its constituent
parts and an unparse_transport_url() which re-forms it again after
updating some of its parts.

This is all fine and, since it's fairly specialized, it wouldn't be a
big deal to leave this code in Nova for now ... except the unparse
method looks at CONF.rpc_backend to know what scheme to use in the
returned URL if now backend was specified.

oslo.messaging registers the rpc_backend option, but the ability to
reference any option registered by the library should not be relied upon
by users of the library. Imagine, for instance, if we renamed the option
in future (with backwards compat for old configurations), then this
would mean API breakage.

So, long story short - an API along these lines makes some sense, but
especially since not having it would mean we'd need to add some way to
query the name of the transport driver.

In this commit, we add a simple new TransportURL class:

  >>> url = messaging.TransportURL.parse(cfg.CONF, 'foo:///')
  >>> str(url), url
  ('foo:///', <TransportURL transport='foo'>)
  >>> url.hosts.append(messaging.TransportHost(hostname='localhost'))
  >>> str(url), url
  ('foo://localhost/', <TransportURL transport='foo', hosts=[<TransportHost hostname='localhost'>]>)
  >>> url.transport = None
  >>> str(url), url
  ('kombu://localhost/', <TransportURL transport='kombu', hosts=[<TransportHost hostname='localhost'>]>)
  >>> cfg.CONF.set_override('rpc_backend', 'bar')
  >>> str(url), url
  ('bar://localhost/', <TransportURL transport='bar', hosts=[<TransportHost hostname='localhost'>]>)

The TransportURL.parse() method equates to parse_transport_url() and
TransportURL.__str__() equates to unparse_transport().

The transport drivers are also updated to take a TransportURL as a
required argument, which simplifies the handling of transport URLs in
the drivers.

Change-Id: Ic04173476329858e4a2c2d2707e9d4aeb212d127
2013-08-12 23:30:43 +01:00
Mark McLoughlin
5aa7c37144 Ensure namespace package is installed
This is apparently fixed in pbr since I3972b3132619e8e2dd7e362ca5fe9d1e3add43b8
but I'm seeing the issue with pbr 0.5.21 on Fedora.

Related-Bug: #1194742
Change-Id: I136b8493c8d8d48a0116facf5f23c2a1479c070f
2013-08-12 14:35:33 +01:00
Jenkins
514e91cc95 Merge "Add transport URL support to rabbit driver" 2013-08-12 09:24:42 +00:00
Jenkins
3a7dde4fe8 Merge "Kill ability to specify exchange in transport URL" 2013-08-12 09:24:08 +00:00
Jenkins
f3b30fde49 Merge "Add thread-local store of request context" 2013-08-12 09:23:42 +00:00
Jenkins
c7236958d0 Merge "Add a context serialization hook" 2013-08-12 09:23:29 +00:00
Mark McLoughlin
9c110a4c94 Add transport URL support to rabbit driver
If a transport URL is supplied, transform it into the server_params
format which was previously used for cast_to_server() etc.

Change-Id: I453734a71748dc8d3ffc02ead7bfb92ffb0a6c7c
2013-08-12 07:50:30 +01:00
Mark McLoughlin
9cc66e1e01 Kill ability to specify exchange in transport URL
My original thinking was that if you're using the exchange name to
separate two instances of the applications (so their queues don't
collide) then the exchange name is pretty key to transport
configuration. In fact, it's really a virtual host that you'd use for
this (at least in the case of rabbit and qpid).

Also, Nova's cells code has already moved ahead with the assumption that
the path specifies a virtual host, so it'd only make sense to deviate
from that if there was a really good reason to.

Change-Id: Ic8b5dc3538b6b17afec524047acc2efa76366377
2013-08-12 06:09:48 +01:00
Jenkins
a7d7eb660e Merge "Fix handling expected exceptions in rabbit driver" 2013-08-12 01:14:12 +00:00
Andreas Jaeger
8cb17b8041 Fix capitalization, it's OpenStack
I'm fixing all other occurences of this string, incl. oslo-incubator.

Change-Id: If607379d4d1d4bc99084db4b01ada5dfd5c9fa3f
2013-08-11 20:14:39 +02:00
Mark McLoughlin
5fa7f93d09 Fix handling expected exceptions in rabbit driver
We shouldn't be logging expected exceptions. Add a failing rabbit driver
test to check this and fix it.

Change-Id: I78b758957117be7c11c5826a27dd6d1d4fffe9cb
2013-08-11 14:10:18 +01:00
Mark McLoughlin
f1612f2895 Add thread-local store of request context
Oslo's logging code has some useful support for including bits of the
request context in log messages. While this isn't exclusively about the
request context in a dispatching RPC method, it seems useful for
oslo.messaging to support the concept for at least this use case simply
by recording the context in a thread local store before dispatching an
endpoint method and immediately clearing it when the method returns.

Note, we don't need to store weak refs in our store because we will
clear the reference in all cases rather than ever leaving a stale
reference around in the store.

Change-Id: I70ac06ed3a2a891a7a7b388b1823a0f3b08f2dd1
2013-08-09 11:21:27 +01:00
Jenkins
6ec2c8bdbf Merge "Removes a redundant version_is_compatible function" 2013-08-09 07:49:37 +00:00
Mark McLoughlin
2abb40f9e9 Add a context serialization hook
The client call() and cast() methods take a request context argument
which is expected to be a dictionary. The RPC endpoint methods on the
server are invoked with a dictionary context argument.

However, Nova passes a nova.context.RequestContext on the client side
and the oslo-incubator RPC code deserializes that as a amqp.RpcContext
which looks vaguely compatible with Nova's RequestContext.

Support the serialization and deserialization of RequestContext objects
with an additional (de)serialize_context() hook on our Serializer class.

Note: this is a backwards incompatible API change because Serializer
implementations which do not implement the new abstract methods would
no longer be possible to instantiate. Not a problem with this commit,
but shows the type of compat concerns we'll need to think about once the
API is locked down for good.

Change-Id: I20782bad77fa0b0e396d082df852ca355548f9b7
2013-08-09 08:24:49 +01:00
Jenkins
e166f38a13 Merge "Add some docs on target version numbers" 2013-08-09 07:22:09 +00:00
Zhongyue Luo
63f1e0e5ec Removes a redundant version_is_compatible function
Two defined in _drivers.common and _utils.
Removed the one in _drivers.common

Change-Id: I613c2d9288a1bf333dd89c8844bd8467e8b34c42
2013-08-09 07:21:07 +00:00
Mark McLoughlin
bf171ede28 Document how call() handles remote exceptions
This is tricky stuff, so document it carefuly.

Related-Bug: #1162063
Change-Id: Id197bf87e9a7ed222508efe5d5246003ac02680e
2013-08-07 13:22:15 +01:00
Mark McLoughlin
ac2176cde3 Add a per-transport allow_remote_exmods API
Currently we have a allowed_rpc_exception_modules configuration variable
which we use to configure a per-project list of modules which we will
allow exceptions to be instantiated from when deserializing remote
errors.

It makes no sense for this to be user configurable, instead the list of
modules should be set when you create a transport.

Closes-Bug: #1031719
Change-Id: Ib40e92cb920996ec5e8f63d6f2cbd88fd01a90f2
2013-08-07 13:11:46 +01:00
Mark McLoughlin
66f597f30d Expose RemoteError exception in the public API
If a remote endpoint raises an exception which the client is not allowed
to (or cannot) deserialize, then RPCClient.call() raises a RemoteError
exception instead.

Make this exception type part of the public API.

Change-Id: I70be0ab7d40af3224d93d6bd0522c1a82f6303c3
2013-08-07 12:52:58 +01:00
Mark McLoughlin
9ac9f615b2 Implement failure replies in the fake driver
Change-Id: Ifd9ede7cb17a471ae2f9024b49ef6bbdc645476a
2013-08-07 12:52:58 +01:00
Mark McLoughlin
f6df32d943 Add API for expected endpoint exceptions
Review I4e7b19dc730342091fd70a717065741d56da4555 gives a lot of the
background here, but the idea is that some exceptions raised by an RPC
endpoint method do not indicate any sort of failure and should not be
logged by the server as an error.

The classic example of this is conductor's instance_get() method raising
InstanceNotFound. This is perfectly normal and should not be considered
an error.

The new API is a decorator which you can use with RPC endpoints methods
to indicate which exceptions are expected:

    @messaging.expected_exceptions(InstanceNotFound)
    def instance_get(self, context, instance_id):
        ...

but we also need to expose the ExpectedException type itself so that
direct "local" users of the endpoint class know what type will be used
to wrap expected exceptions. For example, Nova has an ExceptionHelper
class which unwraps the original exception from an ExpectedException and
re-raises it.

I've changed from client_exceptions() and ClientException to make it
more clear it's intent. I felt that the "client" naming gave the
impression it was intended for use on the client side.

Change-Id: Ieec4600bd6b70cf31ac7925a98a517b84acada4d
2013-08-07 12:52:54 +01:00
Mark McLoughlin
206c19e99e Add a driver method specifically for sending notifications
Notifications are an unusual case in that we need users to manually opt
in to new incompatible message formats by editing configuration because
there may be external consumers expecting the old format.

Add a send_notification() method to the driver interface and add a
format version paramater to the method, to make it clear that this
version selection is specifically for notifications.

In the case of the rabbit/qpid drivers, the 2.0 format is where we added
the message envelope.

Change-Id: Ib4925c308b1252503749962aa16f043281f2b429
2013-08-07 06:51:35 +01:00
Mark McLoughlin
294c99a6d2 Enforce target preconditions outside of drivers
The target preconditions (e.g. you need at least a topic to send) are
the same for all drivers, so enforce them before we ever call into a
driver.

Change-Id: Ic4e9bd94bd9f060ec0662d2bb778c699903dddc4
2013-08-07 06:51:24 +01:00
Mark McLoughlin
89079c6ea1 Add comments to ReplyWaiter.wait()
This method is fairly gnarly, so break my usual preference to make the
intent clear from the code and instead include detailed comments in the
method.

Change-Id: I107272a7eab85c70581652488a3c14ce0e18b906
2013-08-07 06:51:24 +01:00
Mark McLoughlin
b516271a80 Remove some FIXMEs and debug logging
These FIXMEs don't need fixing now that I think it through some more.
The reply format is specific to drivers and the 'ending' flag is part of
the existing wire protocol that we need to support even if we don't
support multicall().

Change-Id: I834f0bb01513b5318f0b365948a7d9247feb49bf
2013-08-07 06:51:24 +01:00
Mark McLoughlin
84a0693737 Remove unused IncomingMessage.done()
We appear to not have a use for this. I had originally thought we might
use this to ack messages one they've been processed and replied to, but
we actually have always acked messages as soon as they have been
deserialized and queued for dispatching.

Change-Id: I8e1fd565814f3b5e3ba0f1bc77e62ed52ff08661
2013-08-07 06:51:24 +01:00
Mark McLoughlin
7c305150ff Implement wait_for_reply timeout in rabbit driver
Note - the tests use timeout=0.01 because timeout=0 doesn't seem to be
working for some reason.

Change-Id: I814a3decdad5ddce0a1a2301ba2d59fa928b53a7
2013-08-07 06:51:03 +01:00
Mark McLoughlin
cb2623f46e Use testtools.TestCase assertion methods
I typically avoided using e.g. assertIsNone() thinking we couldn't use
it on 2.7, but now that we use testtools.TestCast there are a bunch of
useful assertion methods we can use.

Change-Id: I7696dc4744cdfd2466773326f202bc08dcfcbf0f
2013-08-07 06:43:55 +01:00
Mark McLoughlin
3471e02b4a Implement failure replies in rabbit driver
Make the rabbit driver properly serialize exceptions before sending them
back in a reply and then properly re-raise them on the client side.

Also, extend the rabbit driver test to cover this case.

Change-Id: I6b3d03edcd41810125ba6442db5515754f0c1ac9
2013-08-05 13:15:54 +01:00
Mark McLoughlin
a823368b72 Add test with multiple waiting sender threads
The trickiest logic in the rabbit driver is to handle the situation
where multiple threads are waiting for a reply on the same reply queue.

This commit adds unit testing for that scenario and fixes some bugs with
it.

Change-Id: I5c8fbeec49572a4f3badbcdae414dc44dc690b6a
2013-08-05 13:15:54 +01:00
Mark McLoughlin
a3a684d2c9 Fix race condition in ReplyWaiters.wake_all()
While we're iterating over the queues in ReplyWaiters.wake_all(), new
queues can be registered and we get:

  RuntimeError: dictionary changed size during iteration

Instead of using an iterator, take a snapshot list of message IDs and
operate on that.

We don't actually care about any new queues added after wake_all() is
called because the connection lock has already been dropped so one of
the other waiters must have picked it up.

We also don't need to worry about queues being removed - if we write to
a removed queue, that's not going to be a problem.

Change-Id: Ib572cbfd3a7346b76579f82b64aa85a03c1a4fb2
2013-08-05 13:15:54 +01:00
Mark McLoughlin
950c37c595 Add rabbit unit test for sending and receiving replies
Change-Id: I9574940904673257317a0caa86c585459e066ff7
2013-08-05 13:15:54 +01:00
Jenkins
96564b5f58 Merge "Add tests for rabbit driver wire protcol" 2013-08-05 10:57:42 +00:00
Jenkins
2f1b87d2f0 Merge "Pop _unique_id when checking for duplicates" 2013-08-05 10:06:06 +00:00
Jenkins
245d783623 Merge "Add a transport cleanup() method" 2013-08-05 10:05:07 +00:00