qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kenneth Giusti <kgiu...@apache.org>
Subject Re: Review Request 45389: WIP PROTON-1133 - Avoid using hostname as a transport address
Date Sun, 03 Apr 2016 23:34:38 GMT

This is an automatically generated e-mail. To reply, visit:

(Updated April 3, 2016, 11:34 p.m.)

Review request for qpid, Gordon Sim, Justin Ross, and Robbie Gemmell.


I've re-factored the patch based on feedback.  In summary:

1) I've broken out the setting of the address from the creation of the reactor connection.
 This was necessary as the Connector must change the connection's address in order to handle
connection fail-over.

2) These APIs strictly belong to the Reactor class.  This should prevent using these new methods
as part of the Connection API proper.

3) I haven't added the sanitizing of the outgoing hostname.  I'd like to keep that in its
own patch in case we need to back it out/change it based on feedback.

Bugs: PROTON-1133

Repository: qpid-proton-git


This involves a change to the Reactor API.

This patch implements a new API for creating outgoing connections via the Reactor class: pn_reactor_connection_to_url().
 It is meant to replace the existing practice of creating a connection via pn_reactor_connection()
then setting the transport address in the connection's hostname field.

Not 100% convinced this is appropriate.  It introduces a URL parameter to the Proton Connection
object, where it may make better sense to associate the URL with the Transport instead (pn_reactor_transport_to_url()???).

The URL parameter is used by the Proton iohandler to create the socket connection.  If an
application does not use the Proton iohandler (by overriding the reactor's global handler),
then it is the responsiblity of whatever handler is being provided to use the URL to set up
the socket connection.  This was also the case for the old method that used the connection's
hostname setting, so this is not a behavioral change.

Diffs (updated)

  examples/java/reactor/src/main/java/org/apache/qpid/proton/example/reactor/Send.java 22da720

  examples/python/reactor/send.py c718780 
  examples/python/reactor/tornado-send.py 54b8618 
  proton-c/bindings/python/proton/reactor.py cda6248 
  proton-c/include/proton/reactor.h e91b169 
  proton-c/src/reactor/connection.c 4a57bfd 
  proton-c/src/tests/reactor.c 1e706e2 
  proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java 9d67d49 
  proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java 40eddac 
  proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java 0eb126a 
  proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java 10c591a 
  tests/java/org/apache/qpid/proton/ProtonJInterop.java 31306ef 
  tests/java/shim/creactor.py 95fd020 

Diff: https://reviews.apache.org/r/45389/diff/


Updated unit tests and re-checked modified examples.


Kenneth Giusti

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