You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kenneth Giusti <kg...@apache.org> on 2016/05/11 16:52:40 UTC

Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

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

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


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Robbie Gemmell <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/#review132961
-----------------------------------------------------------



Seems good overall, a couple questions/niggles that aren't necessarily worth bothering about.


proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java (line 128)
<https://reviews.apache.org/r/47243/#comment197242>

    This wont do anything unless assertions are explicitly enabled (which often they arent), so it will typically carry on and probably go bang below when trying to use the non-existant host. The C changes protect that explicitly so perhaps the Java changes probably should too. That said, its probably unlikely, and its going to fail either way, so isnt necessarily of much concern.



proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java (lines 456 - 465)
<https://reviews.apache.org/r/47243/#comment197243>

    We would typically throw an exception (e.g IllegalStateException) in Java rather than silently not do what was asked. Any reason not to here?


- Robbie Gemmell


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


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/#review132980
-----------------------------------------------------------




proton-c/src/reactor/acceptor.c (line 49)
<https://reviews.apache.org/r/47243/#comment197272>

    This probably fails with IPv6 addresses. We might have
        [::1], or [2001:db8:85a3:8d3:1319:8a2e:370:7348]
    
    What forms of IPv6 does Proton accept?


- Chug Rolke


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


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/#review133175
-----------------------------------------------------------


Ship it!




This time I  mean it.

- Alan Conway


