You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by michael goulish <mg...@redhat.com> on 2015/06/23 20:49:02 UTC
Review Request 35798: PROTON-919: make the C impl behave same as Java
wrt channel_max error
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35798/
-----------------------------------------------------------
Review request for qpid, Andrew Stitcher and Kenneth Giusti.
Repository: qpid-proton-git
Description
-------
Alter C impl to return error code if we attempt to change local channel_max after OPEN frame has been sent. Also alter python binding to detect that error code, and throw exception. This way, the C and Java versions of one of the test of channel_max functionality have the same behavior.
Diffs
-----
proton-c/include/proton/error.h c6c7d2e
proton-c/include/proton/transport.h 483f5a9
proton-c/src/transport/transport.c ff80e21
tests/python/proton_tests/engine.py 258665d
Diff: https://reviews.apache.org/r/35798/diff/
Testing
-------
ctest -VV --- C and Java
Thanks,
michael goulish
Re: Review Request 35798: PROTON-919: make the C impl behave same as
Java wrt channel_max error
Posted by michael goulish <mg...@redhat.com>.
> On June 23, 2015, 7:48 p.m., Kenneth Giusti wrote:
> > I think you may also have to add code into the python binding for the 'channel_max' property of the transport that checks for the error and then throws the exception.
Yes, sorry -- I had that in there but it somehow weaseled out of this diff
It looks like this:
- pn_transport_set_channel_max(self._impl, value)
+ if(pn_transport_set_channel_max(self._impl, value)):
+ raise(SessionException("Too late to change channel max."))
- michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35798/#review89040
-----------------------------------------------------------
On June 23, 2015, 6:49 p.m., michael goulish wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35798/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 6:49 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Kenneth Giusti.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Alter C impl to return error code if we attempt to change local channel_max after OPEN frame has been sent. Also alter python binding to detect that error code, and throw exception. This way, the C and Java versions of one of the test of channel_max functionality have the same behavior.
>
>
> Diffs
> -----
>
> proton-c/include/proton/error.h c6c7d2e
> proton-c/include/proton/transport.h 483f5a9
> proton-c/src/transport/transport.c ff80e21
> tests/python/proton_tests/engine.py 258665d
>
> Diff: https://reviews.apache.org/r/35798/diff/
>
>
> Testing
> -------
>
> ctest -VV --- C and Java
>
>
> Thanks,
>
> michael goulish
>
>
Re: Review Request 35798: PROTON-919: make the C impl behave same as
Java wrt channel_max error
Posted by Kenneth Giusti <kg...@apache.org>.
> On June 23, 2015, 7:48 p.m., Kenneth Giusti wrote:
> > I think you may also have to add code into the python binding for the 'channel_max' property of the transport that checks for the error and then throws the exception.
>
> michael goulish wrote:
> Yes, sorry -- I had that in there but it somehow weaseled out of this diff
> It looks like this:
>
> - pn_transport_set_channel_max(self._impl, value)
> + if(pn_transport_set_channel_max(self._impl, value)):
> + raise(SessionException("Too late to change channel max."))
couple of changes to be more 'python-ic' (ugh) - lose the extraneous parens, it screams "C programmer" :D
if pn_transport_set_channel_max(self._impl, value):
raise SessionException("Too late to change channel max.")
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35798/#review89040
-----------------------------------------------------------
On June 23, 2015, 6:49 p.m., michael goulish wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35798/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 6:49 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Kenneth Giusti.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Alter C impl to return error code if we attempt to change local channel_max after OPEN frame has been sent. Also alter python binding to detect that error code, and throw exception. This way, the C and Java versions of one of the test of channel_max functionality have the same behavior.
>
>
> Diffs
> -----
>
> proton-c/include/proton/error.h c6c7d2e
> proton-c/include/proton/transport.h 483f5a9
> proton-c/src/transport/transport.c ff80e21
> tests/python/proton_tests/engine.py 258665d
>
> Diff: https://reviews.apache.org/r/35798/diff/
>
>
> Testing
> -------
>
> ctest -VV --- C and Java
>
>
> Thanks,
>
> michael goulish
>
>
Re: Review Request 35798: PROTON-919: make the C impl behave same as
Java wrt channel_max error
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35798/#review89040
-----------------------------------------------------------
I think you may also have to add code into the python binding for the 'channel_max' property of the transport that checks for the error and then throws the exception.
proton-c/include/proton/transport.h (line 348)
<https://reviews.apache.org/r/35798/#comment141644>
PN_TIMEOUT isn't a good choice here IMHO. If that gets bubbled up to an application, it implies a different type of error. Maybe PN_STATE_ERR since you can't change it once the application opens the connection (state change)?
tests/python/proton_tests/engine.py (line 270)
<https://reviews.apache.org/r/35798/#comment141646>
Since you're expecting an exception, add a
assert False, "expected failure"
right after line 269 and before the except
- Kenneth Giusti
On June 23, 2015, 6:49 p.m., michael goulish wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35798/
> -----------------------------------------------------------
>
> (Updated June 23, 2015, 6:49 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Kenneth Giusti.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Alter C impl to return error code if we attempt to change local channel_max after OPEN frame has been sent. Also alter python binding to detect that error code, and throw exception. This way, the C and Java versions of one of the test of channel_max functionality have the same behavior.
>
>
> Diffs
> -----
>
> proton-c/include/proton/error.h c6c7d2e
> proton-c/include/proton/transport.h 483f5a9
> proton-c/src/transport/transport.c ff80e21
> tests/python/proton_tests/engine.py 258665d
>
> Diff: https://reviews.apache.org/r/35798/diff/
>
>
> Testing
> -------
>
> ctest -VV --- C and Java
>
>
> Thanks,
>
> michael goulish
>
>
Re: Review Request 35798: PROTON-919: make the C impl behave same as
Java wrt channel_max error
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35798/#review92087
-----------------------------------------------------------
proton-c/include/proton/transport.h (line 348)
<https://reviews.apache.org/r/35798/#comment146027>
Reopening, because the doc still says PN_TIMEOUT and Ken is correct. It is the wrong error.
proton-c/src/transport/transport.c (line 2662)
<https://reviews.apache.org/r/35798/#comment146028>
Wrong error return as above.
- Andrew Stitcher
On July 17, 2015, 2:39 p.m., michael goulish wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35798/
> -----------------------------------------------------------
>
> (Updated July 17, 2015, 2:39 p.m.)
>
>
> Review request for qpid, Andrew Stitcher and Kenneth Giusti.
>
>
> Repository: qpid-proton-git
>
>
> Description
> -------
>
> Alter C impl to return error code if we attempt to change local channel_max after OPEN frame has been sent. Also alter python binding to detect that error code, and throw exception. This way, the C and Java versions of one of the test of channel_max functionality have the same behavior.
>
>
> Diffs
> -----
>
> proton-c/include/proton/error.h c6c7d2e
> proton-c/include/proton/transport.h 483f5a9
> proton-c/src/transport/transport.c ff80e21
> tests/python/proton_tests/engine.py 258665d
>
> Diff: https://reviews.apache.org/r/35798/diff/
>
>
> Testing
> -------
>
> ctest -VV --- C and Java
>
> Please note: This Jira changes the public interface in that it adds #define PN_OK 0 to the list of possible error return values in error.h
>
>
> Thanks,
>
> michael goulish
>
>
Re: Review Request 35798: PROTON-919: make the C impl behave same as
Java wrt channel_max error
Posted by michael goulish <mg...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35798/
-----------------------------------------------------------
(Updated July 17, 2015, 4:10 p.m.)
Review request for qpid, Andrew Stitcher and Kenneth Giusti.
Changes
-------
Last time I apparently uploaded a diff here before I finished making changes in the code. This diff represents what I actually chacked in.
Repository: qpid-proton-git
Description
-------
Alter C impl to return error code if we attempt to change local channel_max after OPEN frame has been sent. Also alter python binding to detect that error code, and throw exception. This way, the C and Java versions of one of the test of channel_max functionality have the same behavior.
Diffs (updated)
-----
proton-c/bindings/python/proton/__init__.py d5dcceb
proton-c/include/proton/error.h 2ed2f31
proton-c/include/proton/transport.h 483f5a9
proton-c/src/transport/transport.c 7bce3b5
tests/python/proton_tests/engine.py c18683f
Diff: https://reviews.apache.org/r/35798/diff/
Testing
-------
ctest -VV --- C and Java
Please note: This Jira changes the public interface in that it adds #define PN_OK 0 to the list of possible error return values in error.h
Thanks,
michael goulish
Re: Review Request 35798: PROTON-919: make the C impl behave same as
Java wrt channel_max error
Posted by michael goulish <mg...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35798/
-----------------------------------------------------------
(Updated July 17, 2015, 2:39 p.m.)
Review request for qpid, Andrew Stitcher and Kenneth Giusti.
Repository: qpid-proton-git
Description
-------
Alter C impl to return error code if we attempt to change local channel_max after OPEN frame has been sent. Also alter python binding to detect that error code, and throw exception. This way, the C and Java versions of one of the test of channel_max functionality have the same behavior.
Diffs
-----
proton-c/include/proton/error.h c6c7d2e
proton-c/include/proton/transport.h 483f5a9
proton-c/src/transport/transport.c ff80e21
tests/python/proton_tests/engine.py 258665d
Diff: https://reviews.apache.org/r/35798/diff/
Testing (updated)
-------
ctest -VV --- C and Java
Please note: This Jira changes the public interface in that it adds #define PN_OK 0 to the list of possible error return values in error.h
Thanks,
michael goulish