You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@qpid.apache.org by Cliff Jansen <cl...@gmail.com> on 2016/03/16 21:42:39 UTC

Review Request 44927: C++ api changes: handler options become link or connection options

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

Review request for qpid and Justin Ross.


Bugs: PROTON-1138
    https://issues.apache.org/jira/browse/PROTON-1138


Repository: qpid-proton-git


Description
-------

handler options -> link/connection options
prefetch -> credit_window
stop use of pn_flowcontroller (manage credit directly)
override -> update (C++ semi-reserved word) and private
new link_context to store behaviors
have container link_options apply to remote opened links


Diffs
-----

  proton-c/bindings/cpp/include/proton/connection_options.hpp 71e12f1 
  proton-c/bindings/cpp/include/proton/container.hpp 0af1963 
  proton-c/bindings/cpp/include/proton/handler.hpp 3bc0023 
  proton-c/bindings/cpp/include/proton/link.hpp e626398 
  proton-c/bindings/cpp/include/proton/link_options.hpp 2eb145d 
  proton-c/bindings/cpp/src/connection_options.cpp a5ee99e 
  proton-c/bindings/cpp/src/container.cpp fc34f39 
  proton-c/bindings/cpp/src/container_impl.hpp d1b476f 
  proton-c/bindings/cpp/src/container_impl.cpp 55547ca 
  proton-c/bindings/cpp/src/contexts.hpp b4fcdba 
  proton-c/bindings/cpp/src/contexts.cpp c2b76f6 
  proton-c/bindings/cpp/src/handler.cpp 43febf5 
  proton-c/bindings/cpp/src/link_options.cpp 534a6b6 
  proton-c/bindings/cpp/src/messaging_adapter.hpp a5b364a 
  proton-c/bindings/cpp/src/messaging_adapter.cpp 4265644 
  proton-c/bindings/cpp/src/proton_event.hpp 8dd2f8f 
  tests/tools/apps/cpp/reactor_send.cpp 3ba26e8 

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


Testing
-------

linux build and ctest


Thanks,

Cliff Jansen

Re: Review Request 44927: C++ api changes: handler options become link or connection options

Posted by Justin Ross <jr...@apache.org>.

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


Ship it!




After discussion, we agreed peer_close_on_error should be removed for now.  Apart from that, this looks good to me.

- Justin Ross


On March 16, 2016, 8:42 p.m., Cliff Jansen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44927/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 8:42 p.m.)
> 
> 
> Review request for qpid and Justin Ross.
> 
> 
> Bugs: PROTON-1138
>     https://issues.apache.org/jira/browse/PROTON-1138
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> handler options -> link/connection options
> prefetch -> credit_window
> stop use of pn_flowcontroller (manage credit directly)
> override -> update (C++ semi-reserved word) and private
> new link_context to store behaviors
> have container link_options apply to remote opened links
> 
> 
> Diffs
> -----
> 
>   proton-c/bindings/cpp/include/proton/connection_options.hpp 71e12f1 
>   proton-c/bindings/cpp/include/proton/container.hpp 0af1963 
>   proton-c/bindings/cpp/include/proton/handler.hpp 3bc0023 
>   proton-c/bindings/cpp/include/proton/link.hpp e626398 
>   proton-c/bindings/cpp/include/proton/link_options.hpp 2eb145d 
>   proton-c/bindings/cpp/src/connection_options.cpp a5ee99e 
>   proton-c/bindings/cpp/src/container.cpp fc34f39 
>   proton-c/bindings/cpp/src/container_impl.hpp d1b476f 
>   proton-c/bindings/cpp/src/container_impl.cpp 55547ca 
>   proton-c/bindings/cpp/src/contexts.hpp b4fcdba 
>   proton-c/bindings/cpp/src/contexts.cpp c2b76f6 
>   proton-c/bindings/cpp/src/handler.cpp 43febf5 
>   proton-c/bindings/cpp/src/link_options.cpp 534a6b6 
>   proton-c/bindings/cpp/src/messaging_adapter.hpp a5b364a 
>   proton-c/bindings/cpp/src/messaging_adapter.cpp 4265644 
>   proton-c/bindings/cpp/src/proton_event.hpp 8dd2f8f 
>   tests/tools/apps/cpp/reactor_send.cpp 3ba26e8 
> 
> Diff: https://reviews.apache.org/r/44927/diff/
> 
> 
> Testing
> -------
> 
> linux build and ctest
> 
> 
> Thanks,
> 
> Cliff Jansen
> 
>

Re: Review Request 44927: C++ api changes: handler options become link or connection options

Posted by Justin Ross <jr...@apache.org>.

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




proton-c/bindings/cpp/include/proton/connection_options.hpp (line 98)
<https://reviews.apache.org/r/44927/#comment186627>

    What does this do?  It's not documented in the Python API.  Based on your implementation, it appears to fire on_$endpoint_error if we are notified of $endpoint close.
    
    When do you want that?  To simplify shutdown handling?  Is that still important given that we now always fire close as well as error for error cases?
    
    (I know you are simply matching what Python has, but it of course raises the point that we need to sort out its utility.)



proton-c/bindings/cpp/include/proton/link_options.hpp (line 165)
<https://reviews.apache.org/r/44927/#comment186628>

    We discussed using message_window, but I like credit_window as much or better because of the name-based link with credit and add_credit.


- Justin Ross


On March 16, 2016, 8:42 p.m., Cliff Jansen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44927/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 8:42 p.m.)
> 
> 
> Review request for qpid and Justin Ross.
> 
> 
> Bugs: PROTON-1138
>     https://issues.apache.org/jira/browse/PROTON-1138
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> handler options -> link/connection options
> prefetch -> credit_window
> stop use of pn_flowcontroller (manage credit directly)
> override -> update (C++ semi-reserved word) and private
> new link_context to store behaviors
> have container link_options apply to remote opened links
> 
> 
> Diffs
> -----
> 
>   proton-c/bindings/cpp/include/proton/connection_options.hpp 71e12f1 
>   proton-c/bindings/cpp/include/proton/container.hpp 0af1963 
>   proton-c/bindings/cpp/include/proton/handler.hpp 3bc0023 
>   proton-c/bindings/cpp/include/proton/link.hpp e626398 
>   proton-c/bindings/cpp/include/proton/link_options.hpp 2eb145d 
>   proton-c/bindings/cpp/src/connection_options.cpp a5ee99e 
>   proton-c/bindings/cpp/src/container.cpp fc34f39 
>   proton-c/bindings/cpp/src/container_impl.hpp d1b476f 
>   proton-c/bindings/cpp/src/container_impl.cpp 55547ca 
>   proton-c/bindings/cpp/src/contexts.hpp b4fcdba 
>   proton-c/bindings/cpp/src/contexts.cpp c2b76f6 
>   proton-c/bindings/cpp/src/handler.cpp 43febf5 
>   proton-c/bindings/cpp/src/link_options.cpp 534a6b6 
>   proton-c/bindings/cpp/src/messaging_adapter.hpp a5b364a 
>   proton-c/bindings/cpp/src/messaging_adapter.cpp 4265644 
>   proton-c/bindings/cpp/src/proton_event.hpp 8dd2f8f 
>   tests/tools/apps/cpp/reactor_send.cpp 3ba26e8 
> 
> Diff: https://reviews.apache.org/r/44927/diff/
> 
> 
> Testing
> -------
> 
> linux build and ctest
> 
> 
> Thanks,
> 
> Cliff Jansen
> 
>