Re: [pmacct-discussion] [PATCH] Minor AMQP improvements
Hi Will, Thanks for both patches. They both make sense and will go in mainstream code. This second one i've already validated and is already applied to the code. The other one i just need some extra minimal time for QA (count it will be committed tomorrow). https://www.mail-archive.com/pmacct-commits@pmacct.net/msg01406.html Thanks again! Cheers, Paolo On Wed, Apr 01, 2015 at 11:18:43AM +0800, Will Dowling wrote: > Whoopsy, > > I forgot to address the other (more important) bug I encountered: > > * Content type is only specified for messages published when the > amqp_persistent_msg configuration option is specified. This information > should always be applied to describe the payload of the message. > > * Modified the content type declaration to "application/json" in line with > RFC4627. > > The second change may cause issues in clients relying on the content type flag > being output already - so I can understand why this might not be adopted. > > That being said, making the change should improve compatability with tools > that correctly interpret known MIME types. > > This patch is designed to be applied on top of my previous one. > > Cheers :) > > > Will Dowling > > > > > ___ > pmacct-discussion mailing list > http://www.pmacct.net/#mailinglists ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] [PATCH] Minor AMQP improvements
Hi, On 01-04-15 05:18, Will Dowling wrote: > I forgot to address the other (more important) bug I encountered: > > * Content type is only specified for messages published when the > amqp_persistent_msg configuration option is specified. This information > should always be applied to describe the payload of the message. > > * Modified the content type declaration to "application/json" in line with > RFC4627. > > The second change may cause issues in clients relying on the content type flag > being output already - so I can understand why this might not be adopted. > > That being said, making the change should improve compatability with tools > that correctly interpret known MIME types. > > This patch is designed to be applied on top of my previous one. Looks like I made an error indeed in my original amqp_persistent_msg patch. For my use case, application/json as mime-type should not matter. Kind regards, Nick Douma signature.asc Description: OpenPGP digital signature ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] [PATCH] Minor AMQP improvements
Whoopsy, I forgot to address the other (more important) bug I encountered: * Content type is only specified for messages published when the amqp_persistent_msg configuration option is specified. This information should always be applied to describe the payload of the message. * Modified the content type declaration to "application/json" in line with RFC4627. The second change may cause issues in clients relying on the content type flag being output already - so I can understand why this might not be adopted. That being said, making the change should improve compatability with tools that correctly interpret known MIME types. This patch is designed to be applied on top of my previous one. Cheers :) Will Dowling contentfix.patch Description: Binary data ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
[pmacct-discussion] [PATCH] Minor AMQP improvements
Hi Paolo, Small patch to help address some rough edges on the AMQP plugin, specifically: * Added some helpful error messages when handling unexpected replies from the AMQP server. This is particularly helpful when the plugin fails to re-declare an existing exchange with different parameters. It is possible to get more detailed information (there's an example in tools/common.c in the librabbitmq source), but I felt that this is a good compromise between my time/capability and bloating the code. * Generate an error on compile if —enable-rabbitmq is specified without —enable-jansson. It's clear in the documentation that both are required for AMQP support, but if built without jansson it will silently not publish messages to AMQP. Apologies for the quality (lack thereof) of the patch, I'm not a C guy by trade - hope it helps others out there. Maybe next time I’ll RTFM too ;) Cheers :) Will Dowling amqp-improvements.patch Description: Binary data ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
