search

MeowWolf / node-red-contrib-amqp

AMQP nodes for node-red
22 stars 18 forks source link

Fix #41 - support xml payload #43

Closed ashok0617 closed 3 years ago

ashok0617 commented 3 years ago

As suggested - handled stringify using properties member stringify flag. By default keeping stringify for backward compatibility

amodelbello commented 3 years ago

@ashok0617 - please take a look at the changes I just pushed and let me know if this satisfies your use case. I ended up renaming the config field and moving it under propertes.headers.

I also added some minor ui changes to fix an unrelated typedinput ui problem but you can probably ignore those.

ashok0617 commented 3 years ago

@ashok0617 - please take a look at the changes I just pushed and let me know if this satisfies your use case. I ended up renaming the config field and moving it under propertes.headers.

I also added some minor ui changes to fix an unrelated typedinput ui problem but you can probably ignore those.

@amodelbello, Looks it will work. Are you going to publish new version? I will check getting latest code. To me appears to be to keep simple not having depth of objects hence added in properties itself. I just tested and see this value under properties. Since properties.headers is optional amqp message object, I believe this will work. Test result attached as screenshot image image

amodelbello commented 3 years ago

@ashok0617 Thanks, good to hear. The reason I moved it to be under headers was because the properties object represents the amqp properties object as defined in the spec. It wouldn't be a good idea to just add a new field to that. Additionally the headers field within the properties object is traditionally used for ad hoc custom configurations like this one. Going to merge and have a new release shortly.

ashok0617 commented 3 years ago

@ashok0617 Thanks, good to hear. The reason I moved it to be under headers was because the properties object represents the amqp properties object as defined in the spec. It wouldn't be a good idea to just add a new field to that. Additionally the headers field within the properties object is traditionally used for ad hoc custom configurations like this one. Going to merge and have a new release shortly.

@amodelbello , thank you very much taking it up considering the requested change. Already understood by looking into amqp message definition and had my comments as <Since properties.headers is optional amqp message object, I believe this will work.>. thanks. cheers