zf-fr / zfr-eb-worker

ZfrEbWorker is a thin layer to simplify the usage of SQS queues and Elastic Beanstalk environments
MIT License
8 stars 2 forks source link

Use SQS attributes for storing message name #36

Closed bakura10 closed 7 years ago

bakura10 commented 7 years ago

As of today, the name and payload of a message are encoded in the body: https://github.com/zf-fr/zfr-eb-worker/blob/master/src/MessageQueue/MessageQueue.php#L75

This forces us to decode the message to get the message name to resolve, and also it displays this metadata (job name) in the console when inspecting the messages.

A more correct way would be to simplify the body so that the body itself is just the payload:

'body'  => $message->getPayload()

And set the name as an attribute instead. This attribute can later on be read from the header that Elastic Beanstalk adds.

I'm not sure when how to do this change backward compatible for job that are adding in older version. What I've thought is:

1) We release a new version of ZfrEbWorker which includes a first attribute "Version" set to 5. This will allows new messages that are inserted to come with this first attribute in a non-BC way. 2) We release version 6 that removes the name, and flatten the body structure to only contain payload. 3) Based on the version of the message, the Worker middleware either pulls data from body, or from attribute name (https://github.com/zf-fr/zfr-eb-worker/blob/master/src/Middleware/WorkerMiddleware.php#L83)

ping @danizord

bakura10 commented 7 years ago

For instance, this is how one of our job look currently:

capture d ecran 2016-12-30 a 10 22 12

After this change, the idae would be to have the "name" moved to the "Message attributes" part, and the body to be only:

{
   "sms_id": "xxx",
   "shop_domain": "xxx"
}
danizord commented 7 years ago

@bakura10 I see no reason to keep compatibility with messages issued from older versions of ZfrEbWorker, can you clarify? We could just break it and make sure that we update the library everywhere at the same time? :P

bakura10 commented 7 years ago

YEah. In our case it's possible (we have still few messages so even if one or two fails.... it would be retried). But I think that if people are having a lot of messages in the queue it may be problematic as old messages won't be processed by the worker. But yeah ultimately as we are likely the only user of this, and that we're doing a major upgrade, we could do that directly ^^