-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/#review132776
-----------------------------------------------------------


Ship it!




Love it. Give unto the reactor what is the reactor's and unto the container 
what is the container's. One doc nit which you can ignore at will. Ship It.


proton-c/include/proton/connection.h (line 335)
<https://reviews.apache.org/r/47243/#comment197024>

    Nit: It's not illegal, just very weird. It might make sense in some non-DNS 
environment with different rules about "hostname".
    
    How about "Note: the virtual host string is passed verbatim, it is not 
parsed as a URL or modified in any way. It should not contain numeric IP 
addresses or port numbers unless that is what you intend to send as the virtual 
host name"


- Alan Conway


On May 11, 2016, 4:52 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47243/
> -----------------------------------------------------------
> 
> (Updated May 11, 2016, 4:52 p.m.)
> 
> 
> Review request for qpid, Alan Conway, Chug Rolke, Cliff Jansen, Justin Ross, 
> and Robbie Gemmell.
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> The pn_connection_set_hostname() interface is used to set the
> 'hostname' field in the Open performative.  By definition this is the
> 'virtual host' and should not be used by reactor for the network
> address.  The network address for outgoing connections should be set
> by using the reactor's pn_reactor_connection_to_host() factory, or the
> pn_reactor_set_connection_host() when re-connecting to a different
> host.  For inbound connections, the peer address is provided by the
> acceptor and cannot be modified.  In both cases, the
> pn_reactor_get_connection_address() method can be used to obtain the
> peer's network address.
> 
> 
> Diffs
> -----
> 
>   proton-c/bindings/cpp/src/container_impl.cpp a221f45 
>   proton-c/bindings/cpp/src/reactor.hpp 48d9ea1 
>   proton-c/bindings/cpp/src/reactor.cpp 9507d2b 
>   proton-c/bindings/python/proton/reactor.py 1631c35 
>   proton-c/include/proton/connection.h da20f94 
>   proton-c/include/proton/reactor.h be642a9 
>   proton-c/src/posix/io.c 3226594 
>   proton-c/src/reactor/acceptor.c 8f0e99b 
>   proton-c/src/reactor/connection.c 336d1f1 
>   proton-c/src/reactor/reactor.h f996dca 
>   proton-c/src/tests/reactor.c 9564569 
>   proton-c/src/windows/io.c 7ff928d 
>   proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java a3307d2 
>   
> proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/AcceptorImpl.java 
> fb2f892 
>   proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java 
> 5a32824 
>   proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java 
> d13cfbe 
>   tests/python/proton_tests/reactor.py 6ee107d 
> 
> Diff: https://reviews.apache.org/r/47243/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>

Reply via email to