-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom header option for consumers #29
base: master
Are you sure you want to change the base?
Custom header option for consumers #29
Conversation
Hi @ilyashtrikul, thanks for the PR! I also see you fixed the test :) The headers field is definitely useful. But you can achieve the same result if you supply a custom serializer. And supplying a custom serializer is standard for symfony messenger. So I'm wondering if you have considered that option? |
You're right, custom serializer is a solution, but it's not just copy-paste with some fixes, it's a bundle with own configuration and etc only for one header field. |
If you think this changes are not part of this library or will not make work a bit easy - feel free to close this PR ;) |
@ilyashtrikul I'm not sure what you mean with own bundle with configuration. You just specify the service ID of your Serializer in the Messenger config consumer:
dsn: '%env(KAFKA_URL)%'
serializer: App\Infrastructure\Messenger\MySerializer |
MySerializer don't know which type of message come in (without specific headers), so it should be some hardcode for topic -> class in MySerializer or make serializer with own configuration for topic-class (which is better move to bundle for flexible configuring) and it will be a lot of serializer services. My way is set topic-class config nearby all consumer configuration. |
@ilyashtrikul Are you using Avro? Avro includes the qualified name, that you can use in your serializer to determine the output class |
Hi @KonstantinCodes! I find it really helpful to have the ability to pass the Event class as a header option. I'm using your package in one of my projects (Huge thanks for creating this package!!) and I consume about 10+ different topics and for each, I had to introduce 10+ serializers. If we have the ability to mention, okay for this topic this is the Event class by mentioning it in the header, then I can simplify this by just having a single generic serializer What I mean is this
Then in a KafkaSerializer, I would just have
What do you think? Update: If you have a doubt as to whether the headers is the best place to mention the event class, I'd like to suggest an alternative approach like below
|
@gayansanjeewa May I ask, how the payload is serialized? it it plain json? |
Hey @KonstantinCodes So the /**
* @param string[] $encodedEnvelope
*/
public function decode(array $encodedEnvelope): Envelope
{
if (empty($encodedEnvelope['body'])) {
return new Envelope(new EmptyMessageEvent());
}
/** @var OrderDelayedEvent $message */
$message = $this->serializer->deserialize($encodedEnvelope['body'], OrderDelayedEvent::class, 'json');
return new Envelope($message);
} and the payload would be like {
"order_id": "3c77d2f1-4b2a-4eac-bef3-a3c4c74fc64a",
"user_id": 3300317213,
"user_email": "[email protected]",
"order_number": "OPL-100001111"
} |
Hey @KonstantinCodes ping 😄 |
@gayansanjeewa Overriding these properties doesn't really seem like an ideal solution. I'd rather just pass the KafkaMessage to the Serializer so you can map the topic name to a class. |
Okay, thanks for your feedback @KonstantinCodes! |
Messenger Serializer requires
type
key with class FQN. This PR makes it possible with headers option in transport config.