WIP: Jb49688 python3 #2
Conversation
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Make stacktrace more visible in errors Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
finish() should be called to cleanly exit by invoking stop() Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
| password=self.pw, | ||
| virtual_host=self.vhost, | ||
| insist=False) | ||
| self._conn_params = dict( |
There was a problem hiding this comment.
Could just create pika.ConnectionParameters object here, instead of putting them in separate dict that is then just expanded with **
RuoteAMQP/launcher.py
Outdated
| @@ -13,7 +13,7 @@ | |||
| # You should have received a copy of the GNU General Public License | |||
| # along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
|
|
|||
| from amqplib import client_0_8 as amqp | |||
| import pika | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
This try/except is not needed... remnants from python 2.5 times :) And other places using json seem to have similar pattern
| from RuoteAMQP.workitem import Workitem | ||
| import logging | ||
| import time | ||
|
|
||
| logging.basicConfig(format='%(asctime)s %(name)s %(levelname)s: %(message)s', |
There was a problem hiding this comment.
This maybe shouldn't be doing global logging configuration. It's more responsibility of what ever is using the library. Not related to your changes, but just caught my eye.
| self.exception = None | ||
| self.trace = None | ||
| self.log = logging.getLogger(__name__) | ||
|
|
||
| def run(self): | ||
| try: | ||
| self.__participant.consume() | ||
| # Run the actual code for the workitem | ||
| self.__participant.consume(self.workitem) | ||
| except Exception as exobj: | ||
| # This should be configureable: | ||
| self.log.error("Exception in participant %s\n" |
There was a problem hiding this comment.
Could use log.exception(...) here and remove the traceback formatting below
| @@ -190,15 +191,15 @@ def participant_name(self): | |||
| """ | |||
| try: | |||
| return self._h['participant_name'] | |||
| except: | |||
| except IndexError: | |||
| BuildArch: noarch | ||
| Vendor: David Greaves <[email protected]> | ||
| Url: http://github.com/MeegoIntegration/python-ruote-amqp | ||
|
|
There was a problem hiding this comment.
Should there be Requires: python3-pika?
|
|
||
| def finish(self): | ||
| """ | ||
| Closes channel and connection | ||
| Call finish() to close the channel and connection and exit cleanly. | ||
| This invokes the die() callback which can be overridden to clean up. |
| """ | ||
| tag = msg.delivery_info["delivery_tag"] | ||
| tag = method.delivery_tag | ||
| self._consumer_tag = method.consumer_tag |
There was a problem hiding this comment.
The _consumer_tag should probably be taken from the return value of _channel.basic_consume(), so that it is available if finish() called before any messages have arrived
| self.__participant.workitem.wf_name)) | ||
| (self.workitem.participant_name, | ||
| self.workitem.wfid, | ||
| self.workitem.wf_name)) | ||
| self.log.error(format_block(traceback.format_exc())) | ||
| self.exception = exobj |
There was a problem hiding this comment.
It doesn't seem to be necessary to store references to the exception and trace in this thread object anymore, as the error processing is done here (earlier it was on the Participant class side)
| @@ -235,7 +236,7 @@ def forget(self): | |||
| try: | |||
| if self.params.forget: | |||
| return True | |||
| except: | |||
| except Exception: | |||
There was a problem hiding this comment.
self.params is a DictAttrProxy, so params.forget would be None if 'forget' is not in the params dict... I'm not sure what kind of exception this could throw, so the whole try/except seem unnecessary
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Signed-off-by: David Greaves <[email protected]>
Allow: * setting via [] with __setitem__ * creation by passing a dict which is wrapped (not copied) * pretty __str__ and __unicode__ handlers Signed-off-by: David Greaves <[email protected]>
Convert to python3
Use Pika
Use proper packaging
Support cancel and stop