> On Sept. 6, 2016, 8:18 p.m., Alan Conway wrote: > > src/qpid/messaging/ConnectionOptions.cpp, line 56 > > <https://reviews.apache.org/r/51638/diff/3/?file=1492012#file1492012line56> > > > > Nit, I don't like double negatives. What about retryReleased(true), > > raiseRefused(true)? To me "do X" is clearer than "don't do nothing (and X > > is implied but not stated) > > Gordon Sim wrote: > I'll switch to your suggestions.
Though not exactly these names, I think the latest patch addresses this. > On Sept. 6, 2016, 8:18 p.m., Alan Conway wrote: > > src/qpid/messaging/amqp/SenderContext.cpp, line 586 > > <https://reviews.apache.org/r/51638/diff/3/?file=1492016#file1492016line586> > > > > Too many warning logs. In a client lib any info that warrants a warning > > needs to be returned via the API somehow so the user can deal with it. Text > > in a log file is not useful to a running client. > > Gordon Sim wrote: > The warnings aren't new. The current behaviour is just to log the warning > and otherwise ignore things. That behaviour will still be available with the > appropriate configuration. While I do take your point about the ideal > solution, I want to improve on the current situation without changing the API > and I feel in this case that some logging is necessary. I have changed things so that they only log at warning if ignoring the nack (i.e. as per original behaviour). If the message is being redelivered or we are raising an exception, only an info level log statement is made. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51638/#review147907 ----------------------------------------------------------- On Sept. 7, 2016, 11:01 a.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51638/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2016, 11:01 a.m.) > > > Review request for qpid and Alan Conway. > > > Repository: qpid-cpp > > > Description > ------- > > Introduces two new connection options (AMQP 1.0 only): > > * max_delivery_count determines how many times we try to resend a 'released' > message. A value of 0, which is the default, retries indefinitely. > * raise_rejected determines whether an MessageRejected exception is raised > when a message is 'rejected', the default is true > > A message is considered 'released' if the outcome is relased, or if the > outcome as modified and the 'undeliverable-here' flag is not set. A message > is considered 'rejected' if the outcome is rejected, if the outcome is > modified *and* the 'undeliverable-here' flag is set, or if it was 'released' > but we have reached the maximum nuber of delivery attempts. > > Original behaviour can be obtained by setting max_delivery_count=1 and > raise_rejected=false. > > > Diffs > ----- > > src/qpid/messaging/ConnectionOptions.h c8c8798 > src/qpid/messaging/ConnectionOptions.cpp d956e9a > src/qpid/messaging/amqp/ConnectionContext.cpp 25dd68d > src/qpid/messaging/amqp/EncodedMessage.cpp cf60046 > src/qpid/messaging/amqp/SenderContext.h 467a8e0 > src/qpid/messaging/amqp/SenderContext.cpp fe8b4d3 > src/qpid/messaging/amqp/SessionContext.h 67b3c1e > src/qpid/messaging/amqp/SessionContext.cpp 92bdea7 > src/qpid/messaging/amqp/Transaction.cpp 754b00d > > Diff: https://reviews.apache.org/r/51638/diff/ > > > Testing > ------- > > > Thanks, > > Gordon Sim > >
