search

rh-messaging / cli-proton-python

Proton Python client provided by QE for testing.
Apache License 2.0
4 stars 6 forks source link

FIX(Urls): Class Urls is not exported from _reactor.py to reactor.py in Qpid Proton Python #44

Closed rkubis closed 5 years ago

rkubis commented 5 years ago

…tor.py in Qpid Proton Python

Relates: PROTON-1997 [Python] Urls class is not exported by reactor

pematous commented 5 years ago

I don't like this. This is an existing bug in the library that was recently introduced. It should get fixed and I don't want to introduce workarounds to the master. This introduce risks to forget the roll-back after fixed.

@michalxo If you need a working pip package you should probably create your own and use local-install.

ssorj commented 5 years ago

@rkubis, what is the role of Urls in the cli tool code? It doesn't look to me like it's doing anything more than a list of strings would do, and a list of strings is the documented interface to connect.

I think Urls was an implementation detail that was previously exposed by accident.

rkubis commented 5 years ago

@ssorj Thanks for note, I will investigate the proposed solution.

jirkadanek commented 5 years ago

@astitcher Hi, in case you read GH notifications :P Could you please check https://issues.apache.org/jira/browse/PROTON-1997 and confirm if the move of Urls was intentional or not? We could deal with PROTON-1997 and this PR if we knew that for sure.

astitcher commented 5 years ago

To (try to) clarify what Justin is saying: As far as I can tell, you don't need the Urls class at all. It is a purely internal detail that was exposed accidentally and is not part of the API to the reactor connect method. You can replace the line with conn_opts['urls'] = self.opts.conn_urls and that should fix the problem and work as before.