qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "michael goulish" <mgoul...@redhat.com>
Subject Re: Review Request 36992: PROTON-886 and PROTON-930 -- handle_max and AMQP numeric default constants
Date Wed, 05 Aug 2015 09:52:42 GMT


> On July 31, 2015, 8:27 p.m., Kenneth Giusti wrote:
> > proton-c/src/engine/engine.c, line 2230
> > <https://reviews.apache.org/r/36992/diff/1/?file=1026182#file1026182line2230>
> >
> >     use pn_min() in utils.h

Wow we ... have a min macro.  I...  That's....    OK!   Absolutely.


> On July 31, 2015, 8:27 p.m., Kenneth Giusti wrote:
> > proton-c/src/protocol.h.py, line 39
> > <https://reviews.apache.org/r/36992/diff/1/?file=1026183#file1026183line39>
> >
> >     Yay! another chance to make you a Python programmer!
> >     
> >     You don't need the standalone int conversion statement to test the type (it's
not 'natural' python):
> >     
> >     int(default)
> >     
> >     Use '%d' as the format flag (instead of %s), and pass it the int(default) in
the print() statement.

Bah.  What's 'natural'?  standalone makes the intent more clear.  Now I'm putting a comment
there.  Next they'll be telling me how to use whitespace.  Oh, wait...


- michael


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


On Aug. 5, 2015, 9:52 a.m., michael goulish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36992/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 9:52 a.m.)
> 
> 
> Review request for qpid, Kenneth Giusti and Ted Ross.
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> PROTON-886 and PROTON-930 -- handle_max and AMQP numeric default constants.  
> 
>     Note:
>     {
>       I am skipping the java implementation for now,
>       because I would like to get the C version out of
>       my life.  (And this diff is already quite large
>       enough, I think.)
> 
>       I will write a separate JIRA for the java version
>       and make it refer to this one.  I will work on it
>       as a background task.
>     }
> 
>     This diff adds two pieces of functionality:
> 
>       1. extract numeric default valuies from AMQP
>          xml files and store them as defined constants
>          in protocol.h  (PROTON_930)
> 
>       2. add code to enforce AMQP 1.0 spec mandated
>          behavior with respect to handle-max values
>          -- i.e. negotiated limits on number of links --
>          per session.  (PROTON-886)
> 
>     These two changes are combined into one checkin
>     because of some earlier feedback I got suggesting
>     that I not check in PROTON-930 until I had some
>     code that could actually use the constants that it
>     creates.
> 
>     The code that is generated by the changes in
>     proton-c/src/protocol.h.py show up in the file
>     protocol.h.  It is this:
> 
>     --------- begin snippet -----------------------
>     /* Numeric default values */
>     #define AMQP_OPEN_MAX_FRAME_SIZE_DEFAULT 4294967295
>     #define AMQP_OPEN_CHANNEL_MAX_DEFAULT 65535
>     #define AMQP_BEGIN_HANDLE_MAX_DEFAULT 4294967295
>     #define AMQP_SOURCE_TIMEOUT_DEFAULT 0
>     #define AMQP_TARGET_TIMEOUT_DEFAULT 0
>     --------- end snippet -----------------------
> 
> 
> Diffs
> -----
> 
>   proton-c/bindings/python/proton/__init__.py 46b9466 
>   proton-c/include/proton/session.h 94d2869 
>   proton-c/src/engine/engine-internal.h 727f50d 
>   proton-c/src/engine/engine.c 9043e0b 
>   proton-c/src/protocol.h.py bbc0dfe 
>   proton-c/src/transport/transport.c 6abf862 
>   tests/python/proton_tests/engine.py 7a1d539 
> 
> Diff: https://reviews.apache.org/r/36992/diff/
> 
> 
> Testing
> -------
> 
> ctest -VV with Java tests running.
> 
> 
> Thanks,
> 
> michael goulish
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message