Re: Review Request 67661: propagate link-detach as expiry policy for link route if none was explicitly selected by client

2018-06-21 Thread Kenneth Giusti 

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


Ship it!




Ship It!

- Kenneth Giusti


On June 20, 2018, 5 p.m., Gordon Sim wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67661/
> ---
> 
> (Updated June 20, 2018, 5 p.m.)
> 
> 
> Review request for qpid, Alan Conway and Ted Ross.
> 
> 
> Bugs: DISPATCH-1020
> https://issues.apache.org/jira/browse/DISPATCH-1020
> 
> 
> Repository: qpid-dispatch
> 
> 
> Description
> ---
> 
> The default expiry policy for a terminus is session-end. Now that link routed 
> links share a session by default, this is likely not what is wanted (terminus 
> state on a broker would have to be kept until the session ended, which is 
> when the connection between broker and router ends). This change propagates 
> an explicit link-detach policy where none is explicitly requested by the 
> client. If the client does request an explicit policy, that is propagated 
> without alteration.
> 
> Note this requires a change to proton PROTON-1866, 
> https://reviews.apache.org/r/67659/
> 
> 
> Diffs
> -
> 
>   include/qpid/dispatch/router_core.h 8f144b0 
>   src/router_core/connections.c 5fdc3bf 
>   src/router_core/terminus.c 4c0e0a3 
>   tests/system_tests_link_routes.py 57e6d41 
> 
> 
> Diff: https://reviews.apache.org/r/67661/diff/2/
> 
> 
> Testing
> ---
> 
> Automated tests added, all existing tests pass.
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>

Re: Review Request 67661: propagate link-detach as expiry policy for link route if none was explicitly selected by client

2018-06-20 Thread Gordon Sim 


> On June 20, 2018, 1:12 p.m., Kenneth Giusti wrote:
> > include/qpid/dispatch/router_core.h
> > Lines 350 (patched)
> > 
> >
> > nit:
> > 
> > I hate to be "that guy" but I find the the name of the function 
> > ambiguous. I'd expect a "check" to either succeed or fail.  Perhaps 
> > something along the lines of
> > 
> > qdr_terminus_set_default_expiry(*term)
> > 
> > or
> > 
> > qdr_terminus_set_default_expiry(*term, default)
> > 
> > would be more flexible?
> 
> Gordon Sim wrote:
> I agree it is not a great name... how about 
> qdr_terminus_set_link_expire_if_unset? Is that too long?

I've updated the name now.


- Gordon


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


On June 20, 2018, 5 p.m., Gordon Sim wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67661/
> ---
> 
> (Updated June 20, 2018, 5 p.m.)
> 
> 
> Review request for qpid, Alan Conway and Ted Ross.
> 
> 
> Bugs: DISPATCH-1020
> https://issues.apache.org/jira/browse/DISPATCH-1020
> 
> 
> Repository: qpid-dispatch
> 
> 
> Description
> ---
> 
> The default expiry policy for a terminus is session-end. Now that link routed 
> links share a session by default, this is likely not what is wanted (terminus 
> state on a broker would have to be kept until the session ended, which is 
> when the connection between broker and router ends). This change propagates 
> an explicit link-detach policy where none is explicitly requested by the 
> client. If the client does request an explicit policy, that is propagated 
> without alteration.
> 
> Note this requires a change to proton PROTON-1866, 
> https://reviews.apache.org/r/67659/
> 
> 
> Diffs
> -
> 
>   include/qpid/dispatch/router_core.h 8f144b0 
>   src/router_core/connections.c 5fdc3bf 
>   src/router_core/terminus.c 4c0e0a3 
>   tests/system_tests_link_routes.py 57e6d41 
> 
> 
> Diff: https://reviews.apache.org/r/67661/diff/2/
> 
> 
> Testing
> ---
> 
> Automated tests added, all existing tests pass.
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>

Re: Review Request 67661: propagate link-detach as expiry policy for link route if none was explicitly selected by client

2018-06-20 Thread Gordon Sim 

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

(Updated June 20, 2018, 5 p.m.)


Review request for qpid, Alan Conway and Ted Ross.


Changes
---

