search

corvus-ch / rabbitmq-cli-consumer

Consume RabbitMQ messages into any cli program
MIT License
236 stars 36 forks source link

feat: add fast-cgi worker #71

Open johnduro opened 4 years ago

johnduro commented 4 years ago

Hey @corvus-ch ,

Here is a first implementation of fast-cgi, don't hesitate to ask for modification if you think it would be better/more consistent in an other way.

I did not touch nor add any test yet, i prefer you validate this draft before and i ll add them in another commit.

Check carefully the error handling as i'm not totally sure it is ok.

Should i add validation of the configuration ? For executable for example. And where do you see it ?

johnduro commented 4 years ago

Hey @corvus-ch,

Thank you for your review, i'll try to work on it next week.

For the sending format i tried to reproduce the way the data were sent previously but i see what you mean and i think too it would be better to have a single format for the body.

  1. I think sending metadata through header/fast-cgi param would be my first choice but i have to check to doability as it is not obvious the lib will permit it. If this way works i'll send it as a json (encoded in base64 ? not mandatory i think) in a key rmq_metadata.
  2. Sending the json document like now with include would be my third and fallback choice.
  3. Sending 2 documents is doable with the lib so if the first does not work i'll go for this one.

Ok for your others comments, i'll treat them soon.

Regards

corvus-ch commented 4 years ago

If this way works i'll send it as a json (encoded in base64 ? not mandatory i think) in a key rmq_metadata.

Why not sending them as multiple headers? One per meta data field? No need for JSON encode and decode. The worker code can just pick the values he is interested in and ignore the others.

johnduro commented 4 years ago

Some meta might be arrays so we will have to choose a format anyway for them and, from my point of view, i would prefer to do only one decode and maybe validation on a single json document than several.

johnduro commented 4 years ago

Hey @corvus-ch ,

I updated the PR (i'll to be more responsive for the next steps !) with your review, tell me if it is ok and right after i'll start the tests.

For the payload i was able to pass the metadata through a json inside the header HTTP_RMQ_METADATA in the consumer which transform into rmq-metadata at the arrival.

Tell me if things are missing or not the way you'd want them.