Qpid
  1. Qpid
  2. QPID-3333

Make Python SWIG bindings a drop-in replacement for pure Python qpid.messaging package

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.11, Future
    • Fix Version/s: 0.17
    • Component/s: C++ Client
    • Labels:
      None

      Description

      These patches are make the compile Python SWIG bindings to the C++ client much closer to a drop-in replacement for the Python qpid.messaging package.

      1. 0015-Better-exception-translation.with_comment.patch
        7 kB
        Anthony Foglia
      2. 0015-Better-exception-translation.patch
        6 kB
        Anthony Foglia
      3. 0014-Fix-reference-counting.patch
        2 kB
        Anthony Foglia
      4. 0013-Handle-amqp-list-and-amqp-map-content.patch
        3 kB
        Anthony Foglia
      5. 0012-Add-connection-property-to-Session-object.patch
        0.8 kB
        Anthony Foglia
      6. 9900-Wrap-qpid-framing-NotAttachedException.patch
        2 kB
        Anthony Foglia
      7. 0009-Wrap-more-exceptions-2011-08-26.patch
        3 kB
        Anthony Foglia
      8. 9900-Wrap-qpid-framing-NotAttachedException.patch
        2 kB
        Anthony Foglia
      9. 0013-Handle-amqp-list-and-amqp-map-content.patch
        3 kB
        Anthony Foglia
      10. 0012-Add-connection-property-to-Session-object.patch
        0.8 kB
        Anthony Foglia
      11. 0009-Wrap-more-exceptions-2011-08-26.patch
        3 kB
        Anthony Foglia
      12. 0011-Wrap-returned-UUIDs-as-Python-UUIDs.patch
        3 kB
        Anthony Foglia
      13. 0010-Add-session-property-to-senders-and-receivers.patch
        1 kB
        Anthony Foglia
      14. 0009-Wrapping-of-more-exceptions.patch
        4 kB
        Anthony Foglia
      15. 0008-Fixes-and-more-improvements-to-Message-objects.patch
        3 kB
        Anthony Foglia
      16. 0007-Handle-connection-options.patch
        2 kB
        Anthony Foglia
      17. 0006-Change-Message-interface-to-match-pure-python.patch
        5 kB
        Anthony Foglia
      18. 0005-Change-Sender-interface-to-match-pure-python.patch
        2 kB
        Anthony Foglia
      19. 0004-Change-Receiver-interface-to-match-pure-python.patch
        2 kB
        Anthony Foglia
      20. 0003-Change-Session-interface-to-match-pure-python.patch
        2 kB
        Anthony Foglia
      21. 0002-Change-Connection-interface-to-match-pure-python.patch
        2 kB
        Anthony Foglia
      22. 0001-Wrap-NoMessageAvailable-as-Empty-exception.patch
        2 kB
        Anthony Foglia

        Activity

        Hide
        Anthony Foglia added a comment -

        These patches are my progress so far in aligning the two APIs. It's not perfect, but it's good enough to cover our current use case. It's ready for wider testing, and any other advice on getting these patches in the next version.

        Among the areas that still need improvement:

        • Exceptions: Only the NoMessagesFound <-> Empty translation is currently done. And no work has been done on building a heirarchy as in the pure Python API
        • Dispositions: No work has been done. Trying to use them will throw an exception.

        They only affect cpp/bindings/qpid/python/python.i file, so nothing but the Python SWIG wrappers will be affected.

        Show
        Anthony Foglia added a comment - These patches are my progress so far in aligning the two APIs. It's not perfect, but it's good enough to cover our current use case. It's ready for wider testing, and any other advice on getting these patches in the next version. Among the areas that still need improvement: Exceptions: Only the NoMessagesFound <-> Empty translation is currently done. And no work has been done on building a heirarchy as in the pure Python API Dispositions: No work has been done. Trying to use them will throw an exception. They only affect cpp/bindings/qpid/python/python.i file, so nothing but the Python SWIG wrappers will be affected.
        Hide
        Gordon Sim added a comment -

        Fantastic! This is an important step towards a more unified project offering. Patches look good to me, I vote we commit them.

        There are as yet no tests or examples for the python bindings which is something we need to address. Just to try out these patches I used a modified spout and drain. Getting the existing examples and tests to run over the bindings would be the ideal solution, minimising work.

        Show
        Gordon Sim added a comment - Fantastic! This is an important step towards a more unified project offering. Patches look good to me, I vote we commit them. There are as yet no tests or examples for the python bindings which is something we need to address. Just to try out these patches I used a modified spout and drain. Getting the existing examples and tests to run over the bindings would be the ideal solution, minimising work.
        Hide
        Anthony Foglia added a comment - - edited

        How did you have to modify your spout and drain, other than any changes in module names?

        One problem with making this a fully drop-in replacements is that the python api is a subpackage of the qpid package. So users typically import qpid.messaging and use the full package name (e.g. connection = qpid.messaging.Connection.). One can't do the usual trick of trying to open the compiled version easily, e.g.

        try :
          import cPickle as pickle
        except ImportError :
          import pickle
        

        Instead in our code we've been doing,

        try :
          import cqpid as messaging
        except ImportError :
          import qpid.messaging as messaging
        

        I don't like using the generic name "messaging", but it was the easiest.

        Another problem, which I need to dive into more, is that the make files install the compiled _cqpid.so in $

        {PREFIX}/lib, not ${PREFIX}

        /lib/pythonX.Y/site-packages or $

        {PREFIX}

        /lib64/pythonX.Y/site-packages. I don't know any of the details of autotools or cmake, but I want to dig in a little before making a new bug report on it.

        Show
        Anthony Foglia added a comment - - edited How did you have to modify your spout and drain, other than any changes in module names? One problem with making this a fully drop-in replacements is that the python api is a subpackage of the qpid package. So users typically import qpid.messaging and use the full package name (e.g. connection = qpid.messaging.Connection.). One can't do the usual trick of trying to open the compiled version easily, e.g. try : import cPickle as pickle except ImportError : import pickle Instead in our code we've been doing, try : import cqpid as messaging except ImportError : import qpid.messaging as messaging I don't like using the generic name "messaging", but it was the easiest. Another problem, which I need to dive into more, is that the make files install the compiled _cqpid.so in $ {PREFIX}/lib, not ${PREFIX} /lib/pythonX.Y/site-packages or $ {PREFIX} /lib64/pythonX.Y/site-packages. I don't know any of the details of autotools or cmake, but I want to dig in a little before making a new bug report on it.
        Hide
        Gordon Sim added a comment -

        The only changes other than the import were around the connection options which I changed to explicitly use setOption(). Looking at the swig input file and the changes you made already it seemed like that would be easy to resolve as well (I just didn't have time right then). One other thing that would be nice would be getting the string representation of a message to work as in qpid.messaging (not as important, but nice to have and based again on your existing changes looked straightforward to a SWIG novice!).

        On the imports I just changed:

        < from qpid.messaging import *
        ---
        > from cqpid import *
        > from qpid.datatypes import uuid4
        
        Show
        Gordon Sim added a comment - The only changes other than the import were around the connection options which I changed to explicitly use setOption(). Looking at the swig input file and the changes you made already it seemed like that would be easy to resolve as well (I just didn't have time right then). One other thing that would be nice would be getting the string representation of a message to work as in qpid.messaging (not as important, but nice to have and based again on your existing changes looked straightforward to a SWIG novice!). On the imports I just changed: < from qpid.messaging import * --- > from cqpid import * > from qpid.datatypes import uuid4
        Hide
        Anthony Foglia added a comment -

        Can you be more precise about what you did with setOption? We've currently stumbled across a problem where the default sasl-mechanism is different in the two APIs. And since the option name (sasl-mechanism vs. sasl_mechanisms) is different, I was going to just translate underscores to dashes. (The meaning is slightly different, but close enough for now.) Perhaps you had found a better way?

        (Another issue is that there's no way to get option values of Connection objects in the C++, so debugging will be harder.)

        And I'll consider changing the string representation of a Message, though I might wait till I have to also fix other issues with them, which will certainly crop up.

        Show
        Anthony Foglia added a comment - Can you be more precise about what you did with setOption? We've currently stumbled across a problem where the default sasl-mechanism is different in the two APIs. And since the option name (sasl-mechanism vs. sasl_mechanisms) is different, I was going to just translate underscores to dashes. (The meaning is slightly different, but close enough for now.) Perhaps you had found a better way? (Another issue is that there's no way to get option values of Connection objects in the C++, so debugging will be harder.) And I'll consider changing the string representation of a Message, though I might wait till I have to also fix other issues with them, which will certainly crop up.
        Hide
        Gordon Sim added a comment -

        fyi: I'm going to commit this patch next week. If anyone has any comments on it, now is the time to make them.

        Show
        Gordon Sim added a comment - fyi: I'm going to commit this patch next week. If anyone has any comments on it, now is the time to make them.
        Hide
        Gordon Sim added a comment -

        Spooky! You commented at (almost) exactly the same time as me!

        Here is what I changed, this was not intended to be a solution just a quick hack to test out the changes:

        22c22,23
        < from qpid.messaging import *
        ---
        > from cqpid import *
        > from qpid.datatypes import uuid4
        94,97c95,98
        < conn = Connection(opts.broker,
        <                   reconnect=opts.reconnect,
        <                   reconnect_interval=opts.reconnect_interval,
        <                   reconnect_limit=opts.reconnect_limit)
        ---
        > conn = Connection(opts.broker)
        > if opts.reconnect: conn.setOption("reconnect", opts.reconnect)
        > if opts.reconnect_interval: conn.setOption("reconnect_interval", opts.reconnect_interval)
        > if opts.reconnect_limit: conn.setOption("reconnect_limit", opts.reconnect_limit)
        

        The c++ client now recognises the same options as python so e.g. sasl_mechanisms is fine. Good point about the lack of getOption(), I'll add that in.

        Show
        Gordon Sim added a comment - Spooky! You commented at (almost) exactly the same time as me! Here is what I changed, this was not intended to be a solution just a quick hack to test out the changes: 22c22,23 < from qpid.messaging import * --- > from cqpid import * > from qpid.datatypes import uuid4 94,97c95,98 < conn = Connection(opts.broker, < reconnect=opts.reconnect, < reconnect_interval=opts.reconnect_interval, < reconnect_limit=opts.reconnect_limit) --- > conn = Connection(opts.broker) > if opts.reconnect: conn.setOption("reconnect", opts.reconnect) > if opts.reconnect_interval: conn.setOption("reconnect_interval", opts.reconnect_interval) > if opts.reconnect_limit: conn.setOption("reconnect_limit", opts.reconnect_limit) The c++ client now recognises the same options as python so e.g. sasl_mechanisms is fine. Good point about the lack of getOption(), I'll add that in.
        Hide
        Anthony Foglia added a comment - - edited

        Here are two additional patches.

        The first adds support for options in the Connection initializer method. (Note multiple sasl-mechanisms are not supported. I don't understan the C++ code enough to do that.)

        The second fixes some bugs with the original Message patch--The reply to was not being set properly, and the ttl property was a C++ duration, not a number--and adds a __repr__ method based on the one in the qpid.messaging package.

        Show
        Anthony Foglia added a comment - - edited Here are two additional patches. The first adds support for options in the Connection initializer method. (Note multiple sasl-mechanisms are not supported. I don't understan the C++ code enough to do that.) The second fixes some bugs with the original Message patch--The reply to was not being set properly, and the ttl property was a C++ duration, not a number--and adds a __repr__ method based on the one in the qpid.messaging package.
        Hide
        Anthony Foglia added a comment -

        Okay. Another issue between the two. For defining auto-deleting queues in the python library, we've been using "x-declare:

        {auto_delete: true}

        ", as used in the tests, but in SWIG, the correct key is auto-delete, and auto_delete is silently ignored.

        Luckily, it appears the python library accepts auto-delete as well. Are we correct? And if so, should we open up a bug to bring the test code in the Python library (and any documentation) into compliance?

        Show
        Anthony Foglia added a comment - Okay. Another issue between the two. For defining auto-deleting queues in the python library, we've been using "x-declare: {auto_delete: true} ", as used in the tests, but in SWIG, the correct key is auto-delete, and auto_delete is silently ignored. Luckily, it appears the python library accepts auto-delete as well. Are we correct? And if so, should we open up a bug to bring the test code in the Python library (and any documentation) into compliance?
        Hide
        Gordon Sim added a comment -

        Committed patches 0001-0008 as r1145698. I'll leave this open as there are still some areas to address. Getting pretty close though, thanks Anthony this is invaluable!

        Show
        Gordon Sim added a comment - Committed patches 0001-0008 as r1145698. I'll leave this open as there are still some areas to address. Getting pretty close though, thanks Anthony this is invaluable!
        Hide
        Gordon Sim added a comment -

        Re auto_delete v. auto-delete, we probably want to support both. Certainly not good that auto_delete is ignored by c++ client.

        Show
        Gordon Sim added a comment - Re auto_delete v. auto-delete, we probably want to support both. Certainly not good that auto_delete is ignored by c++ client.
        Hide
        Robbie Gemmell added a comment -

        The commit for this, r1145698, seems to have broken (log below) the python bindings build on my RHEL 5.3 installation, which has python 2.4.3 and swig 1.3.29 on it. Is a particular version of python or swig required but not being enforced via the configure script?

        Thanks,
        Robbie

        Making all in python
        make[2]: Entering directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python'
        /usr/bin/swig -c++ -python -w362,401 -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src  -I../../../src/qmf -I/usr/include -o cqpid.cpp ./python.i
        /usr/bin/swig -c++ -python -w362,401 -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src  -I../../../src/qmf -I/usr/include -o cqpid.cpp ./python.i
        ../../../include/qpid/messaging/Address.h:153: Warning(361): operator! ignored
        ../../../include/qpid/messaging/Address.h:153: Warning(361): operator! ignored
        ../../../include/qpid/messaging/Address.h:152: Warning(503): Can't wrap 'operator bool' unless renamed to a valid identifier.
        ../../../include/qpid/messaging/Address.h:152: Warning(503): Can't wrap 'operator bool' unless renamed to a valid identifier.
        ../../../include/qpid/messaging/Message.h:199: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::Map &map) not supported (no type checking rule for 'qpid::types::Variant::Map &').
        ../../../include/qpid/messaging/Message.h:211: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::List &list,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List &').
        ../../../include/qpid/messaging/Message.h:199: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::Map &map) not supported (no type checking rule for 'qpid::types::Variant::Map &').
        ../../../include/qpid/messaging/Message.h:211: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::List &list,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List &').
        ../../../include/qpid/messaging/Message.h:223: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::Map const &map,qpid::messaging::Message &message) not supported (no type checking rule for 'qpid::types::Variant::Map const &').
        ../../../include/qpid/messaging/Message.h:235: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::List const &list,qpid::messaging::Message &message,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List const &').
        ../../../include/qpid/messaging/Message.h:223: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::Map const &map,qpid::messaging::Message &message) not supported (no type checking rule for 'qpid::types::Variant::Map const &').
        ../../../include/qpid/messaging/Message.h:235: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::List const &list,qpid::messaging::Message &message,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List const &').
        ../../../include/qpid/messaging/Connection.h:80: Warning(467): Overloaded qpid::messaging::Connection(std::string const &url,qpid::types::Variant::Map const &options=qpid::types::Variant::Map()) not supported (no type checking rule for 'qpid::types::Variant::Map const &').
        ../../../include/qpid/messaging/Connection.h:80: Warning(467): Overloaded qpid::messaging::Connection(std::string const &url,qpid::types::Variant::Map const &options=qpid::types::Variant::Map()) not supported (no type checking rule for 'qpid::types::Variant::Map const &').
        make  all-am
        make[3]: Entering directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python'
        if /bin/sh ../../../libtool --tag=CXX --mode=compile g++ -DHAVE_CONFIG_H -I. -I. -I../../../src -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src   -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I/usr/include/python2.4 -fno-strict-aliasing -g -O2 -MT _cqpid_la-cqpid.lo -MD -MP -MF ".deps/_cqpid_la-cqpid.Tpo" -c -o _cqpid_la-cqpid.lo `test -f 'cqpid.cpp' || echo './'`cqpid.cpp; \
                then mv -f ".deps/_cqpid_la-cqpid.Tpo" ".deps/_cqpid_la-cqpid.Plo"; else rm -f ".deps/_cqpid_la-cqpid.Tpo"; exit 1; fi
        mkdir .libs
         g++ -DHAVE_CONFIG_H -I. -I. -I../../../src -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I/usr/include/python2.4 -fno-strict-aliasing -g -O2 -MT _cqpid_la-cqpid.lo -MD -MP -MF .deps/_cqpid_la-cqpid.Tpo -c cqpid.cpp  -fPIC -DPIC -o .libs/_cqpid_la-cqpid.o
        cqpid.cpp: In function 'qpid::messaging::Session qpid_messaging_Connection__session(qpid::messaging::Connection*, const std::string&, bool)':
        cqpid.cpp:2986: error: '$self' was not declared in this scope
        cqpid.cpp:2992: error: '$self' was not declared in this scope
        cqpid.cpp:2995: error: '$self' was not declared in this scope
        make[3]: *** [_cqpid_la-cqpid.lo] Error 1
        make[3]: Leaving directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python'
        make[2]: *** [all] Error 2
        make[2]: Leaving directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python'
        make[1]: *** [all-recursive] Error 1
        make[1]: Leaving directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid'
        make: *** [all-recursive] Error 1
        
        Show
        Robbie Gemmell added a comment - The commit for this, r1145698, seems to have broken (log below) the python bindings build on my RHEL 5.3 installation, which has python 2.4.3 and swig 1.3.29 on it. Is a particular version of python or swig required but not being enforced via the configure script? Thanks, Robbie Making all in python make[2]: Entering directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python' /usr/bin/swig -c++ -python -w362,401 -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I../../../src/qmf -I/usr/include -o cqpid.cpp ./python.i /usr/bin/swig -c++ -python -w362,401 -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I../../../src/qmf -I/usr/include -o cqpid.cpp ./python.i ../../../include/qpid/messaging/Address.h:153: Warning(361): operator! ignored ../../../include/qpid/messaging/Address.h:153: Warning(361): operator! ignored ../../../include/qpid/messaging/Address.h:152: Warning(503): Can't wrap 'operator bool' unless renamed to a valid identifier. ../../../include/qpid/messaging/Address.h:152: Warning(503): Can't wrap 'operator bool' unless renamed to a valid identifier. ../../../include/qpid/messaging/Message.h:199: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::Map &map) not supported (no type checking rule for 'qpid::types::Variant::Map &'). ../../../include/qpid/messaging/Message.h:211: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::List &list,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List &'). ../../../include/qpid/messaging/Message.h:199: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::Map &map) not supported (no type checking rule for 'qpid::types::Variant::Map &'). ../../../include/qpid/messaging/Message.h:211: Warning(467): Overloaded qpid::messaging::decode(qpid::messaging::Message const &message,qpid::types::Variant::List &list,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List &'). ../../../include/qpid/messaging/Message.h:223: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::Map const &map,qpid::messaging::Message &message) not supported (no type checking rule for 'qpid::types::Variant::Map const &'). ../../../include/qpid/messaging/Message.h:235: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::List const &list,qpid::messaging::Message &message,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List const &'). ../../../include/qpid/messaging/Message.h:223: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::Map const &map,qpid::messaging::Message &message) not supported (no type checking rule for 'qpid::types::Variant::Map const &'). ../../../include/qpid/messaging/Message.h:235: Warning(467): Overloaded qpid::messaging::encode(qpid::types::Variant::List const &list,qpid::messaging::Message &message,std::string const &encoding=std::string()) not supported (no type checking rule for 'qpid::types::Variant::List const &'). ../../../include/qpid/messaging/Connection.h:80: Warning(467): Overloaded qpid::messaging::Connection(std::string const &url,qpid::types::Variant::Map const &options=qpid::types::Variant::Map()) not supported (no type checking rule for 'qpid::types::Variant::Map const &'). ../../../include/qpid/messaging/Connection.h:80: Warning(467): Overloaded qpid::messaging::Connection(std::string const &url,qpid::types::Variant::Map const &options=qpid::types::Variant::Map()) not supported (no type checking rule for 'qpid::types::Variant::Map const &'). make all-am make[3]: Entering directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python' if /bin/sh ../../../libtool --tag=CXX --mode=compile g++ -DHAVE_CONFIG_H -I. -I. -I../../../src -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I/usr/include/python2.4 -fno-strict-aliasing -g -O2 -MT _cqpid_la-cqpid.lo -MD -MP -MF ".deps/_cqpid_la-cqpid.Tpo" -c -o _cqpid_la-cqpid.lo `test -f 'cqpid.cpp' || echo './'`cqpid.cpp; \ then mv -f ".deps/_cqpid_la-cqpid.Tpo" ".deps/_cqpid_la-cqpid.Plo"; else rm -f ".deps/_cqpid_la-cqpid.Tpo"; exit 1; fi mkdir .libs g++ -DHAVE_CONFIG_H -I. -I. -I../../../src -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I../../../include -I../../../include -I../../../src/qmf -I../../../src -I../../../src -I/usr/include/python2.4 -fno-strict-aliasing -g -O2 -MT _cqpid_la-cqpid.lo -MD -MP -MF .deps/_cqpid_la-cqpid.Tpo -c cqpid.cpp -fPIC -DPIC -o .libs/_cqpid_la-cqpid.o cqpid.cpp: In function 'qpid::messaging::Session qpid_messaging_Connection__session(qpid::messaging::Connection*, const std::string&, bool)': cqpid.cpp:2986: error: '$self' was not declared in this scope cqpid.cpp:2992: error: '$self' was not declared in this scope cqpid.cpp:2995: error: '$self' was not declared in this scope make[3]: *** [_cqpid_la-cqpid.lo] Error 1 make[3]: Leaving directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python' make[2]: *** [all] Error 2 make[2]: Leaving directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid/python' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/gemmellr/workspace/qpid/qpid/cpp/bindings/qpid' make: *** [all-recursive] Error 1
        Hide
        Gordon Sim added a comment -

        Possibly, I'll see if I can get a RHEL5.3 to debug. On 5.5 with the same versions of swig and python I get a clean build, though the python bindings appear not to be part of it.

        You can workaround the build error by specifying --without-swig to configure if needed.

        Show
        Gordon Sim added a comment - Possibly, I'll see if I can get a RHEL5.3 to debug. On 5.5 with the same versions of swig and python I get a clean build, though the python bindings appear not to be part of it. You can workaround the build error by specifying --without-swig to configure if needed.
        Hide
        Gordon Sim added a comment -

        Doh, just forgot to install python-devel. With that done I see the same error. Looks like it may actually just be a typo that somehow is handled differently in later versions of swig. Trying out a fix now.

        Show
        Gordon Sim added a comment - Doh, just forgot to install python-devel. With that done I see the same error. Looks like it may actually just be a typo that somehow is handled differently in later versions of swig . Trying out a fix now.
        Hide
        Gordon Sim added a comment -

        Fixed in r1153906.

        Show
        Gordon Sim added a comment - Fixed in r1153906.
        Hide
        Robbie Gemmell added a comment -

        Thanks Gordon, all working again

        Show
        Robbie Gemmell added a comment - Thanks Gordon, all working again
        Hide
        Anthony Foglia added a comment -

        Thanks for testing the patches. I'm working on a RHEL6 server, and didn't test it on other machines.

        As we keep working with the bindings, I keep finding little improvements. I'm adding two additional patches. The first wraps more exceptions. (I still don't have any exception heirarchy, and I think I should have renamed the exceptions in the compiled _cqpid.so, not in the bare-bones python cqpid lib that wraps it, but that's shouldn't affect the capabilities.) The second adds the Sender.session and Receiver.session properties/member variables.

        Show
        Anthony Foglia added a comment - Thanks for testing the patches. I'm working on a RHEL6 server, and didn't test it on other machines. As we keep working with the bindings, I keep finding little improvements. I'm adding two additional patches. The first wraps more exceptions. (I still don't have any exception heirarchy, and I think I should have renamed the exceptions in the compiled _cqpid.so, not in the bare-bones python cqpid lib that wraps it, but that's shouldn't affect the capabilities.) The second adds the Sender.session and Receiver.session properties/member variables.
        Hide
        Gordon Sim added a comment -

        One question on the 0009 patch: How are you triggering the NotAttachedException (i.e. Detached in python)? The messaging API should not throw any qpid::framing exceptions, if it does that is a bug. I'd prefer not to include files from that in qpid.i.

        The 0010 patch is fine, I can apply that now or wait 'till we resolve 0009.

        Show
        Gordon Sim added a comment - One question on the 0009 patch: How are you triggering the NotAttachedException (i.e. Detached in python)? The messaging API should not throw any qpid::framing exceptions, if it does that is a bug. I'd prefer not to include files from that in qpid.i. The 0010 patch is fine, I can apply that now or wait 'till we resolve 0009.
        Hide
        Anthony Foglia added a comment -

        I don't know how we got the NotAttachedException. One of the other coders on my team discovered it. I have the chat logs, and but they're not very helpful.

        Me: No. Any idea what's different? You're running on what machine?
        Him: it happened to two machines at once
        Him: oh, maybe a queue overflowed and it's just a different error message than I'm used to from the qpid messaging library
        [...]
        Him: maybe it's idsconnecting from the broker when it sees the queue is full?
        Him: because sessionDisconnected is a pretty different sounding exception from ueueOverFlow or whatever it is we're currently catching

        This was a month ago, and we were performing some stress tests on our code. And our code isn't currently catching Detached anymore, so there's no help there.

        I'd suggest leaving the patch in. If you/we ever figure out what threw it, and properly translate it, we can add a patch to also convert that. There's no reason you can't have multiple C++ exception types translate to the same Python type. It's not ideal, but it's possible.

        The only concern would be if the NotAttachedException shouldn't be translated to a Detached in a signficant number of cases.

        Another option is to include the qpid::framing header in python.i, not qpid.i, but I figured other bindings should eventually do the same translations. It does give you one less place to change code, should you ever catch and translate the exception in the qpid::messaging space.

        I vote for putting it in.

        If I have time, I'll work on breaking that patch up so the Detached exception stuff is separate from the other three. (Ironically, I think it was multiple changes originally, that I rebased into one for your convenience. )

        Show
        Anthony Foglia added a comment - I don't know how we got the NotAttachedException. One of the other coders on my team discovered it. I have the chat logs, and but they're not very helpful. Me: No. Any idea what's different? You're running on what machine? Him: it happened to two machines at once Him: oh, maybe a queue overflowed and it's just a different error message than I'm used to from the qpid messaging library [...] Him: maybe it's idsconnecting from the broker when it sees the queue is full? Him: because sessionDisconnected is a pretty different sounding exception from ueueOverFlow or whatever it is we're currently catching This was a month ago, and we were performing some stress tests on our code. And our code isn't currently catching Detached anymore, so there's no help there. I'd suggest leaving the patch in. If you/we ever figure out what threw it, and properly translate it, we can add a patch to also convert that. There's no reason you can't have multiple C++ exception types translate to the same Python type. It's not ideal, but it's possible. The only concern would be if the NotAttachedException shouldn't be translated to a Detached in a signficant number of cases. Another option is to include the qpid::framing header in python.i, not qpid.i, but I figured other bindings should eventually do the same translations. It does give you one less place to change code, should you ever catch and translate the exception in the qpid::messaging space. I vote for putting it in. If I have time, I'll work on breaking that patch up so the Detached exception stuff is separate from the other three. (Ironically, I think it was multiple changes originally, that I rebased into one for your convenience. )
        Hide
        Anthony Foglia added a comment -

        I haven't had time to break patch 0009 up yet, but I did work on a patch to convert the qpid::type::UUID objects to Python's uuid.UUID objects. The conversion is only being done one way for now. But this does fix the problem where converting a UUID would return a pointer to random memory.

        Show
        Anthony Foglia added a comment - I haven't had time to break patch 0009 up yet, but I did work on a patch to convert the qpid::type::UUID objects to Python's uuid.UUID objects. The conversion is only being done one way for now. But this does fix the problem where converting a UUID would return a pointer to random memory.
        Hide
        Gordon Sim added a comment -

        Thanks! Regarding the NotAttachedException, I don't believe it should be mapped to Detached. I think it is an internal error and should map to a RuntimeException. That will also make it more likely that we catch the root cause in future (I've been trying different theories without success in reproducing it).

        Show
        Gordon Sim added a comment - Thanks! Regarding the NotAttachedException, I don't believe it should be mapped to Detached. I think it is an internal error and should map to a RuntimeException. That will also make it more likely that we catch the root cause in future (I've been trying different theories without success in reproducing it).
        Hide
        Gordon Sim added a comment -

        and obviously if it maps to RuntimeException then there is no need to explicitly reference it as I believe that will be the default.

        Show
        Gordon Sim added a comment - and obviously if it maps to RuntimeException then there is no need to explicitly reference it as I believe that will be the default.
        Hide
        Anthony Foglia added a comment -

        Okay, I've split the 0009 patch wrapping the exceptions into two. The new 0009 only wraps the exceptions from qpid::messaging. The wrapping of the qpid::framing::NotAttachedException is now in a patch I numbered 9900. I still think it should be included, but at least now it's separate, so you can include the others without it.

        In addition I've added two more patches. The first adds a connection property to the Session objects. The second adds the ability to use lists and maps as the content of Messages.

        Show
        Anthony Foglia added a comment - Okay, I've split the 0009 patch wrapping the exceptions into two. The new 0009 only wraps the exceptions from qpid::messaging. The wrapping of the qpid::framing::NotAttachedException is now in a patch I numbered 9900. I still think it should be included, but at least now it's separate, so you can include the others without it. In addition I've added two more patches. The first adds a connection property to the Session objects. The second adds the ability to use lists and maps as the content of Messages.
        Hide
        Anthony Foglia added a comment -

        Re-adding, with the desired license.

        Show
        Anthony Foglia added a comment - Re-adding, with the desired license.
        Hide
        Anthony Foglia added a comment -

        Also, I've added a subtask/bug report (QPID-3458); the compiled part of the swig libraries is installed in the wrong place. It's really a separate bug, but as this is becoming a place to track the SWIG binding progress, it made sense to make it a subtask for now.

        Show
        Anthony Foglia added a comment - Also, I've added a subtask/bug report ( QPID-3458 ); the compiled part of the swig libraries is installed in the wrong place. It's really a separate bug, but as this is becoming a place to track the SWIG binding progress, it made sense to make it a subtask for now.
        Hide
        Gordon Sim added a comment -

        Thanks Anthony! I've committed patches 0009 through 0013.

        Show
        Gordon Sim added a comment - Thanks Anthony! I've committed patches 0009 through 0013.
        Hide
        Anthony Foglia added a comment -

        This is a patch to fix the reference counting when passing objects from C++ to Python. As described on the mailing list last month (http://qpid.2158936.n2.nabble.com/Memory-leak-in-QPID-SWIG-Python-Bindings-td7001193.html), objects returned from the C++ library had one too many references set in the swig wrapper. This patch fixes that.

        Here's some example code run with the patch. (Without the patch, the refcount is 2, which is incorrect.)

        >>> import cqpid, sys
        >>> m = cqpid.Message(content={"foo": [5, 10]})
        >>> m.content
        {'foo': [5L, 10L]}
        >>> sys.getrefcount(m.content)
        1
        
        Show
        Anthony Foglia added a comment - This is a patch to fix the reference counting when passing objects from C++ to Python. As described on the mailing list last month ( http://qpid.2158936.n2.nabble.com/Memory-leak-in-QPID-SWIG-Python-Bindings-td7001193.html ), objects returned from the C++ library had one too many references set in the swig wrapper. This patch fixes that. Here's some example code run with the patch. (Without the patch, the refcount is 2, which is incorrect.) >>> import cqpid, sys >>> m = cqpid.Message(content={ "foo" : [5, 10]}) >>> m.content {'foo': [5L, 10L]} >>> sys.getrefcount(m.content) 1
        Hide
        Anthony Foglia added a comment -

        This patch does a better job of translating exceptions from C++ to Python. I'm not near a computer with a qpid server, so I didn't check this beyond compilation. The changes are straightforward though, so it should work.

        Show
        Anthony Foglia added a comment - This patch does a better job of translating exceptions from C++ to Python. I'm not near a computer with a qpid server, so I didn't check this beyond compilation. The changes are straightforward though, so it should work.
        Hide
        Anthony Foglia added a comment -

        Whoops. I forgot to include this comment in the patch explaining why some of the definitions of Python exceptions were commented out. (Because they aren't thrown by the C++ library.) This is an updated version of the patch, with that comment.

        Show
        Anthony Foglia added a comment - Whoops. I forgot to include this comment in the patch explaining why some of the definitions of Python exceptions were commented out. (Because they aren't thrown by the C++ library.) This is an updated version of the patch, with that comment.
        Hide
        Gordon Sim added a comment -

        Though there are still probably some slight differences, I'm closing this issue to make it easier to track progress. Further differences can be raised as a new issue.

        A huge thank you to Anthony for his invaluable work on this so far!

        Show
        Gordon Sim added a comment - Though there are still probably some slight differences, I'm closing this issue to make it easier to track progress. Further differences can be raised as a new issue. A huge thank you to Anthony for his invaluable work on this so far!

          People

          • Assignee:
            Gordon Sim
            Reporter:
            Anthony Foglia
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development