Changed method name to hopefully be a bit more intuitive


Bugs: DISPATCH-1020
https://issues.apache.org/jira/browse/DISPATCH-1020


Repository: qpid-dispatch


Description
---

The default expiry policy for a terminus is session-end. Now that link routed 
links share a session by default, this is likely not what is wanted (terminus 
state on a broker would have to be kept until the session ended, which is when 
the connection between broker and router ends). This change propagates an 
explicit link-detach policy where none is explicitly requested by the client. 
If the client does request an explicit policy, that is propagated without 
alteration.

Note this requires a change to proton PROTON-1866, 
https://reviews.apache.org/r/67659/


Diffs (updated)
-

  include/qpid/dispatch/router_core.h 8f144b0 
  src/router_core/connections.c 5fdc3bf 
  src/router_core/terminus.c 4c0e0a3 
  tests/system_tests_link_routes.py 57e6d41 


Diff: https://reviews.apache.org/r/67661/diff/2/

Changes: https://reviews.apache.org/r/67661/diff/1-2/


Testing
---

Automated tests added, all existing tests pass.


Thanks,

Gordon Sim

Re: Review Request 67661: propagate link-detach as expiry policy for link route if none was explicitly selected by client

2018-06-20 Thread Ganesh Murthy 

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


Ship it!




Ship It!

- Ganesh Murthy


On June 19, 2018, 10:25 p.m., Gordon Sim wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67661/
> ---
> 
> (Updated June 19, 2018, 10:25 p.m.)
> 
> 
> Review request for qpid, Alan Conway and Ted Ross.
> 
> 
> Bugs: DISPATCH-1020
> https://issues.apache.org/jira/browse/DISPATCH-1020
> 
> 
> Repository: qpid-dispatch
> 
> 
> Description
> ---
> 
> The default expiry policy for a terminus is session-end. Now that link routed 
> links share a session by default, this is likely not what is wanted (terminus 
> state on a broker would have to be kept until the session ended, which is 
> when the connection between broker and router ends). This change propagates 
> an explicit link-detach policy where none is explicitly requested by the 
> client. If the client does request an explicit policy, that is propagated 
> without alteration.
> 
> Note this requires a change to proton PROTON-1866, 
> https://reviews.apache.org/r/67659/
> 
> 
> Diffs
> -
> 
>   include/qpid/dispatch/router_core.h 8f144b0 
>   src/router_core/connections.c 5fdc3bf 
>   src/router_core/terminus.c 4c0e0a3 
>   tests/system_tests_link_routes.py 57e6d41 
> 
> 
> Diff: https://reviews.apache.org/r/67661/diff/1/
> 
> 
> Testing
> ---
> 
> Automated tests added, all existing tests pass.
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>

Re: Review Request 67661: propagate link-detach as expiry policy for link route if none was explicitly selected by client

2018-06-20 Thread Gordon Sim 


> On June 20, 2018, 1:12 p.m., Kenneth Giusti wrote:
> > include/qpid/dispatch/router_core.h
> > Lines 350 (patched)
> > 
> >
> > nit:
> > 
> > I hate to be "that guy" but I find the the name of the function 
> > ambiguous. I'd expect a "check" to either succeed or fail.  Perhaps 
> > something along the lines of
> > 
> > qdr_terminus_set_default_expiry(*term)
> > 
> > or
> > 
> > qdr_terminus_set_default_expiry(*term, default)
> > 
> > would be more flexible?

I agree it is not a great name... how about 
qdr_terminus_set_link_expire_if_unset? Is that too long?


- Gordon


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


