Re: [openstack-dev] Opinions needed: Changing method signature in RPC callback ...
On 07/18/2013 05:56 PM, Eric Windisch wrote: These callback methods are part of the Kombu driver (and maybe part of Qpid), but are NOT part of the RPC abstraction. These are private methods. They can be broken for external consumers of these methods, because there shouldn't be any. It will be a good lesson to anyone that tries to abuse private methods. I was wondering about that, but I assumed some parts of amqp.py were used by other transports as well (and not just impl_kombu.py) There are several callbacks in amqp.py that would be affected. The code in amqp.py is used by the Kombu and Qpid drivers and might implement the public methods expected by the abstraction, but does not define it. The RPC abstraction is defined in __init__.py, and does not define callbacks. Other drivers, granted only being the ZeroMQ driver at present, are not expected to define a callback method and as a private method -- would have no template to follow nor an expectation to have this method. I'm not saying your proposed changes are bad or invalid, but there is no need to make concessions to the possibility that code outside of oslo would be using callback(). This opens up the option, besides creating a new method, to simply updating all the existing method calls that exist in amqp.py, impl_kombu.py, and impl_qpid.py. Gotcha ... thanks Eric. Yeah, the outer api is very generic. I did a little more research and, unfortunately, it seems the inner amqp implementations are being used by others. So I'll have to be careful with the callback signature. Ceilometer, for example, seems to be leaving zeromq support as an exercise for the reader. Perhaps oslo-messaging will make this abstraction easier to enforce. Cheers! -S -- Regards, Eric Windisch ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Opinions needed: Changing method signature in RPC callback ...
On 07/18/2013 11:09 AM, Sandy Walsh wrote:
2. make a generic CallContext() object to include with message that has
anything else we need (a one-time signature break)
call_context = CallContext({delivery_info: {...}, wait: False})
callback(message, call_context)
or just callback(message, **kwargs) of course.
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Opinions needed: Changing method signature in RPC callback ...
On Thu, 2013-07-18 at 11:09 -0300, Sandy Walsh wrote: 3. some other ugly python hack that I haven't thought of yet. I have two possibilities, though neither is really ideal. One is to provide a duplicate call for registering the callback, but where the callback has the extra arguments. The other is to use the functions in the inspect module to determine what convention the callback is expecting. -- Kevin L. Mitchell kevin.mitch...@rackspace.com ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Opinions needed: Changing method signature in RPC callback ...
On 07/18/2013 10:09 AM, Sandy Walsh wrote:
Hey y'all!
Running into an interesting little dilemma with a branch I'm working on.
Recently, I introduced a branch in oslo-common to optionally .reject() a
kombu message on an exception. Currently, we always .ack() all messages
even if the processing callback fails. For Ceilometer, this is a problem
... we have to guarantee we get all notifications.
The patch itself was pretty simple, but didn't work :) The spawn_n()
call was eating the exceptions coming from the callback. So, in order to
get the exceptions it's simple enough to re-wrap the callback, but I
need to pool.waitall() after the spawn_n() to ensure none of the
consumers failed. Sad, but a necessary evil. And remember, it's only
used in a special case, normal openstack rpc is unaffected and remains
async.
But it does introduce a larger problem ... I have to change the rpc
callback signature.
Old: callback(message)
New: callback(message, delivery_info=None, wait_for_consumers=False)
(The delivery_info is another thing, we were dumping the message info on
the floor, but this has important info in it)
My worry is busting all the other callbacks out there that use
olso-common.rpc
Some options:
1. embed all these flags and extra data in the message structure
message = {'_context_stuff': ...,
'payload: {...},
'_extra_magic': {...}}
This would be my preference. #2 below is essentially the same thing but
with some object-orientation for sugar -- and it breaks the existing
structure, which #1 doesn't.
best,
-jay
2. make a generic CallContext() object to include with message that has
anything else we need (a one-time signature break)
call_context = CallContext({delivery_info: {...}, wait: False})
callback(message, call_context)
3. some other ugly python hack that I haven't thought of yet.
Look forward to your thoughts on a solution!
Thanks
-S
My work-in-progess is here:
https://github.com/SandyWalsh/openstack-common/blob/callback_exceptions/openstack/common/rpc/amqp.py#L373
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Opinions needed: Changing method signature in RPC callback ...
On Thu, Jul 18, 2013 at 10:09 AM, Sandy Walsh sandy.wa...@rackspace.comwrote: My worry is busting all the other callbacks out there that use olso-common.rpc These callback methods are part of the Kombu driver (and maybe part of Qpid), but are NOT part of the RPC abstraction. These are private methods. They can be broken for external consumers of these methods, because there shouldn't be any. It will be a good lesson to anyone that tries to abuse private methods. -- Regards, Eric Windisch ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Opinions needed: Changing method signature in RPC callback ...
On 07/18/2013 03:55 PM, Eric Windisch wrote: On Thu, Jul 18, 2013 at 10:09 AM, Sandy Walsh sandy.wa...@rackspace.com mailto:sandy.wa...@rackspace.com wrote: My worry is busting all the other callbacks out there that use olso-common.rpc These callback methods are part of the Kombu driver (and maybe part of Qpid), but are NOT part of the RPC abstraction. These are private methods. They can be broken for external consumers of these methods, because there shouldn't be any. It will be a good lesson to anyone that tries to abuse private methods. I was wondering about that, but I assumed some parts of amqp.py were used by other transports as well (and not just impl_kombu.py) There are several callbacks in amqp.py that would be affected. -- Regards, Eric Windisch ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] Opinions needed: Changing method signature in RPC callback ...
These callback methods are part of the Kombu driver (and maybe part of Qpid), but are NOT part of the RPC abstraction. These are private methods. They can be broken for external consumers of these methods, because there shouldn't be any. It will be a good lesson to anyone that tries to abuse private methods. I was wondering about that, but I assumed some parts of amqp.py were used by other transports as well (and not just impl_kombu.py) There are several callbacks in amqp.py that would be affected. The code in amqp.py is used by the Kombu and Qpid drivers and might implement the public methods expected by the abstraction, but does not define it. The RPC abstraction is defined in __init__.py, and does not define callbacks. Other drivers, granted only being the ZeroMQ driver at present, are not expected to define a callback method and as a private method -- would have no template to follow nor an expectation to have this method. I'm not saying your proposed changes are bad or invalid, but there is no need to make concessions to the possibility that code outside of oslo would be using callback(). This opens up the option, besides creating a new method, to simply updating all the existing method calls that exist in amqp.py, impl_kombu.py, and impl_qpid.py. -- Regards, Eric Windisch ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
