-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22864/#review46611
-----------------------------------------------------------
All the automated test failures were due to a bug in
cpp/src/tests/brokertest.py that allowed connection option "protocol" to be set
to amqp1.0 even for native / 0-10 client. Below patch makes all tests passing:
Index: cpp/src/tests/brokertest.py
===================================================================
--- cpp/src/tests/brokertest.py (revision 1605024)
+++ cpp/src/tests/brokertest.py (working copy)
-344,9 +344,11 @@
@param native if True force use of the native qpid.messaging client
even if swig client is available.
"""
- if self.test.protocol: kwargs.setdefault("protocol",
self.test.protocol)
if native: connection_class = qpid.messaging.Connection
- else: connection_class = qm.Connection
+ else:
+ connection_class = qm.Connection
+ if (self.test.protocol and qm == qpid_messaging):
+ kwargs.setdefault("protocol", self.test.protocol)
return connection_class.establish(self.host_port(), timeout=timeout,
**kwargs)
@property
Until there will be some feedback, I will commit both the patch above and the
connection option handling within a day or two.
- Pavel Moravec
On June 24, 2014, 1:01 p.m., Pavel Moravec wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22864/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 1:01 p.m.)
>
>
> Review request for qpid, Alan Conway, Kenneth Giusti, Darryl Pierce, and
> Rafael Schloming.
>
>
> Bugs: QPID-5836
> https://issues.apache.org/jira/browse/QPID-5836
>
>
> Repository: qpid
>
>
> Description
> -------
>
> options need to be iterated to identify possible unknown options. Default
> values (even None, otherwise Python complains it does not know them) have to
> be set before the loop that overwrites the defaults.
>
> This is in my eyes better option than adding an extra check like:
>
> for key in options.keys():
> if key not in ["reconnect", "transport", "port", .. ]:
> raise error
>
> As this check would have to be maintained and there is a risk one forgets to
> add a new connection option there while handling it otherwise.
>
> The code change is more than reasonable but several automated tests failed -
> some of them (like ha_tests.RecoveryTests.test_queue_hold) due to connection
> option protocol:amqp1.0. While that option should be present _only_ for swig
> client that does not call endpoints.py and Connection class. Other test
> failures are not directly related to that "missing" option but I guess it
> would be the indirect reason (some connection supposed to provision something
> failed due to that option, so the provisioning was missing etc.).
>
> Any idea where & why _native_ client sets protocol:amqp1.0 ? (I even added
> the possibility protocol:amqp0-10 but that didnt help, obviously). Automated
> tests results added.
>
>
> Diffs
> -----
>
> /trunk/qpid/python/qpid/messaging/endpoints.py 1604713
>
> Diff: https://reviews.apache.org/r/22864/diff/
>
>
> Testing
> -------
>
> Manual test with connection option {'a':'b'} raises error:
>
> Traceback (most recent call last):
> File "get_queue_arguments.py", line 17, in <module>
> connection = Connection(brokerurl, **parms)
> File "/usr/lib/python2.6/site-packages/qpid/messaging/endpoints.py", line
> 195, in __init__
> raise ConnectionError("Unknown connection option " + key + " with value "
> + value)
> qpid.messaging.exceptions.ConnectionError: None(Unknown connection option a
> with value b)
>
>
> Thanks,
>
> Pavel Moravec
>
>
