> On April 7, 2016, 5:57 p.m., Justin Ross wrote: > > proton-c/bindings/python/proton/reactor.py, line 197 > > <https://reviews.apache.org/r/45389/diff/6/?file=1330014#file1330014line197> > > > > Perhaps these setters and getters should be python properties. That > > seems to be the pattern elsewhere in proton python, and elsewhere in your > > patch.
I'm not sure they can. The host and port are actually associated with the _connection_, not the _reactor_. I crafted these APIs so they belong to the reactor API because we didn't want host/port information leaking into the AMQP Connection API. We can't make them a property of the Reactor, since we have to specify the connection. And I really don't want to associate host and port properties with the connection class. - Kenneth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45389/#review127631 ----------------------------------------------------------- On April 7, 2016, 2:55 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45389/ > ----------------------------------------------------------- > > (Updated April 7, 2016, 2:55 p.m.) > > > Review request for qpid, Gordon Sim, Justin Ross, and Robbie Gemmell. > > > Bugs: PROTON-1133 > https://issues.apache.org/jira/browse/PROTON-1133 > > > Repository: qpid-proton-git > > > Description > ------- > > 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 > ----- > > > 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/__init__.py 5ffede8 > proton-c/bindings/python/proton/reactor.py cda6248 > proton-c/include/proton/connection.h f8a688c > 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/engine/Connection.java > feff80b > 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/ > > > Testing > ------- > > Updated unit tests and re-checked modified examples. > > > Thanks, > > Kenneth Giusti > >
