qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pavel Moravec" <pmora...@redhat.com>
Subject Re: Review Request 22864: [Python client] Report error when encountering unrecognized connection option
Date Wed, 25 Jun 2014 08:48:14 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message