You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kenneth Giusti <kg...@apache.org> on 2016/02/11 21:29:33 UTC
Review Request 43500: QPID-7053: provide a means to asynchronously
wake up a
python application when an error occurs in the background driver thread
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/
-----------------------------------------------------------
Review request for qpid and Justin Ross.
Bugs: QPID-7053
https://issues.apache.org/jira/browse/QPID-7053
Repository: qpid
Description
-------
An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
Diffs
-----
trunk/qpid/python/qpid/messaging/endpoints.py 1726802
trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
Diff: https://reviews.apache.org/r/43500/diff/
Testing
-------
new tests added for the api
Thanks,
Kenneth Giusti
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Brian Bouterse <bm...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review119084
-----------------------------------------------------------
trunk/qpid/python/qpid/messaging/endpoints.py (line 78)
<https://reviews.apache.org/r/43500/#comment180355>
This line should be indended like L#58 according to PEP0257
trunk/qpid/python/qpid/messaging/endpoints.py (line 85)
<https://reviews.apache.org/r/43500/#comment180357>
I like the function signature here. +1 to this.
- Brian Bouterse
On Feb. 11, 2016, 8:29 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 8:29 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Justin Ross <jr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review118915
-----------------------------------------------------------
trunk/qpid/python/qpid/messaging/endpoints.py (line 49)
<https://reviews.apache.org/r/43500/#comment180228>
This is called _error_callback here and _exception_notify_handler below. Why are they different?
- Justin Ross
On Feb. 11, 2016, 8:29 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 8:29 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Justin Ross <jr...@apache.org>.
> On Feb. 11, 2016, 9:18 p.m., Kenneth Giusti wrote:
> > trunk/qpid/python/qpid/messaging/endpoints.py, line 49
> > <https://reviews.apache.org/r/43500/diff/1/?file=1240394#file1240394line49>
> >
> > I originally called the new method 'set_error_callback()' and forgot to rename the internal member.
Follow up: did you and Brian decide exception_notify_handler was the right name?
- Justin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review118919
-----------------------------------------------------------
On Feb. 11, 2016, 8:29 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 8:29 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Brian Bouterse <bm...@gmail.com>.
> On Feb. 11, 2016, 9:18 p.m., Kenneth Giusti wrote:
> > trunk/qpid/python/qpid/messaging/endpoints.py, line 49
> > <https://reviews.apache.org/r/43500/diff/1/?file=1240394#file1240394line49>
> >
> > I originally called the new method 'set_error_callback()' and forgot to rename the internal member.
>
> Justin Ross wrote:
> Follow up: did you and Brian decide exception_notify_handler was the right name?
I propose it be: set_async_exception_notify_handler
It's a long name but it's very explicit. The addition of set_ is conventional to indicate setting versus unsetting. It is also consistent with the set_message_received_handler on Session. The addition of async identifies that this is for asynchronous exceptions only which I feel is a useful distinction.
- Brian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review118919
-----------------------------------------------------------
On Feb. 11, 2016, 8:29 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 8:29 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review118919
-----------------------------------------------------------
trunk/qpid/python/qpid/messaging/endpoints.py (line 49)
<https://reviews.apache.org/r/43500/#comment180232>
I originally called the new method 'set_error_callback()' and forgot to rename the internal member.
- Kenneth Giusti
On Feb. 11, 2016, 8:29 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 8:29 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Brian Bouterse <bm...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review119323
-----------------------------------------------------------
trunk/qpid/python/qpid/messaging/endpoints.py (line 47)
<https://reviews.apache.org/r/43500/#comment180640>
Please add Endpoint to __all__ at the bottom of the module so that Endpoint will be included in the docs explicitly. That will allow its docstrings to be shown also.
trunk/qpid/python/qpid/messaging/endpoints.py (line 48)
<https://reviews.apache.org/r/43500/#comment180647>
Consider adding this into the @undocumented list also. It accepts no arguments so it's not something we need to tell users anything special about. If you do want to add it along with __setattr__ I tested that the following would work at the object docstring:
@undocumented: __setattr_, __init__
trunk/qpid/python/qpid/messaging/endpoints.py (line 57)
<https://reviews.apache.org/r/43500/#comment180641>
Please move docstring here, and remove set_async_exception_notify_handler from Session, Connection, Receiver, and Sender
trunk/qpid/python/qpid/messaging/endpoints.py (line 60)
<https://reviews.apache.org/r/43500/#comment180643>
Consider giving Endpoint the following docstring to hide its __setattr__ method:
"""
@undocumented: __setattr__
"""
Consider adding a docstring back that explains that this calls the _async_exception_notify_handler when 'error' is set.
trunk/qpid/python/qpid/messaging/endpoints.py (line 644)
<https://reviews.apache.org/r/43500/#comment180644>
This should have the object itself passed into the callback.
We had talked about using inspect. After thinking it over I propose we start passing the object as an argument and if the registered handler is incompatible it will raise an exception that the handler doesn't accept enough arguments. In other words let normal Python error handling handle this for us.
trunk/qpid/python/qpid/messaging/endpoints.py (line 731)
<https://reviews.apache.org/r/43500/#comment180645>
Please rename this function to:
set_message_received_notify_handler
- Brian Bouterse
On Feb. 15, 2016, 7:15 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2016, 7:15 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/driver.py 1726802
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
> trunk/qpid/python/setup.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Brian Bouterse <bm...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review119349
-----------------------------------------------------------
Ship it!
Ship It!
- Brian Bouterse
On Feb. 16, 2016, 6:58 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2016, 6:58 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/driver.py 1726802
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
> trunk/qpid/python/setup.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Brian Bouterse <bm...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/#review119346
-----------------------------------------------------------
trunk/qpid/python/qpid/messaging/endpoints.py (line 651)
<https://reviews.apache.org/r/43500/#comment180658>
nice, +1
- Brian Bouterse
On Feb. 16, 2016, 6:58 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43500/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2016, 6:58 p.m.)
>
>
> Review request for qpid and Justin Ross.
>
>
> Bugs: QPID-7053
> https://issues.apache.org/jira/browse/QPID-7053
>
>
> Repository: qpid
>
>
> Description
> -------
>
> An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
>
> This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
>
>
> Diffs
> -----
>
> trunk/qpid/python/qpid/messaging/driver.py 1726802
> trunk/qpid/python/qpid/messaging/endpoints.py 1726802
> trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
> trunk/qpid/python/setup.py 1726802
>
> Diff: https://reviews.apache.org/r/43500/diff/
>
>
> Testing
> -------
>
> new tests added for the api
>
>
> Thanks,
>
> Kenneth Giusti
>
>
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/
-----------------------------------------------------------
(Updated Feb. 16, 2016, 6:58 p.m.)
Review request for qpid and Justin Ross.
Bugs: QPID-7053
https://issues.apache.org/jira/browse/QPID-7053
Repository: qpid
Description
-------
An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
Diffs (updated)
-----
trunk/qpid/python/qpid/messaging/driver.py 1726802
trunk/qpid/python/qpid/messaging/endpoints.py 1726802
trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
trunk/qpid/python/setup.py 1726802
Diff: https://reviews.apache.org/r/43500/diff/
Testing
-------
new tests added for the api
Thanks,
Kenneth Giusti
Re: Review Request 43500: QPID-7053: provide a means to
asynchronously wake
up a python application when an error occurs in the background driver thread
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43500/
-----------------------------------------------------------
(Updated Feb. 15, 2016, 7:15 p.m.)
Review request for qpid and Justin Ross.
Bugs: QPID-7053
https://issues.apache.org/jira/browse/QPID-7053
Repository: qpid
Description
-------
An application can use the python-qpid client API via a "semi-polled" approach by leveraging the "message_received" callback registered to a Session. This allows the app to do other things while waiting for a message to arrive. However, if an error occurs in the background driver thread, the app will never be notified. And since a message may never arrive due to the error, the app can hang.
This change allows the task to register a callback that is invoked when the driver thread detects an error. This is in addition to the original behavior - to raise the error when the app next calls into the API. The intent of the callback is to cause the app to schedule itself to re-poll the API.
Diffs (updated)
-----
trunk/qpid/python/qpid/messaging/driver.py 1726802
trunk/qpid/python/qpid/messaging/endpoints.py 1726802
trunk/qpid/python/qpid/tests/messaging/endpoints.py 1726802
trunk/qpid/python/setup.py 1726802
Diff: https://reviews.apache.org/r/43500/diff/
Testing
-------
new tests added for the api
Thanks,
Kenneth Giusti