Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMQ-6828: Improvement to python AMQP example. #262

Merged
merged 1 commit into from Oct 18, 2017

Conversation

jameshnsears
Copy link
Contributor

As per feedback from https://issues.apache.org/jira/browse/AMQ-6828.

All feedback appreciated.

@jameshnsears jameshnsears changed the title Improvement to python AMQP example. AMQ-6828: Improvement to python AMQP example. Oct 6, 2017
@@ -0,0 +1,26 @@
## Overview

This is an example of how to use the [Qpid Messaging API](https://qpid.apache.org/components/messaging-api/index.html) with ActiveMQ via the AMQP protocol.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually an example of Qpid Proton, rather than the linked Qpid Messaging API.

More specifically however, its an example of the old 'Messenger' API, which has seen little attention for some time due and wouldn't be recommended for new users. PROTON-1585 even removed the examples for it at Qpid .

I'd suggest the example actually be removed instead, or else updated to use the newer Proton reactive APIs, e.g. demonstrated by http://qpid.apache.org/releases/qpid-proton-0.17.0/proton/python/examples/index.html and http://qpid.apache.org/releases/qpid-proton-0.17.0/proton/python/book/tutorial.html

@jameshnsears
Copy link
Contributor Author

It is not my day today. The link is wrong, it should of course be Qpid Proton API.

I think though, what kills this push request is PROTON-1585 - in that it looks like the end of the proton messenger API, or at least its promotion. I'm grateful to have been told about this though, so at least something positive has come out of this experience.

I also think it would be best to remove the python AMQP example as well.

@jameshnsears
Copy link
Contributor Author

OK - I've changed my mind after all, and produced a python proton reactor example. In the end, to me, it kind of felt wrong not having one.

Please let me know what / if anything I need to change.

And thanks for your feedback.

@jameshnsears jameshnsears reopened this Oct 11, 2017
@tabish121
Copy link
Contributor

First thing to do is to rebase against master and then squash your commits into one, right now the history in the PR is a bit messy

@tabish121
Copy link
Contributor

I've tried running the new samples but so far I didn't have any luck, the readme steps don't seem to work, I get an error on the 'pip install' bit when it hits the proton stage, and the pytest bits don't work as they complain about invalid command line args such as the '--flake8' option. The original example on master while not working real well for me does still manage to send and receive some messages.

@jameshnsears jameshnsears force-pushed the AMQ-6828 branch 4 times, most recently from 4f55c33 to b3eacfa Compare October 12, 2017 12:54
@jameshnsears
Copy link
Contributor Author

jameshnsears commented Oct 12, 2017

Thanks for pushing things further & sorry you encountered a problem

I admit I am surprised - I wasn't expecting anything like this. Live and learn.

If your able to send me a https://gist.github.com/, or a comment here in git, of what pip said then it will help me investigate further. At the moment it looks like it might be an environment problem, possibly some photon dependencies that screwed up the pip install.

In the meantime I've updated the readme to try and be more helpful; rebased off master; squashed my commits & pushed.

@tabish121
Copy link
Contributor

tabish121 commented Oct 12, 2017

Would be nice if I could just run the example via the standard 'python sender.py' as the old ones worked but that just seems to do nothing either with or without a broker running.

Some output from the pip install:

Collecting pytest==3.2.2 (from -r requirements.txt (line 20))
Using cached pytest-3.2.2-py2.py3-none-any.whl
Collecting pytest-cov==2.5.1 (from -r requirements.txt (line 21))
Using cached pytest_cov-2.5.1-py2.py3-none-any.whl
Collecting pytest-flake8==0.8.1 (from -r requirements.txt (line 22))
Using cached pytest_flake8-0.8.1-py2.py3-none-any.whl
Collecting pytest-ordering==0.5 (from -r requirements.txt (line 23))
Using cached pytest_ordering-0.5-py2.py3-none-any.whl
Collecting python-qpid-proton==0.17.0 (from -r requirements.txt (line 24))
Using cached python-qpid-proton-0.17.0.tar.gz
Complete output from command python setup.py egg_info:
Traceback (most recent call last):
File "", line 1, in
ImportError: No module named setuptools

----------------------------------------

Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-mR5kGQ/python-qpid-proton/
You are using pip version 8.1.1, however version 9.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.

I've already got proton installed so really don't want it messing about with my installation anyway

@jameshnsears
Copy link
Contributor Author

jameshnsears commented Oct 12, 2017

I'm pretty sure that it's a local environment issue: your python environment can't build python-qpid-proton.

I'm saying this as I created a simple travis evironment, using the same requirements.txt, that seems to prove it: https://travis-ci.org/jameshnsears/activemq/builds/287164818

If you open up one of the travis build jobs from that link, look at the Raw log (I used pip -vv, so tonnes of logging I'm afraid) you can search for "python setup.py egg_info", then at the end of the Raw log you can see what "pip list" says. There's a load of _cproton.so compiling between these two points.

What do you think?

I can remove all this requirements.txt related stuff (modify the readme) and go back to two, or three, simple python files. I can assume that whomever is running these files has an environment that contains a pre-built compatabile python-qpid-proton. I can live with that.

What would you like me to do?

@tabish121
Copy link
Contributor

I don't have an issue in general with the setup script, I just can't validate it works. As for the others I mainly can't tell if they are working, when I run the sender and receiver they print nothing regardless of there being a broker running or not so there's no feedback. With the original examples they produce output to indicate that the sender has sent 10k messages, and the receiver oddly reads only 1k or them hehe.

@jameshnsears
Copy link
Contributor Author

I've simplified the code, rebased & squashed it. The integration test has gone (and all that pytest stuff). You can now test the code as before - simply by running the examples. The readme has links to dependency instillation. Best of all, the examples print messages out!

Please give it a go and let me know how it looks :-)

@tabish121
Copy link
Contributor

Great thanks for keeping at it. I ran them and they do indeed now produce and consume so that's a plus :)

I did get some errors on the console so not sure if there's something else going on:

Message{subject="s0", body=b"b0"}
Message{subject="s1", body=b"b1"}
Message{subject="s2", body=b"b2"}
Message{subject="s3", body=b"b3"}
Message{subject="s4", body=b"b4"}
Message{subject="s5", body=b"b5"}
Message{subject="s6", body=b"b6"}
Message{subject="s7", body=b"b7"}
Message{subject="s8", body=b"b8"}
Message{subject="s9", body=b"b9"}
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored
Exception TypeError: "'NoneType' object is not callable" in <object repr() failed> ignored

@gemmellr
Copy link
Member

FWIW I also see the same thing as above, running against proton master which will shortly become 0.18.0. I did a run with -v for verbose output which suggested maybe the issue is in the 'config' file, or how its used, but alas I know next to nothing about python and am unsure how to further identify and fix whatever the issue is.

@jameshnsears jameshnsears force-pushed the AMQ-6828 branch 2 times, most recently from 95da925 to e450368 Compare October 13, 2017 17:06
@jameshnsears
Copy link
Contributor Author

OK - I've pushed a fix. I was able to reproduce the problem on an upto date debian python 2.7.13.

As sugguested it was to do with the "config.py" file - it was being imported into the "receiver.py" file and brought in with it the import "from proton import Message".

The same original code (and the new code) works just fine on my python 3.5.3. And also worked just fine in an ubuntu travis environment that, amongst 4 different python versions, also featured 2.7.13. Maybe it's the way python 2..7.13 has been built? Or maybe it's what SWIG (the C to python library) outputs. I don't know.

It's been an interesting lesson if nothing else :-D

Let me know if you need me to do anything else. And thanks for your time.

@tabish121
Copy link
Contributor

tabish121 commented Oct 13, 2017

I see the same error on cleanup of "content" when running the 'sender.py' bit as that still pulls in the content bit via the import. I'm trying now to rebuild my proton install from upstream to see if that helps at all

My machine has python 2.7.12 installed

@jameshnsears
Copy link
Contributor Author

jameshnsears commented Oct 14, 2017

Rightly or wrongly it looks like its an issue with python-qpid-proton (0.17.0). For example, with python 3.5.3 when running receiver.py I get inconsistent output from the print statement (example below is me having run sender.py twice):

jsears@debian:~/GIT_REPOS/activemq/assembly/src/release/examples/amqp/python$ ~/bin/python3.5-ve01/bin/python receiver.py
Message{subject="s0", body="b0"}
Message{subject="s1", body="b1"}
Message{subject="s2", body="b2"}
Message{subject="s3", body="b3"}
Message{subject="s4", body="b4"}
Message{subject="s5", body="b5"}
Message{subject="s6", body="b6"}
Message{subject="s7", body="b7"}
Message{subject="s8", body="b8"}
Message{subject="s9", body="b9"}
jsears@debian:~/GIT_REPOS/activemq/assembly/src/release/examples/amqp/python$ ~/bin/python3.5-ve01/bin/python receiver.py
Message{subject="s0", body="b0"}
Message{delivery_count=1, subject="s1", body="b1"}
Message{delivery_count=1, subject="s2", body="b2"}
Message{delivery_count=1, subject="s3", body="b3"}
Message{delivery_count=1, subject="s4", body="b4"}
Message{delivery_count=1, subject="s5", body="b5"}
Message{delivery_count=1, subject="s6", body="b6"}
Message{delivery_count=1, subject="s7", body="b7"}
Message{delivery_count=1, subject="s8", body="b8"}
Message{subject="s9", body="b9"}
jsears@debian:~/GIT_REPOS/activemq/assembly/src/release/examples/amqp/python$

Maybe I'm just using the reactor API incorrectly? Then again I'd kind of expect consistent output.

Any advice welcome. Should I raise this as a JIRA in https://issues.apache.org/jira/projects/PROTON?

@ssorj
Copy link

ssorj commented Oct 17, 2017

I found three things worth noting.

First, the TypeError that Tim saw looks like a Proton bug to do with a relatively recent change in object lifecycle. For me, it does not occur if I construct the messages in the same package (as opposed to in content.py). Worth noting that is annoying but harmless. It's some faulty error handling in a destructor that only occurs on shutdown.

Second, I don't understand the redelivery stuff in the second run of the receiver. It looks to me like the redelivery accounting on the broker is weird. I'm not finding anything wrong on the client ends. Here's what I get in a protocol trace from the receiver side.

https://gist.github.com/ssorj/cd098c3f7054da39752f6c1ead75bc28

Last, I modified the receiver program to shutdown more cleanly (avoid going back into on_message during close).

https://gist.github.com/ssorj/cf94cc306ceea30850feb0035d506bb1

@jameshnsears
Copy link
Contributor Author

Justin - the broker I've been using is ActiveMQ 5.15.0

I'll update the receiver.py with your improvements (it showed my lack of understanding of the api). I'm not tied to any broker, as this effort is only for a pet project of mine.

I'll try things out with 5.15.1, and another broker (qpid or maybe Artemis). I'll post some feedback tomorrow.

Thanks for your help :-)

@tabish121
Copy link
Contributor

Thanks for looking @ssorj that helps. So if we get those changes into the samples then I think we can go ahead and merge this. The delivery count bit is interesting but seemingly unrelated at this point to the examples. That's something we could dig into in another issue, I'm not sure right now what is triggering it to behave that way. At fist I thought it could be a bug in the lastDeliveredSequenceId tracking bits in the broker side session handler but given that the last message arrives without a delivery count I'm a bit stumped at the moment on what is going on.

@ssorj
Copy link

ssorj commented Oct 17, 2017

I raised https://issues.apache.org/jira/browse/PROTON-1634 for the TypeError output.

Migrate example python code to proton reactor API.
@jameshnsears
Copy link
Contributor Author

I've fetched; rebased; applied @ssorj's patch; squashed and pushed.

Out of interest I ran the sender and receiver against artemis-2.3.0 & don't see the issue I see in activemq-5.15.0 or activemq-5.15.1

Let me know if I need to do anything else. Thanks everyone for the help.

@asfgit asfgit merged commit cc6cb74 into apache:master Oct 18, 2017
asfgit pushed a commit that referenced this pull request Oct 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants