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
Rework Python binding to use only core APIs #173
Conversation
Tested against Pyngus library - works for me |
Built qpid-proton; passes self tests.
The one_router and two_router tests fail with a ctest output of: 19: ====================================================================== Does the event still have a container? Is there another way to get at the container from the event? |
@ChugR I'll take a look at the new code, but from the stack trace you gave I'd say the event here in on_timer_task() does not have a container! |
@ChugR @ganeshmurthy I've fixed the problem of the missing container for on_timer_task events. If you have a moment please run the dispatch tests again and tell me what the other problems are. It has been difficult (and unreliable) for me to run the entire dispatch built in test suite although I'm making some progress with that. So I'd be grateful if you can give it another go. |
Hi @astitcher, I got down the very latest code from this PR and ran qpid-dispatch master against this PR. All but one test passed. The test is called system_tests_authz_service_plugin. This test never fails against proton master branch. Here is the traceback from the failure - 39: Test command: /usr/bin/python "/home/gmurthy/opensource/qpid-dispatch/build/tests/run.py" "unit2" "-v" "system_tests_authz_service_plugin" |
@ganeshmurthy Thank you - I'll fix that issue then get back to you again. |
I get the same two errors/stack traces seen by @ganeshmurthy. In addition, I occasionally (50%) see a failure in system_tests_delivery_abort. The stack trace starts out the same but then diverges for the last three frames: (M=1914e ?6) chug@unused build> ctest -VV -R system_tests_delivery_abort 40: Test command: /bin/python "/home/chug/git/qpid-dispatch/build/tests/run.py" "unit2" "-v" "system_tests_delivery_abort" 0% tests passed, 1 tests failed out of 1 Total Test time (real) = 5.96 sec The following tests FAILED: |
1 similar comment
I get the same two errors/stack traces seen by @ganeshmurthy. In addition, I occasionally (50%) see a failure in system_tests_delivery_abort. The stack trace starts out the same but then diverges for the last three frames: (M=1914e ?6) chug@unused build> ctest -VV -R system_tests_delivery_abort 40: Test command: /bin/python "/home/chug/git/qpid-dispatch/build/tests/run.py" "unit2" "-v" "system_tests_delivery_abort" 0% tests passed, 1 tests failed out of 1 Total Test time (real) = 5.96 sec The following tests FAILED: |
@ChugR @ganeshmurthy another version of code to test. |
All router tests pass now. Thanks. |
- Python binding now only uses APIs from Proton Core library. It uses Python APIs to do all IO and uses Proton purely to process the AMQP protocol. - It is very compatible with the existing higher level Python APIs. [In modules proton, proton.reactor, proton.handlers, proton.utils] - Passes the python tests as well as before - Works with Python 2 and Python 3 - Works on Unix and Windows - Runs all the python examples
- Remove all swig bound APIs that are only in libqpid-proton - Link Python extension lib to libqpid-proton-core
- Fixes a problem with dispatch unit tests
- Ignore interrupted select syscalls -- We do this as the dispatch systems tests use proton calls in a signal handler -- It's not clear to me that uis actually allowed - it wouldn't be in raw C - So it's not entirely clear this is the correct way to go or that the code that causes this issue doesn't need fixinf itself!
d1899ea
to
8dc796d
Compare
This is a reworking of the Python binding to Proton to avoid using APIs outside the Proton Core library. Thus the Python based reactive code has been rewritten so that the proton.reactor and proton.handler modules use only core APIs.
Necessarily this is a large change to the Python binding code, but a lot of effort has gone into maintaining the existing proton.reactor and proton.handler APIs. However any code that relied on functionality of the Proton-C reactor APIs directly will no longer work as these APIs aren't available from Python any longer.
One area that may be different and is still not 100% solid is in the area of error/exception handling. I think that you will get exceptions in the same cases as before, but they could very probably be different exceptions. It is also possible that some errors are handled differently. Please raise bugs for any differences/problems found.
This code has been tested with the integrated tests and passes on Linux, MacOS and Windows on CI systems. Some tests (related to SSL/TLS) fail on some Windows systems, but the same tests fail previous to these changes on the the same systems - the failures don't seem due to these changes. The included python examples all run with this reworked binding. It has also been tested with the quiver benchmark tool and is broadly the same performance as the previous binding.
Please try this with your Python code and let me know of any problems.