You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Rajith Attapattu <ra...@gmail.com> on 2011/03/08 18:35:15 UTC
Request for the following commits to be included in 0.10
Hi Justin,
I'd like the following commits to be included in the 0.10 release branch.
1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 - Fairly
low risk.
This is a simple bug fix.
2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 - Low
risk.
This is an addition to the initial commit made under this JIRA.
It adds default reliability modes and throws an exception for an
unsupported combination.
3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low Risk
The reliability mode is used to mark a transfer unreliable and that flag
is used to determine if a transfer should be stored for replay or not.
Again a fairly straightforward and low risk commit. However I have asked
Rafi to have a quick look at it as well.
4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 - Very low
risk.
Fixes a bug in the initial commit for QPID-2732.
It just sets accept mode to NONE for NO_ACKNOWLEDGE.
Regards,
Rajith
Re: Request for the following commits to be included in 0.10
Posted by Rajith Attapattu <ra...@gmail.com>.
On Tue, Mar 8, 2011 at 3:33 PM, Rajith Attapattu <ra...@gmail.com> wrote:
>
>
> On Tue, Mar 8, 2011 at 3:01 PM, Robbie Gemmell <ro...@gmail.com>wrote:
>
>> Done. To summarise here though:
>>
>> 1. QPID-3109: looks good, although as mentioned on the JIRA a second
>> commit
>> http://svn.apache.org/viewvc?view=rev&rev=1079059 is also required here,
>> to
>> enable compilation to succeed.
>>
>
> There is test case for this at
> http://svn.apache.org/viewvc?view=revision&revision=1076670
> All though I agree that it would be good to add a test case to check close
> on a producer created with a null destination.
> However that functionality is being tested in another test case that I have
> put in place to test bug that I am currently working on.
> So btw the two test cases I think I have covered the bases.
> Due to some in-progress local changes the compilation issue wasn't detected
> on my local. Sorry about that.
>
>
>>
>> 2. & 3. QPID-2732: changes look reasonable.
>>
>> 4. This specific change looks good to restore the prior behaviour, though
>> in
>> general I wouldn't consider work around change of accept mode to be very
>> low
>> risk. I would suggest there should have been tests added to verify the
>> behaviour of the original change that caused the issue though (same goes
>> for
>> all really).
>>
>> I have a test in place for the reliability modes to ensure that the string
> is passed properly and the exception is thrown for the unsupported
> combination.
> But I am also trying to see if I can enhance the test to see if I can test
> if the subscription is created with the correct accept-mode. So far it
> hasn't been easy to get that part going.
> So the tests are on the way.
>
>
Further to the above comment,
All though I don't have an automated test to work out if the accept-mode is
set properly the changes are verified manually using logging.
Similarly Andrew Kennedy has also tested the change he made. So I am quite
confident about the changes.
> Robbie
>>
>> -----Original Message-----
>> From: Justin Ross [mailto:jross@redhat.com]
>> Sent: 08 March 2011 18:08
>> To: dev@qpid.apache.org
>> Subject: Re: Request for the following commits to be included in 0.10
>>
>> Hi, Rajith. I approved item 4.
>>
>> Robbie, would you indicate briefly up or down for 0.10 in the comments of
>> qpid-3109 and -2732?
>>
>> Justin
>>
>> On Tue, 8 Mar 2011, Rajith Attapattu wrote:
>>
>> > Hi Justin,
>> >
>> > I'd like the following commits to be included in the 0.10 release
>> branch.
>> >
>> > 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
>> > Fairly low risk.
>> > This is a simple bug fix.
>> >
>> > 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
>> > Low risk.
>> > This is an addition to the initial commit made under this JIRA.
>> > It adds default reliability modes and throws an exception for an
>> > unsupported combination.
>> >
>> > 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low
>> Risk
>> > The reliability mode is used to mark a transfer unreliable and that
>> > flag is used to determine if a transfer should be stored for replay or
>> not.
>> > Again a fairly straightforward and low risk commit. However I have
>> > asked Rafi to have a quick look at it as well.
>> >
>> > 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
>> > Very low risk.
>> > Fixes a bug in the initial commit for QPID-2732.
>> > It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>> >
>> > Regards,
>> >
>> > Rajith
>> >
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>
Re: Request for the following commits to be included in 0.10
Posted by Rajith Attapattu <ra...@gmail.com>.
On Tue, Mar 8, 2011 at 3:01 PM, Robbie Gemmell <ro...@gmail.com>wrote:
> Done. To summarise here though:
>
> 1. QPID-3109: looks good, although as mentioned on the JIRA a second commit
> http://svn.apache.org/viewvc?view=rev&rev=1079059 is also required here,
> to
> enable compilation to succeed.
>
There is test case for this at
http://svn.apache.org/viewvc?view=revision&revision=1076670
All though I agree that it would be good to add a test case to check close
on a producer created with a null destination.
However that functionality is being tested in another test case that I have
put in place to test bug that I am currently working on.
So btw the two test cases I think I have covered the bases.
Due to some in-progress local changes the compilation issue wasn't detected
on my local. Sorry about that.
>
> 2. & 3. QPID-2732: changes look reasonable.
>
> 4. This specific change looks good to restore the prior behaviour, though
> in
> general I wouldn't consider work around change of accept mode to be very
> low
> risk. I would suggest there should have been tests added to verify the
> behaviour of the original change that caused the issue though (same goes
> for
> all really).
>
> I have a test in place for the reliability modes to ensure that the string
is passed properly and the exception is thrown for the unsupported
combination.
But I am also trying to see if I can enhance the test to see if I can test
if the subscription is created with the correct accept-mode. So far it
hasn't been easy to get that part going.
So the tests are on the way.
> Robbie
>
> -----Original Message-----
> From: Justin Ross [mailto:jross@redhat.com]
> Sent: 08 March 2011 18:08
> To: dev@qpid.apache.org
> Subject: Re: Request for the following commits to be included in 0.10
>
> Hi, Rajith. I approved item 4.
>
> Robbie, would you indicate briefly up or down for 0.10 in the comments of
> qpid-3109 and -2732?
>
> Justin
>
> On Tue, 8 Mar 2011, Rajith Attapattu wrote:
>
> > Hi Justin,
> >
> > I'd like the following commits to be included in the 0.10 release branch.
> >
> > 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
> > Fairly low risk.
> > This is a simple bug fix.
> >
> > 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
> > Low risk.
> > This is an addition to the initial commit made under this JIRA.
> > It adds default reliability modes and throws an exception for an
> > unsupported combination.
> >
> > 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low
> Risk
> > The reliability mode is used to mark a transfer unreliable and that
> > flag is used to determine if a transfer should be stored for replay or
> not.
> > Again a fairly straightforward and low risk commit. However I have
> > asked Rafi to have a quick look at it as well.
> >
> > 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
> > Very low risk.
> > Fixes a bug in the initial commit for QPID-2732.
> > It just sets accept mode to NONE for NO_ACKNOWLEDGE.
> >
> > Regards,
> >
> > Rajith
> >
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
RE: Request for the following commits to be included in 0.10
Posted by Robbie Gemmell <ro...@gmail.com>.
As mentioned in the other email I was not saying this change was a risk (I
was actually the one who noticed this particular problem whilst discussing a
different issue with Andrew earlier). I was just saying that in general I
wouldn't be messing with accept modes at this point in the release process
normally, however taking this corrective action to fix the inadvertent
changes already introduced is a no-brainer.
Robbie.
-----Original Message-----
From: Rajith Attapattu [mailto:rajith77@gmail.com]
Sent: 08 March 2011 20:42
To: dev@qpid.apache.org
Cc: Justin Ross
Subject: Re: Request for the following commits to be included in 0.10
I disagree with item for #4 being a risk.
Andrew's commit was to restore a feature that I dropped when I committed rev
http://svn.apache.org/viewvc?rev=1076800&view=rev
Chances are that there might be apps out there that rely on NO_ACKNOWLEDGE
mode (even though it's not a standard JMS option).
All what Andrew has done is to use that ack mode to set a consumer level
default and if somebody has a destination level setting that will be
overridden.
Rajith
On Tue, Mar 8, 2011 at 3:33 PM, Justin Ross <jr...@redhat.com> wrote:
> Hi, Robbie. I appreciate your caution regarding item 4, and I'm
> prepared to revert that change if it's not truly low risk.
>
> Anyone else have an opinion on this item?
>
>
> On Tue, 8 Mar 2011, Robbie Gemmell wrote:
>
> 4. This specific change looks good to restore the prior behaviour,
> though
>> in
>> general I wouldn't consider work around change of accept mode to be
>> very low risk. I would suggest there should have been tests added to
>> verify the behaviour of the original change that caused the issue
>> though (same goes for all really).
>>
>> Robbie
>>
>> -----Original Message-----
>> From: Justin Ross [mailto:jross@redhat.com]
>> Sent: 08 March 2011 18:08
>> To: dev@qpid.apache.org
>> Subject: Re: Request for the following commits to be included in 0.10
>>
>> Hi, Rajith. I approved item 4.
>>
>> Robbie, would you indicate briefly up or down for 0.10 in the
>> comments of
>> qpid-3109 and -2732?
>>
>> Justin
>>
>> On Tue, 8 Mar 2011, Rajith Attapattu wrote:
>>
>> Hi Justin,
>>>
>>> I'd like the following commits to be included in the 0.10 release
branch.
>>>
>>> 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
>>> Fairly low risk.
>>> This is a simple bug fix.
>>>
>>> 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
>>> Low risk.
>>> This is an addition to the initial commit made under this JIRA.
>>> It adds default reliability modes and throws an exception for an
>>> unsupported combination.
>>>
>>> 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 -
>>> Low
>>>
>> Risk
>>
>>> The reliability mode is used to mark a transfer unreliable and
>>> that flag is used to determine if a transfer should be stored for
>>> replay or
>>>
>> not.
>>
>>> Again a fairly straightforward and low risk commit. However I have
>>> asked Rafi to have a quick look at it as well.
>>>
>>> 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
>>> Very low risk.
>>> Fixes a bug in the initial commit for QPID-2732.
>>> It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>>>
>>> Regards,
>>>
>>> Rajith
>>>
>>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: Request for the following commits to be included in 0.10
Posted by Rajith Attapattu <ra...@gmail.com>.
I disagree with item for #4 being a risk.
Andrew's commit was to restore a feature that I dropped when I committed rev
http://svn.apache.org/viewvc?rev=1076800&view=rev
Chances are that there might be apps out there that rely on NO_ACKNOWLEDGE
mode (even though it's not a standard JMS option).
All what Andrew has done is to use that ack mode to set a consumer level
default and if somebody has a destination level setting that will be
overridden.
Rajith
On Tue, Mar 8, 2011 at 3:33 PM, Justin Ross <jr...@redhat.com> wrote:
> Hi, Robbie. I appreciate your caution regarding item 4, and I'm prepared
> to revert that change if it's not truly low risk.
>
> Anyone else have an opinion on this item?
>
>
> On Tue, 8 Mar 2011, Robbie Gemmell wrote:
>
> 4. This specific change looks good to restore the prior behaviour, though
>> in
>> general I wouldn't consider work around change of accept mode to be very
>> low
>> risk. I would suggest there should have been tests added to verify the
>> behaviour of the original change that caused the issue though (same goes
>> for
>> all really).
>>
>> Robbie
>>
>> -----Original Message-----
>> From: Justin Ross [mailto:jross@redhat.com]
>> Sent: 08 March 2011 18:08
>> To: dev@qpid.apache.org
>> Subject: Re: Request for the following commits to be included in 0.10
>>
>> Hi, Rajith. I approved item 4.
>>
>> Robbie, would you indicate briefly up or down for 0.10 in the comments of
>> qpid-3109 and -2732?
>>
>> Justin
>>
>> On Tue, 8 Mar 2011, Rajith Attapattu wrote:
>>
>> Hi Justin,
>>>
>>> I'd like the following commits to be included in the 0.10 release branch.
>>>
>>> 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
>>> Fairly low risk.
>>> This is a simple bug fix.
>>>
>>> 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
>>> Low risk.
>>> This is an addition to the initial commit made under this JIRA.
>>> It adds default reliability modes and throws an exception for an
>>> unsupported combination.
>>>
>>> 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low
>>>
>> Risk
>>
>>> The reliability mode is used to mark a transfer unreliable and that
>>> flag is used to determine if a transfer should be stored for replay or
>>>
>> not.
>>
>>> Again a fairly straightforward and low risk commit. However I have
>>> asked Rafi to have a quick look at it as well.
>>>
>>> 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
>>> Very low risk.
>>> Fixes a bug in the initial commit for QPID-2732.
>>> It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>>>
>>> Regards,
>>>
>>> Rajith
>>>
>>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
Re: Request for the following commits to be included in 0.10
Posted by Rajith Attapattu <ra...@gmail.com>.
Andrew,
I did do a grep on "NO_ACKNOWLEDGE" and saw those test cases. Thanks for
confirming.
Just wanted to make sure I verified the changes properly.
Btw does the various QueueBrowser tests pass for you ?
Rajith
On Wed, Mar 9, 2011 at 4:55 PM, Andrew Kennedy <
andrewinternational@gmail.com> wrote:
> Yes,
>
>
> ./systests/src/main/java/org/apache/qpid/test/unit/ack/Acknowledge2ConsumersTest.java
>
> ./systests/src/main/java/org/apache/qpid/test/unit/ack/AcknowledgeOnMessageTest.java
>
> ./systests/src/main/java/org/apache/qpid/test/unit/ack/AcknowledgeAfterFailoverOnMessageTest.java
> ./systests/src/main/java/org/apache/qpid/test/unit/ack/AcknowledgeTest.java
>
> ./systests/src/main/java/org/apache/qpid/test/unit/ack/AcknowledgeAfterFailoverTest.java
>
> The AcknowledgeTest class has a test called 'testNoAck' which exercises
> this mode, and all the other classes extend this. This is how I both
> discovered and confirmed the fix for the regression.
>
>
> Andrew.
> --
> -- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
> -- http://grkvlt.blogspot.com/ ? edinburgh : +44 7582 293 255 ;
>
> On 9 Mar 2011, at 20:47, Rajith Attapattu wrote:
>
> In rev 1079986 I added test cases for the reliability options including
>> testing if the correct accept modes are set.
>> I am wondering if there are existing test cases for NO_ACKNOWLEDGE other
>> than the QueueBrowser test cases ?
>>
>> Regards,
>>
>> Rajith
>>
>> On Tue, Mar 8, 2011 at 8:52 PM, Andrew Kennedy <
>> andrewinternational@gmail.com> wrote:
>>
>> On 9 Mar 2011, at 00:40, Robbie Gemmell wrote:
>>
>> Sorry if I wasn't clear. I wasn't saying it shouldn't go in, quite the
>> opposite; in general I wouldn't allow this kind of change at this point,
>> but
>> in this particular case it is actually putting something back roughly the
>> way it was until changed a few days ago so I would actually say that it
>> must
>> be put in.
>>
>> +1 on including #4 in 0.10
>>
>> When we discovered and fixed this, we weren't actually aware it was a
>> regression introduced recently, hence the separate JIRA and careful testing.
>>
>> Andrew.
>> --
>> -- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
>> -- http://grkvlt.blogspot.com/ ? edinburgh : +44 7582 293 255 ;
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>>
>
Re: Request for the following commits to be included in 0.10
Posted by Andrew Kennedy <an...@gmail.com>.
Yes,
./systests/src/main/java/org/apache/qpid/test/unit/ack/
Acknowledge2ConsumersTest.java
./systests/src/main/java/org/apache/qpid/test/unit/ack/
AcknowledgeOnMessageTest.java
./systests/src/main/java/org/apache/qpid/test/unit/ack/
AcknowledgeAfterFailoverOnMessageTest.java
./systests/src/main/java/org/apache/qpid/test/unit/ack/
AcknowledgeTest.java
./systests/src/main/java/org/apache/qpid/test/unit/ack/
AcknowledgeAfterFailoverTest.java
The AcknowledgeTest class has a test called 'testNoAck' which
exercises this mode, and all the other classes extend this. This is
how I both discovered and confirmed the fix for the regression.
Andrew.
--
-- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
-- http://grkvlt.blogspot.com/ ? edinburgh : +44 7582 293 255 ;
On 9 Mar 2011, at 20:47, Rajith Attapattu wrote:
> In rev 1079986 I added test cases for the reliability options
> including testing if the correct accept modes are set.
> I am wondering if there are existing test cases for NO_ACKNOWLEDGE
> other than the QueueBrowser test cases ?
>
> Regards,
>
> Rajith
>
> On Tue, Mar 8, 2011 at 8:52 PM, Andrew Kennedy
> <an...@gmail.com> wrote:
>
> On 9 Mar 2011, at 00:40, Robbie Gemmell wrote:
>
> Sorry if I wasn't clear. I wasn't saying it shouldn't go in, quite the
> opposite; in general I wouldn't allow this kind of change at this
> point, but
> in this particular case it is actually putting something back
> roughly the
> way it was until changed a few days ago so I would actually say
> that it must
> be put in.
>
> +1 on including #4 in 0.10
>
> When we discovered and fixed this, we weren't actually aware it was
> a regression introduced recently, hence the separate JIRA and
> careful testing.
>
> Andrew.
> --
> -- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
> -- http://grkvlt.blogspot.com/ ? edinburgh : +44 7582 293 255 ;
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: Request for the following commits to be included in 0.10
Posted by Rajith Attapattu <ra...@gmail.com>.
In rev 1079986 I added test cases for the reliability options including
testing if the correct accept modes are set.
I am wondering if there are existing test cases for NO_ACKNOWLEDGE other
than the QueueBrowser test cases ?
Regards,
Rajith
On Tue, Mar 8, 2011 at 8:52 PM, Andrew Kennedy <
andrewinternational@gmail.com> wrote:
>
>> On 9 Mar 2011, at 00:40, Robbie Gemmell wrote:
>>
>> Sorry if I wasn't clear. I wasn't saying it shouldn't go in, quite the
>>> opposite; in general I wouldn't allow this kind of change at this point,
>>> but
>>> in this particular case it is actually putting something back roughly the
>>> way it was until changed a few days ago so I would actually say that it
>>> must
>>> be put in.
>>>
>>
> +1 on including #4 in 0.10
>
> When we discovered and fixed this, we weren't actually aware it was a
> regression introduced recently, hence the separate JIRA and careful testing.
>
> Andrew.
> --
> -- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
> -- http://grkvlt.blogspot.com/ ? edinburgh : +44 7582 293 255 ;
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
Re: Request for the following commits to be included in 0.10
Posted by Andrew Kennedy <an...@gmail.com>.
>
> On 9 Mar 2011, at 00:40, Robbie Gemmell wrote:
>
>> Sorry if I wasn't clear. I wasn't saying it shouldn't go in, quite
>> the
>> opposite; in general I wouldn't allow this kind of change at this
>> point, but
>> in this particular case it is actually putting something back
>> roughly the
>> way it was until changed a few days ago so I would actually say
>> that it must
>> be put in.
+1 on including #4 in 0.10
When we discovered and fixed this, we weren't actually aware it was a
regression introduced recently, hence the separate JIRA and careful
testing.
Andrew.
--
-- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
-- http://grkvlt.blogspot.com/ ? edinburgh : +44 7582 293 255 ;
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
RE: Request for the following commits to be included in 0.10
Posted by Justin Ross <jr...@redhat.com>.
Okay, understood.
On Wed, 9 Mar 2011, Robbie Gemmell wrote:
> Sorry if I wasn't clear. I wasn't saying it shouldn't go in, quite the
> opposite; in general I wouldn't allow this kind of change at this point, but
> in this particular case it is actually putting something back roughly the
> way it was until changed a few days ago so I would actually say that it must
> be put in.
>
> Robbie
>
> -----Original Message-----
> From: Justin Ross [mailto:jross@redhat.com]
> Sent: 08 March 2011 20:34
> To: dev@qpid.apache.org
> Subject: RE: Request for the following commits to be included in 0.10
>
> Hi, Robbie. I appreciate your caution regarding item 4, and I'm prepared to
> revert that change if it's not truly low risk.
>
> Anyone else have an opinion on this item?
>
> On Tue, 8 Mar 2011, Robbie Gemmell wrote:
>
>> 4. This specific change looks good to restore the prior behaviour,
>> though in general I wouldn't consider work around change of accept
>> mode to be very low risk. I would suggest there should have been tests
>> added to verify the behaviour of the original change that caused the
>> issue though (same goes for all really).
>>
>> Robbie
>>
>> -----Original Message-----
>> From: Justin Ross [mailto:jross@redhat.com]
>> Sent: 08 March 2011 18:08
>> To: dev@qpid.apache.org
>> Subject: Re: Request for the following commits to be included in 0.10
>>
>> Hi, Rajith. I approved item 4.
>>
>> Robbie, would you indicate briefly up or down for 0.10 in the comments
>> of
>> qpid-3109 and -2732?
>>
>> Justin
>>
>> On Tue, 8 Mar 2011, Rajith Attapattu wrote:
>>
>>> Hi Justin,
>>>
>>> I'd like the following commits to be included in the 0.10 release branch.
>>>
>>> 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
>>> Fairly low risk.
>>> This is a simple bug fix.
>>>
>>> 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
>>> Low risk.
>>> This is an addition to the initial commit made under this JIRA.
>>> It adds default reliability modes and throws an exception for an
>>> unsupported combination.
>>>
>>> 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 -
>>> Low
>> Risk
>>> The reliability mode is used to mark a transfer unreliable and
>>> that flag is used to determine if a transfer should be stored for
>>> replay or
>> not.
>>> Again a fairly straightforward and low risk commit. However I have
>>> asked Rafi to have a quick look at it as well.
>>>
>>> 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
>>> Very low risk.
>>> Fixes a bug in the initial commit for QPID-2732.
>>> It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>>>
>>> Regards,
>>>
>>> Rajith
>>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>>
>> ---------------------------------------------------------------------
>> Apache Qpid - AMQP Messaging Implementation
>> Project: http://qpid.apache.org
>> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
RE: Request for the following commits to be included in 0.10
Posted by Robbie Gemmell <ro...@gmail.com>.
Sorry if I wasn't clear. I wasn't saying it shouldn't go in, quite the
opposite; in general I wouldn't allow this kind of change at this point, but
in this particular case it is actually putting something back roughly the
way it was until changed a few days ago so I would actually say that it must
be put in.
Robbie
-----Original Message-----
From: Justin Ross [mailto:jross@redhat.com]
Sent: 08 March 2011 20:34
To: dev@qpid.apache.org
Subject: RE: Request for the following commits to be included in 0.10
Hi, Robbie. I appreciate your caution regarding item 4, and I'm prepared to
revert that change if it's not truly low risk.
Anyone else have an opinion on this item?
On Tue, 8 Mar 2011, Robbie Gemmell wrote:
> 4. This specific change looks good to restore the prior behaviour,
> though in general I wouldn't consider work around change of accept
> mode to be very low risk. I would suggest there should have been tests
> added to verify the behaviour of the original change that caused the
> issue though (same goes for all really).
>
> Robbie
>
> -----Original Message-----
> From: Justin Ross [mailto:jross@redhat.com]
> Sent: 08 March 2011 18:08
> To: dev@qpid.apache.org
> Subject: Re: Request for the following commits to be included in 0.10
>
> Hi, Rajith. I approved item 4.
>
> Robbie, would you indicate briefly up or down for 0.10 in the comments
> of
> qpid-3109 and -2732?
>
> Justin
>
> On Tue, 8 Mar 2011, Rajith Attapattu wrote:
>
>> Hi Justin,
>>
>> I'd like the following commits to be included in the 0.10 release branch.
>>
>> 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
>> Fairly low risk.
>> This is a simple bug fix.
>>
>> 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
>> Low risk.
>> This is an addition to the initial commit made under this JIRA.
>> It adds default reliability modes and throws an exception for an
>> unsupported combination.
>>
>> 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 -
>> Low
> Risk
>> The reliability mode is used to mark a transfer unreliable and
>> that flag is used to determine if a transfer should be stored for
>> replay or
> not.
>> Again a fairly straightforward and low risk commit. However I have
>> asked Rafi to have a quick look at it as well.
>>
>> 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
>> Very low risk.
>> Fixes a bug in the initial commit for QPID-2732.
>> It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>>
>> Regards,
>>
>> Rajith
>>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
RE: Request for the following commits to be included in 0.10
Posted by Justin Ross <jr...@redhat.com>.
Hi, Robbie. I appreciate your caution regarding item 4, and I'm prepared
to revert that change if it's not truly low risk.
Anyone else have an opinion on this item?
On Tue, 8 Mar 2011, Robbie Gemmell wrote:
> 4. This specific change looks good to restore the prior behaviour, though in
> general I wouldn't consider work around change of accept mode to be very low
> risk. I would suggest there should have been tests added to verify the
> behaviour of the original change that caused the issue though (same goes for
> all really).
>
> Robbie
>
> -----Original Message-----
> From: Justin Ross [mailto:jross@redhat.com]
> Sent: 08 March 2011 18:08
> To: dev@qpid.apache.org
> Subject: Re: Request for the following commits to be included in 0.10
>
> Hi, Rajith. I approved item 4.
>
> Robbie, would you indicate briefly up or down for 0.10 in the comments of
> qpid-3109 and -2732?
>
> Justin
>
> On Tue, 8 Mar 2011, Rajith Attapattu wrote:
>
>> Hi Justin,
>>
>> I'd like the following commits to be included in the 0.10 release branch.
>>
>> 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
>> Fairly low risk.
>> This is a simple bug fix.
>>
>> 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
>> Low risk.
>> This is an addition to the initial commit made under this JIRA.
>> It adds default reliability modes and throws an exception for an
>> unsupported combination.
>>
>> 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low
> Risk
>> The reliability mode is used to mark a transfer unreliable and that
>> flag is used to determine if a transfer should be stored for replay or
> not.
>> Again a fairly straightforward and low risk commit. However I have
>> asked Rafi to have a quick look at it as well.
>>
>> 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
>> Very low risk.
>> Fixes a bug in the initial commit for QPID-2732.
>> It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>>
>> Regards,
>>
>> Rajith
>>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
RE: Request for the following commits to be included in 0.10
Posted by Robbie Gemmell <ro...@gmail.com>.
Done. To summarise here though:
1. QPID-3109: looks good, although as mentioned on the JIRA a second commit
http://svn.apache.org/viewvc?view=rev&rev=1079059 is also required here, to
enable compilation to succeed.
2. & 3. QPID-2732: changes look reasonable.
4. This specific change looks good to restore the prior behaviour, though in
general I wouldn't consider work around change of accept mode to be very low
risk. I would suggest there should have been tests added to verify the
behaviour of the original change that caused the issue though (same goes for
all really).
Robbie
-----Original Message-----
From: Justin Ross [mailto:jross@redhat.com]
Sent: 08 March 2011 18:08
To: dev@qpid.apache.org
Subject: Re: Request for the following commits to be included in 0.10
Hi, Rajith. I approved item 4.
Robbie, would you indicate briefly up or down for 0.10 in the comments of
qpid-3109 and -2732?
Justin
On Tue, 8 Mar 2011, Rajith Attapattu wrote:
> Hi Justin,
>
> I'd like the following commits to be included in the 0.10 release branch.
>
> 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 -
> Fairly low risk.
> This is a simple bug fix.
>
> 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 -
> Low risk.
> This is an addition to the initial commit made under this JIRA.
> It adds default reliability modes and throws an exception for an
> unsupported combination.
>
> 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low
Risk
> The reliability mode is used to mark a transfer unreliable and that
> flag is used to determine if a transfer should be stored for replay or
not.
> Again a fairly straightforward and low risk commit. However I have
> asked Rafi to have a quick look at it as well.
>
> 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 -
> Very low risk.
> Fixes a bug in the initial commit for QPID-2732.
> It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>
> Regards,
>
> Rajith
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: Request for the following commits to be included in 0.10
Posted by Justin Ross <jr...@redhat.com>.
Hi, Rajith. I approved item 4.
Robbie, would you indicate briefly up or down for 0.10 in the comments of
qpid-3109 and -2732?
Justin
On Tue, 8 Mar 2011, Rajith Attapattu wrote:
> Hi Justin,
>
> I'd like the following commits to be included in the 0.10 release branch.
>
> 1. http://svn.apache.org/viewvc?rev=1078961&view=rev - QPID-3109 - Fairly
> low risk.
> This is a simple bug fix.
>
> 2. http://svn.apache.org/viewvc?rev=1078971&view=rev - QPID-2732 - Low
> risk.
> This is an addition to the initial commit made under this JIRA.
> It adds default reliability modes and throws an exception for an
> unsupported combination.
>
> 3. http://svn.apache.org/viewvc?rev=1079408&view=rev - QPID-2732 - Low Risk
> The reliability mode is used to mark a transfer unreliable and that flag
> is used to determine if a transfer should be stored for replay or not.
> Again a fairly straightforward and low risk commit. However I have asked
> Rafi to have a quick look at it as well.
>
> 4. http://svn.apache.org/viewvc?rev=1079402&view=rev - QPID-3127 - Very low
> risk.
> Fixes a bug in the initial commit for QPID-2732.
> It just sets accept mode to NONE for NO_ACKNOWLEDGE.
>
> Regards,
>
> Rajith
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org