On May 13, 2016, 5:59 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47243/
> -----------------------------------------------------------
> 
> (Updated May 13, 2016, 5:59 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/CMakeLists.txt 2aec4e2 
>   proton-c/bindings/cpp/include/proton/connection_options.hpp d736600 
>   proton-c/bindings/cpp/src/connection_options.cpp fed645c 
>   proton-c/bindings/cpp/src/connector.cpp dbf74be 
>   proton-c/bindings/cpp/src/container_impl.cpp a221f45 
>   proton-c/bindings/cpp/src/container_test.cpp PRE-CREATION 
>   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/engine/impl/ConnectionImpl.java b708d83 
>   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 
>   proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java 4e92cd9 
>   tests/python/proton_tests/reactor.py 6ee107d 
> 
> Diff: https://reviews.apache.org/r/47243/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Robbie Gemmell <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/#review133179
-----------------------------------------------------------



Looks good other than a couple niggles.


proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java (line 310)
<https://reviews.apache.org/r/47243/#comment197467>

    Try "this.port = port" rather than the trailing _. (or leading _ is used among much of the java code)



proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java (lines 356 - 357)
<https://reviews.apache.org/r/47243/#comment197468>

    We use camelCase for Java bits



tests/python/proton_tests/reactor.py (lines 612 - 614)
<https://reviews.apache.org/r/47243/#comment197469>

    Perhaps a reason thats shorter/less alarming given it shows up on the console?
    
    "Does not apply for proton-j" with a comment after to elaborate?


- Robbie Gemmell


On May 13, 2016, 7:21 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47243/
> -----------------------------------------------------------
> 
> (Updated May 13, 2016, 7:21 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/CMakeLists.txt 2aec4e2 
>   proton-c/bindings/cpp/include/proton/connection_options.hpp d736600 
>   proton-c/bindings/cpp/src/connection_options.cpp fed645c 
>   proton-c/bindings/cpp/src/connector.cpp dbf74be 
>   proton-c/bindings/cpp/src/container_impl.cpp a221f45 
>   proton-c/bindings/cpp/src/container_test.cpp PRE-CREATION 
>   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/engine/impl/ConnectionImpl.java b708d83 
>   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 
>   proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java 4e92cd9 
>   tests/python/proton_tests/reactor.py 6ee107d 
> 
> Diff: https://reviews.apache.org/r/47243/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/
-----------------------------------------------------------

(Updated May 13, 2016, 7:21 p.m.)


Review request for qpid, Alan Conway, Chug Rolke, Cliff Jansen, Justin Ross, and Robbie Gemmell.


Changes
-------

camelCase for Java


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 (updated)
-----

  proton-c/bindings/cpp/CMakeLists.txt 2aec4e2 
  proton-c/bindings/cpp/include/proton/connection_options.hpp d736600 
  proton-c/bindings/cpp/src/connection_options.cpp fed645c 
  proton-c/bindings/cpp/src/connector.cpp dbf74be 
  proton-c/bindings/cpp/src/container_impl.cpp a221f45 
  proton-c/bindings/cpp/src/container_test.cpp PRE-CREATION 
  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/engine/impl/ConnectionImpl.java b708d83 
  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 
  proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java 4e92cd9 
  tests/python/proton_tests/reactor.py 6ee107d 

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


Testing
-------

New unit tests added.


Thanks,

Kenneth Giusti


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Justin Ross <jr...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/#review133173
-----------------------------------------------------------


Ship it!




Ship It!

- Justin Ross


On May 13, 2016, 5:59 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47243/
> -----------------------------------------------------------
> 
> (Updated May 13, 2016, 5:59 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/CMakeLists.txt 2aec4e2 
>   proton-c/bindings/cpp/include/proton/connection_options.hpp d736600 
>   proton-c/bindings/cpp/src/connection_options.cpp fed645c 
>   proton-c/bindings/cpp/src/connector.cpp dbf74be 
>   proton-c/bindings/cpp/src/container_impl.cpp a221f45 
>   proton-c/bindings/cpp/src/container_test.cpp PRE-CREATION 
>   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/engine/impl/ConnectionImpl.java b708d83 
>   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 
>   proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java 4e92cd9 
>   tests/python/proton_tests/reactor.py 6ee107d 
> 
> Diff: https://reviews.apache.org/r/47243/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/
-----------------------------------------------------------

(Updated May 13, 2016, 5:59 p.m.)


Review request for qpid, Alan Conway, Chug Rolke, Cliff Jansen, Justin Ross, and Robbie Gemmell.


Changes
-------

Last update: addresses the last set of comments, and updates the Java Reactor to follow the policy for setting the virtual hostname used by Python, C, and C++.


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 (updated)
-----

  proton-c/bindings/cpp/CMakeLists.txt 2aec4e2 
  proton-c/bindings/cpp/include/proton/connection_options.hpp d736600 
  proton-c/bindings/cpp/src/connection_options.cpp fed645c 
  proton-c/bindings/cpp/src/connector.cpp dbf74be 
  proton-c/bindings/cpp/src/container_impl.cpp a221f45 
  proton-c/bindings/cpp/src/container_test.cpp PRE-CREATION 
  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/engine/impl/ConnectionImpl.java b708d83 
  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 
  proton-j/src/test/java/org/apache/qpid/proton/reactor/ReactorTest.java 4e92cd9 
  tests/python/proton_tests/reactor.py 6ee107d 

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


Testing
-------

New unit tests added.


Thanks,

Kenneth Giusti


Re: Review Request 47243: PROTON-1133: decouple the virtual host from the network address used by reactor

Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47243/
-----------------------------------------------------------

(Updated May 13, 2016, 2:16 a.m.)


Review request for qpid, Alan Conway, Chug Rolke, Cliff Jansen, Justin Ross, and Robbie Gemmell.


Changes
-------

This update changes the C++ and Python Reactor Container to use the host portion of the connection's URL as a default for the Open performative if no virtual host is explicitly given.

Java doesn't seem to offer a Reactor Container class, so no changes as far as Java is concerned.


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 (updated)
-----

  proton-c/bindings/cpp/CMakeLists.txt 2aec4e2 
  proton-c/bindings/cpp/include/proton/connection_options.hpp d736600 
  proton-c/bindings/cpp/src/connection_options.cpp fed645c 
  proton-c/bindings/cpp/src/connector.cpp dbf74be 
  proton-c/bindings/cpp/src/container_impl.cpp a221f45 
  proton-c/bindings/cpp/src/container_test.cpp PRE-CREATION 
  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