On June 19, 2018, 10:25 p.m., Gordon Sim wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67661/
> ---
> 
> (Updated June 19, 2018, 10:25 p.m.)
> 
> 
> Review request for qpid, Alan Conway and Ted Ross.
> 
> 
> Bugs: DISPATCH-1020
> https://issues.apache.org/jira/browse/DISPATCH-1020
> 
> 
> Repository: qpid-dispatch
> 
> 
> Description
> ---
> 
> The default expiry policy for a terminus is session-end. Now that link routed 
> links share a session by default, this is likely not what is wanted (terminus 
> state on a broker would have to be kept until the session ended, which is 
> when the connection between broker and router ends). This change propagates 
> an explicit link-detach policy where none is explicitly requested by the 
> client. If the client does request an explicit policy, that is propagated 
> without alteration.
> 
> Note this requires a change to proton PROTON-1866, 
> https://reviews.apache.org/r/67659/
> 
> 
> Diffs
> -
> 
>   include/qpid/dispatch/router_core.h 8f144b0 
>   src/router_core/connections.c 5fdc3bf 
>   src/router_core/terminus.c 4c0e0a3 
>   tests/system_tests_link_routes.py 57e6d41 
> 
> 
> Diff: https://reviews.apache.org/r/67661/diff/1/
> 
> 
> Testing
> ---
> 
> Automated tests added, all existing tests pass.
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>

Re: Review Request 67661: propagate link-detach as expiry policy for link route if none was explicitly selected by client

2018-06-20 Thread Kenneth Giusti 

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



For future reference: if you submit dispatch patches via a Pull Request against 
the apache mirror https://github.com/apache/qpid-dispatch instead of 
reviewboard, travis will run a unit test coverage report which will show how 
well the patch is covered.

thanks


include/qpid/dispatch/router_core.h
Lines 350 (patched)


nit:

I hate to be "that guy" but I find the the name of the function ambiguous. 
I'd expect a "check" to either succeed or fail.  Perhaps something along the 
lines of

qdr_terminus_set_default_expiry(*term)

or

qdr_terminus_set_default_expiry(*term, default)

would be more flexible?


- Kenneth Giusti


On June 19, 2018, 10:25 p.m., Gordon Sim wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67661/
> ---
> 
> (Updated June 19, 2018, 10:25 p.m.)
> 
> 
> Review request for qpid, Alan Conway and Ted Ross.
> 
> 
> Bugs: DISPATCH-1020
> https://issues.apache.org/jira/browse/DISPATCH-1020
> 
> 
> Repository: qpid-dispatch
> 
> 
> Description
> ---
> 
> The default expiry policy for a terminus is session-end. Now that link routed 
> links share a session by default, this is likely not what is wanted (terminus 
> state on a broker would have to be kept until the session ended, which is 
> when the connection between broker and router ends). This change propagates 
> an explicit link-detach policy where none is explicitly requested by the 
> client. If the client does request an explicit policy, that is propagated 
> without alteration.
> 
> Note this requires a change to proton PROTON-1866, 
> https://reviews.apache.org/r/67659/
> 
> 
> Diffs
> -
> 
>   include/qpid/dispatch/router_core.h 8f144b0 
>   src/router_core/connections.c 5fdc3bf 
>   src/router_core/terminus.c 4c0e0a3 
>   tests/system_tests_link_routes.py 57e6d41 
> 
> 
> Diff: https://reviews.apache.org/r/67661/diff/1/
> 
> 
> Testing
> ---
> 
> Automated tests added, all existing tests pass.
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>

Review Request 67661: propagate link-detach as expiry policy for link route if none was explicitly selected by client

2018-06-19 Thread Gordon Sim 

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

Review request for qpid, Alan Conway and Ted Ross.


Bugs: DISPATCH-1020
https://issues.apache.org/jira/browse/DISPATCH-1020


Repository: qpid-dispatch


Description
---

The default expiry policy for a terminus is session-end. Now that link routed 
links share a session by default, this is likely not what is wanted (terminus 
state on a broker would have to be kept until the session ended, which is when 
the connection between broker and router ends). This change propagates an 
explicit link-detach policy where none is explicitly requested by the client. 
If the client does request an explicit policy, that is propagated without 
alteration.

Note this requires a change to proton PROTON-1866, 
https://reviews.apache.org/r/67659/


Diffs
-

  include/qpid/dispatch/router_core.h 8f144b0 
  src/router_core/connections.c 5fdc3bf 
  src/router_core/terminus.c 4c0e0a3 
  tests/system_tests_link_routes.py 57e6d41 


Diff: https://reviews.apache.org/r/67661/diff/1/


Testing
---

Automated tests added, all existing tests pass.


Thanks,

Gordon Sim